Skip to content

Add caching for GenericVersion instances in GenericVersionScheme #1498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

gnodet
Copy link
Contributor

@gnodet gnodet commented Jun 19, 2025

Summary

This PR implements caching for GenericVersion instances in GenericVersionScheme to improve performance when parsing identical version strings repeatedly, which is common in dependency resolution scenarios.

Changes Made

Core Implementation

  • Added ConcurrentMap<String, GenericVersion> cache in GenericVersionScheme with initial capacity of 256
  • Modified parseVersion() method to use computeIfAbsent() for thread-safe caching
  • Updated main method to use scheme.parseVersion() for consistency with caching approach

Test Improvements

  • Added comprehensive caching tests in GenericVersionSchemeTest to verify same instances are returned for identical strings
  • Created performance test class GenericVersionSchemeCachingPerformanceTest with tests for:
    • Performance comparison between cached and direct instantiation
    • Correctness verification
    • Concurrent access safety
  • Updated test utilities to use shared scheme instances:
    • GenericVersionRangeTest.newVersion() now uses versionScheme.parseVersion()
    • UnionVersionRangeTest uses shared scheme instance instead of creating new ones

Benefits

  1. Performance: Significant speedup for repeated parsing of identical version strings
  2. Memory efficiency: Identical version strings share the same GenericVersion instance
  3. Thread safety: Uses ConcurrentMap.computeIfAbsent() for safe concurrent access
  4. Backward compatibility: No breaking changes to public API
  5. Immutability: Safe to cache since GenericVersion is immutable

Technical Details

  • Thread Safety: Uses ConcurrentHashMap with computeIfAbsent() for thread-safe lazy initialization
  • Cache Key Strategy: Uses exact string representation as key ("1.0" and "1.0.0" are different entries)
  • Memory Management: Initial capacity of 256 provides good performance for typical usage patterns

Testing

The implementation includes:

  • Unit tests verifying caching behavior
  • Performance tests demonstrating benefits
  • Concurrent access tests ensuring thread safety
  • Existing tests continue to pass, ensuring no regressions

This optimization is particularly beneficial in dependency resolution scenarios where the same version strings are parsed multiple times during graph traversal and conflict resolution.

Files Changed

  • GenericVersionScheme.java - Core caching implementation
  • GenericVersionSchemeTest.java - Added caching tests
  • GenericVersionRangeTest.java - Updated to use scheme consistently
  • UnionVersionRangeTest.java - Updated to use shared scheme instance
  • GenericVersionSchemeCachingPerformanceTest.java - New performance test class

Pull Request opened by Augment Code with guidance from the PR author

gnodet and others added 3 commits June 19, 2025 14:49
- Add ConcurrentMap-based cache in GenericVersionScheme to store parsed versions
- Use computeIfAbsent for thread-safe caching of GenericVersion instances
- Update main method to use scheme.parseVersion() for consistency
- Add comprehensive tests for caching behavior and thread safety
- Update test utilities to use shared scheme instances for better performance
- Add performance test demonstrating caching benefits

This optimization reduces repeated parsing overhead for identical version strings,
which is common in dependency resolution scenarios. Since GenericVersion is
immutable, caching is safe and provides significant performance improvements.
- Format GenericVersionSchemeCachingPerformanceTest.java according to project style
- Fix array formatting and remove trailing whitespace
- Ensure compliance with project code style guidelines
@cstamas
Copy link
Member

cstamas commented Jun 19, 2025

Not for merge as is, as this cache just grows and never lets version instances free. We need to see some numbers first...
Ideal would be to keep cache in session or somehow tied to session? Flush per session? etc

@gnodet gnodet marked this pull request as draft June 20, 2025 05:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants