3836140 of #1345 introduced a subtle issue:
- Previously
process_easyconfig returned an independent EasyConfig instance through deepcopy in case it was cached.
- After this
ec.copy() is used but ec is not an EasyConfig but a dict containing one as a value. Hence the same EasyConfig instance is always returned
When that instance is modified this will be visible when process_easyconfig is called with the same path again. As callers do not expect this and assume they can either modify it get the original values (from the file) this can cause subtly wrong behavior.
#4818 attempted to fix this by calling .copy on the easyconfig instance as was (indirectly) done before.
However that causes heavy slowdowns as easyconfigs get parsed many times: EasyConfig.copy is implemented to parse the file again instead of copying instance values. Parsing an easyconfig involves resolving dependencies. Those are obtained through process_easyconfig which copies that dependency again, triggering parsing it again (and all its dependencies)
(I think) we get for a dependency chain of A->B->C:
- Parse A
- Resolve B
- Parse B
- Resolve C
- Parse C
- Copy C
- Parse C
- Copy B
- Parse B
- Resolve C
- Copy C
- Copy A
- Resolve B
- Copy B
- Parse B
- Resolve C
- Copy C
- Parse C
A better solution of EasyConfig.copy is required to resolve this which might not be possible:
A similar issue applies to a "valid" deepcopy: We will at least deepcopy all toolchain components and their dependencies multiple times as well as common dependencies.
We probably don't need deep-copies of those and could just copy them directly, i.e. the references, as we shouldn't modify dependency instances.
3836140 of #1345 introduced a subtle issue:
process_easyconfigreturned an independentEasyConfiginstance throughdeepcopyin case it was cached.ec.copy()is used butecis not anEasyConfigbut adictcontaining one as a value. Hence the sameEasyConfiginstance is always returnedWhen that instance is modified this will be visible when
process_easyconfigis called with the same path again. As callers do not expect this and assume they can either modify it get the original values (from the file) this can cause subtly wrong behavior.#4818 attempted to fix this by calling
.copyon the easyconfig instance as was (indirectly) done before.However that causes heavy slowdowns as easyconfigs get parsed many times:
EasyConfig.copyis implemented to parse the file again instead of copying instance values. Parsing an easyconfig involves resolving dependencies. Those are obtained throughprocess_easyconfigwhich copies that dependency again, triggering parsing it again (and all its dependencies)(I think) we get for a dependency chain of A->B->C:
A better solution of
EasyConfig.copyis required to resolve this which might not be possible:A similar issue applies to a "valid" deepcopy: We will at least deepcopy all toolchain components and their dependencies multiple times as well as common dependencies.
We probably don't need deep-copies of those and could just copy them directly, i.e. the references, as we shouldn't modify dependency instances.