[UIMA-6393]: Circular imports break resource manager cache #155
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…to-access-config-names-of-fresh-context [UIMA-6390] NPE when trying to access config names of fresh context
- Added a minimal and a randomized test to reproduce the problem
- Started refactoring the import resolving - WIP - Added another test - Renamed some test files
- various exploratory changes for testing different solutions, very work in progress
- comment temp code variants
- resolve imports if possible for better caching - added another test for that scenario
- some cleanup and renaming
- allow cycles again in bulk test
- fixed and refactored algorithm
- Removed boolean return value for `resolvedImports` - we can simply check if there are no more imports in the resolved descriptor to see if it was fully resolved - Refactored methods to load an imported type system from cache or from disk - Pass original import through to the disk loading method because we want to have the original import URL as well as the resolved URL there for an error message - Bit of cleaning up - Added several comments and a sprinkle of JavaDoc - Added cache synchronization
- Remove type cast from TypeSystemDescription to TypeSystemDescription_impl
- Change test so that it incrementally generates larger scenarios - Pull code to generate the scenario out of the principal test method - Improve import resolving performance by remembering which type systems were already visited (still over-generates type descriptions in collectec types and likely in the cached type systems)
- Skip self-imports - Improve test - Use a set to collect the unique found type descriptions
- Undo change to use a set for the collected types - this breaks some tests
- Fix bug that types that are imported from a type system which has already been visited last in a circle may not be resolved when the type system is reached a second time - Added test for this scenario - Added a few comments
- Added comments - Updated a test to make it more detailed
- Keep track of where imported types come from and avoid collecting the same type twice (theoretically - clashes a bit with the inlining of types... but the inlining of types is a bad idea anyway) - Added flag `iReallyReallyWantToInlineTypesAndRiskStrongAccumulation` to control the inlining of types to facilitate checking how the code behaves with and without inlining - By default the flag is on because we have some unit tests that check the cache state and these currently would fail if inlining would be turned off
- Use a proper object instead of a concatenated string as the disambiguation key - the key uses object equality (basically turns the `aAllImportedTypes` into a LinkedIdentityHashSet
- No longer inline types or remove imports
- Pulled import resolving code out into a separate class - Pulled import resolving tests out into a separate test class - Changed log layout - Use generic import resolver in TypeSystemDescription_impl, FsIndexCollection_impl and TypePriorities_impl - Deprecate import resolving method allowing the passing "already imported" URLs in all three descriptor types - Added back setting the source URL on imports if it was null - Added back using the "already imported" argument of the deprecated resolve method in the generic import resolver - not because it is a good idea, just for backwards compatibility
- Deprecate ResourceManager_impl.getImportUrlsCache() - it is no longer used. - Rename DescriptionAdapter to DescriptorAdapter
- Move import resolver test data to different test folder - Better/added missing license header in the test files
- Defensive copy of the type descriptors into the resolved type system to avoid cache pollution - Adjust unit test to check that cache pollution risk is gone - Made complex import resolving scenario test multi-threaded
- Do not defensively copy type descriptions that already were in the type system descritor before resolving - Added unit test for this change - Added unit test for self-importing
- Log a warning with decreasing frequency if somebody still uses `ResourceManager_impl.getImportUrlsCache()`
- Remove unused list - Added an information message about multi-threaded average duration to the randomized test
…urce-manager-cache_pk * main: (65 commits) [UIMA-6390] NPE when trying to access config names of fresh context [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6372] Upgrade from JUnit 3 to JUnit 4 to JUnit 5 [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6373] Format UIMA Core Java SDK codebase [UIMA-6378] Java SDK does not build on Java 16 [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite [UIMA-6374] Create CAS (de)serialization test suite ... % Conflicts: % uimaj-core/src/main/java/org/apache/uima/resource/metadata/FsIndexCollection.java % uimaj-core/src/main/java/org/apache/uima/resource/metadata/TypePriorities.java % uimaj-core/src/main/java/org/apache/uima/resource/metadata/TypeSystemDescription.java % uimaj-core/src/main/java/org/apache/uima/resource/metadata/impl/FsIndexCollection_impl.java % uimaj-core/src/main/java/org/apache/uima/resource/metadata/impl/TypePriorities_impl.java % uimaj-core/src/main/java/org/apache/uima/resource/metadata/impl/TypeSystemDescription_impl.java % uimaj-core/src/test/java/org/apache/uima/resource/metadata/impl/TypeSystemDescription_implTest.java
reckart
deleted the
bugfix/UIMA-6393-Circular-imports-break-resource-manager-cache_pk
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
resolveImports(). The imports are also no longer removed from the cached descriptors. This simplifies the algorithm and fixes the bug. Performance implications seem acceptable.ImportResolverclass and is re-used for FSIndexCollections and TypePriorities