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

Add a way to configure caches Jackson uses #2502

Closed
cowtowncoder opened this issue Oct 16, 2019 · 10 comments
Closed

Add a way to configure caches Jackson uses #2502

cowtowncoder opened this issue Oct 16, 2019 · 10 comments
Labels
2.16 Issues planned for 2.16 most-wanted Tag to indicate that there is heavy user +1'ing action
Milestone

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented Oct 16, 2019

(note: follow up on #2456 )

Currently most of caching Jackson does internally (for reusing things like serializers, deserializers, to avoid re-resolving some generic types and so on) is internally defined with settings that can not be changed. But it would make sense to expose an interface, like cache provider, that could be changed to override either some parameters (sizes for default caches) or even cache implementations if that matters. This would also allow things like instrumentation (for cache hit rates) and using time-based eviction, dynamic sizing and so on.

Known caches are in:

Some modules may add their own caches as well although not many do; and so configuration of those can be separate from databind.

@cowtowncoder
Copy link
Member Author

Marking as "most-wanted" as this tends to come up occasionally, esp. wrt Scala module.

@pjfanning
Copy link
Member

If there is a rewrite of LRUMap, maybe ConcurrentLinkedHashMap could be copied into jackson-databind. LRUMap is currently based on ConcurrentHashMap but ConcurrentLinkedHashMap allows for an eviction policy that would allow one in, one out semantics once maxEntries is reached.

@cowtowncoder
Copy link
Member Author

cowtowncoder commented Jul 1, 2022

Merged #3531 (improve LRUMap impl not to clear all entries on max size) which is related but not necessarily 100% what this is so leaving open.

Also implemented #3311 (add serializer cache limits in 2.14).

These should make it easier to work on this one.

@pjfanning Now that both SerializerCache and DeserializerCache (see #1905) use New and Improved LRUMap, it might be possible to work on this. I think we'd need a new config object to be passed via ObjectMapper.builder() but should be doable for 2.x not just 3.0. Might not make it in 2.14 due to timing, but at least closer to being doable.

@JooHyukKim
Copy link
Member

JooHyukKim commented Aug 28, 2023

Tried writing a PoC in #4101....(click to expand)
    public void testCacheConfig() throws Exception {
        CacheProvider cacheProvider = CacheProvider.builder()
                .forDeserializerCache(new SimpleExampleCache())
                //... and more of forXXXCache() methods
                .build();
        
        ObjectMapper mapper = JsonMapper.builder().cacheProvider(cacheProvider).build();
        
        assertNotNull(mapper.readValue("{\"point\":24}", RandomBean.class));
    }

Straightforward solution. We just add on more cache configuration points (e.g. DeserializerCache, `TypeFactory, and so on). This way seems intuitive for users and module maintainers. WDYT?

@JooHyukKim
Copy link
Member

Known caches are in:

And also JacksonAnnotationIntrospector.__annotationsInside.

@JooHyukKim
Copy link
Member

JooHyukKim commented Sep 6, 2023

Excecution Tracker

@JooHyukKim
Copy link
Member

There is ongoning delay due to I am moving to a new company, returned laptop to previous, and I am waiting for a new laptop 😅. Will get back to it in a on monday or tuesday

@cowtowncoder
Copy link
Member Author

No problem -- thank you for update @JooHyukKim !

I am thinking of soon closing 2.16 branch from new features but we can definitely make sure this gets completed, at least wrt SerializerCache (the main remaining important cache -- I don't think TypeFactory matters nearly as much).

@cowtowncoder cowtowncoder changed the title Add a way to configure Caches Jackson uses Add a way to configure caches Jackson uses Sep 23, 2023
@cowtowncoder cowtowncoder added this to the 2.16.0 milestone Sep 23, 2023
@cowtowncoder cowtowncoder added 2.16 Issues planned for 2.16 and removed 2.15 Issues planned for 2.15 labels Sep 23, 2023
@JooHyukKim
Copy link
Member

JooHyukKim commented Sep 26, 2023

Should we keep this issue open? While there are still a few potential caches for the CacheProvider, we've addressed the ones that make a significant difference. It might be beneficial for a sense of achievement and clarity to close this issue. What do you think? :)

@cowtowncoder
Copy link
Member Author

@JooHyukKim Let's close this -- I don't think other listed caches are worth worrying about in general. Can tackle if someone finds them problematic.
And I think I already added an entry for this issue in release notes.
Big achievement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.16 Issues planned for 2.16 most-wanted Tag to indicate that there is heavy user +1'ing action
Projects
None yet
Development

No branches or pull requests

3 participants