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

bugfix(BeanELResolver): using WeakReferences instead of SoftReference… #219

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lprimak
Copy link

@lprimak lprimak commented May 31, 2024

…s for caching to avoid unnecessary memory pressure

Possibly fixes #214
fixes #218
Possibly supersedes #215

…s for caching to avoid a memory unnecessary memory pressure
@lprimak
Copy link
Author

lprimak commented Jun 5, 2024

Thinking more about this, I am not sure this is going to cache things correctly. BeanProperties is now a WeakReference so it will most likely be cleaned out on every GC.

@markt-asf What do you think?
I am not sure that clean() method can be avoided.
Or maybe introduce a 'Real' cache via a library of some kind?

@lprimak
Copy link
Author

lprimak commented Jun 6, 2024

Thought about this for 2-3 hours, made a few examples, and I came to the conclusion (again)
that there is no automatic way to "scope" the cache correctly.
Either it will cause unnecessary memory pressure (SoftReference) or can hang on too long (WeakReference or time/LRU-based cache)

The only way to correctly invalidate this cache is to have a clearProperties() method as in #215

@lprimak lprimak marked this pull request as draft June 6, 2024 04:16
@lprimak
Copy link
Author

lprimak commented Jun 6, 2024

I have made this PR draft, as it probably should be closed in favor of #215

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.

None yet

1 participant