Skip to content
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

Change LRUMap to just evict one entry when maxEntries reached #3530

Closed
pjfanning opened this issue Jun 26, 2022 · 3 comments
Closed

Change LRUMap to just evict one entry when maxEntries reached #3530

pjfanning opened this issue Jun 26, 2022 · 3 comments
Milestone

Comments

@pjfanning
Copy link
Member

Current implementation clears full map when maxEntries reached. jackson-dataformat-csv has its own separate LRUMap[1] and it only evicts the eldest entry when you add a new entry after the maxEntries is reached. Would it be possible to have jackson-databind LRUMap use similar logic? The change would require LRUMap to subclass LinkedHashMap.

[1] https://github.com/FasterXML/jackson-dataformats-text/blob/2.14/csv/src/main/java/com/fasterxml/jackson/dataformat/csv/impl/LRUMap.java

@pjfanning pjfanning added the to-evaluate Issue that has been received but not yet evaluated label Jun 26, 2022
@pjfanning
Copy link
Member Author

Closing as this rehashes #2502 and FasterXML/jackson-module-scala#428. Making the change in the issue description would incolve dropping the ConcurrentHashMap that currently backs LRUMap.

@pjfanning pjfanning changed the title Chnage LRUMap to just evict one entry when maxEntries reached Change LRUMap to just evict one entry when maxEntries reached Jun 26, 2022
@cowtowncoder
Copy link
Member

Makes sense. And just to be clear: the reason LRUMap just cleared it all was because incremental updates were not available using backing map choice (originally LinkedHashMap was used, but there were concurrency problems as both reads and writes need synchronization and only writes were sync'ed... long story). Clear-all was not a feature we want, just the limitation of the approach.

@cowtowncoder
Copy link
Member

Hooray! Big thanks to @pjfanning for contributing the implementation and @ben-manes for helping with suggestions.

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

No branches or pull requests

2 participants