jackson-databind icon indicating copy to clipboard operation
jackson-databind copied to clipboard

Add a way to configure Caches Jackson uses

Open cowtowncoder opened this issue 4 years ago • 3 comments

(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:

  • DeserializerCache
  • SerializerCache
  • TypeFactory
  • RootNameLookup (small)

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

cowtowncoder avatar Oct 16 '19 02:10 cowtowncoder

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

cowtowncoder avatar Dec 04 '20 21:12 cowtowncoder

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.

pjfanning avatar Jun 26 '22 14:06 pjfanning

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.

cowtowncoder avatar Jul 01 '22 00:07 cowtowncoder

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 avatar Aug 28 '23 23:08 JooHyukKim

Known caches are in:

And also JacksonAnnotationIntrospector.__annotationsInside.

JooHyukKim avatar Aug 29 '23 15:08 JooHyukKim

Excecution Tracker

  • [x] "Minor addition to DeserializerCache" (https://github.com/FasterXML/jackson-databind/commit/c4303bbc839dd80398eaa8be33c006e61c0fceb6)
  • [x] More work wrt 4101 (Skeletal works) (https://github.com/FasterXML/jackson-databind/commit/b2487d0a33a347ce476282f94a0a125f2adcb559)
  • [x] for DeserializerCache, #4101
    • [x] bunch of tweaking and re-touch done by @cowtowncoder. Referenced in same PR 4101
  • [x] for SerializerCache #4111
    • [x] #4114
  • [x] #4115
  • [x] Improve JavaDoc wrt setting DefaultCacheProvider cache sizes to zero (as per comment)

JooHyukKim avatar Sep 06 '23 03:09 JooHyukKim

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

JooHyukKim avatar Sep 09 '23 05:09 JooHyukKim

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 avatar Sep 09 '23 16:09 cowtowncoder

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? :)

JooHyukKim avatar Sep 26 '23 23:09 JooHyukKim

@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!

cowtowncoder avatar Sep 27 '23 01:09 cowtowncoder