Split lazy-loading logic into separate subclasses #64
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change slightly improves:
Structures and arrays of structures no longer store a
_lazyboolean attribute. Lazy contexts are only stored when an IDS is lazy loaded.Observed 0.5-1.5% memory reduction as measured by
pympler.asizeof.asizeof.if self._lazychecks in (arrays of) structures and initialization ofself._lazy. This is hardly noticable in most use cases. Specific use cases, like making a deepcopy of an IDS, can be sped up by a couple percent.The introduction of new subclasses has some impact on users:
<LazyIDSToplevel (IDS:magnetics)>instead of<IDSToplevel (IDS:magnetics)>. This is actually very useful as it makes it immediately clear that the IDS is lazy loaded! But it's a change that may be unexpected when upgrading to a newer minor versionif type(xyz) is IDSStructure:. This is not good practice, but this logic would stop working for lazy loaded IDSs. The recommendation is to checkif isinstance(xyz, IDSStructure)which keeps working correctly.Testing succeeded (both unit tests, and the IMAS-Paraview plugins which rely heavily on lazy loading). Also, from a software engineering perspective, this change is useful. However, I had hoped for bigger performance improvements.
Given the potential impact for users, we should discus if we want to include this in an
IMAS-Python2.x release. @olivhoenen what are your thoughts on this?