Contention in `TypeNameIdResolver.idFromClass()`

[CodingFabian]

We are using Jackson 2.9.10 and we have many threads serializing Java objects onto kafka.

Under high load (cough, black friday), we see several threads blocking on the type map that is contained in TypeNameIdResolver

downstream-7 id=121 state=BLOCKED
    - waiting to lock <0x1a5caa08> (a java.util.HashMap)
     owned by downstream-4 id=118
    at com.fasterxml.jackson.databind.jsontype.impl.TypeNameIdResolver.idFromClass(TypeNameIdResolver.java:95)
    at com.fasterxml.jackson.databind.jsontype.impl.TypeNameIdResolver.idFromValue(TypeNameIdResolver.java:83)
    at com.fasterxml.jackson.databind.jsontype.impl.TypeSerializerBase.idFromValue(TypeSerializerBase.java:82)
    at com.fasterxml.jackson.databind.jsontype.impl.TypeSerializerBase._generateTypeId(TypeSerializerBase.java:67)
    at com.fasterxml.jackson.databind.jsontype.impl.TypeSerializerBase.writeTypePrefix(TypeSerializerBase.java:45)
    at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeWithType(BeanSerializerBase.java:600)
    at com.fasterxml.jackson.databind.ser.std.CollectionSerializer.serializeContents(CollectionSerializer.java:147)
    at com.fasterxml.jackson.databind.ser.std.CollectionSerializer.serialize(CollectionSerializer.java:107)
    at com.fasterxml.jackson.databind.ser.std.CollectionSerializer.serialize(CollectionSerializer.java:25)
    at com.fasterxml.jackson.databind.ser.BeanPropertyWriter.serializeAsField(BeanPropertyWriter.java:727)
    at com.fasterxml.jackson.module.afterburner.ser.ObjectMethodPropertyWriter.serializeAsField(ObjectMethodPropertyWriter.java:40)
    at com.fasterxml.jackson.databind.ser.std.BeanSerializerBase.serializeFields(BeanSerializerBase.java:719)
    at com.fasterxml.jackson.databind.ser.BeanSerializer.serialize(BeanSerializer.java:155)
    at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider._serialize(DefaultSerializerProvider.java:480)
    at com.fasterxml.jackson.databind.ser.DefaultSerializerProvider.serializeValue(DefaultSerializerProvider.java:400)
    at com.fasterxml.jackson.databind.ObjectWriter$Prefetch.serialize(ObjectWriter.java:1392)
    at com.fasterxml.jackson.databind.ObjectWriter._configAndWriteValue(ObjectWriter.java:1120)
    at com.fasterxml.jackson.databind.ObjectWriter.writeValueAsBytes(ObjectWriter.java:1017)

I see that this is a synchronized block here:
https://github.com/FasterXML/jackson-databind/blob/master/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java#L99

Could that be changed? Should we somehow use several TypeNameIdResolvers (how would one do that?

[CodingFabian]

One solution we are thinking of is to copy() the ObjectMapper instance before obtaining a writer (which we then put in a ThreadLocal)

[cowtowncoder]

Results should quickly get cached, so even with synchronization it would seem odd that it would become bottle-neck, so my first suspicion is usually regarding whether something else is not getting reused.
In this case, scope of TypeNameIdResolver is per-property (or possibly, per-polymorphic class).

I would not recommend copying ObjectMapper as that is as expensive operation is fully creating a new mapper instance, and having separate copy per-thread is rather heavy-weight (due to caching of all (de)serializers).

However: I can change synchronization itself to reduce scope so that (usually successful) lookup, and other processing (introspection of cached id not found, addition to lookup map) are not within same sync block, and use separate sync for put'ing of new value.

My main concern is just that I do not have any way to really test this, so if there was a way to test this somehow (like running under profiler) to verify improvement, that'd be great.

But I can make a change to get released in 2.10.2 I think; I have made similar changes to synchronized scope in the past, with good results.

[CodingFabian]

The ThreadLocal copies gave us 10% throughput improvements. We have plenty of memory, so that was a valuable tradeoff.

Not sure if we can make a good benchmark, but now that we know where the Threads block, we should be able to compare versions with our loadtests.

[CodingFabian]

turning _idToType and _typeToId into ConcurrentHashMap with computeIfAbsent should probably fix this.

[cowtowncoder]

Jackson-databind can not use Java 8 methods until 3.0, so unfortunately it is not available.
But I think change I am checking in will reduce or remove the issue.

[cowtowncoder]

With fix, it should be possible to build (or use from Sonatype OSS) 2.10.2-SNAPSHOT build to see if change resolved thread block.

I am still somewhat surprised as to how it could have resulted in performance problems: after resolving type ids once, they should stick, unless ObjectMapper instances were discarded, or deserializers not cached. Or something...

[CodingFabian]

We will check it. But you are right. That’s why I proposed the ConcurrentHashMap. I think in our case the successful reads just block (so the synchronized block that is still in). We have around 50k ObjectMapper.writeValue per second.

[cowtowncoder]

That rate does explain why it could matter (impressive that polymorphic handling can scale as that seems like it could add quite a bit of overhead).
But the part I do not get is not that resolution gets called often enough to matter, but rather that "miss" case -- id not yet resolved -- would occur often enough to matter. It should only be hit once per type id, fill the Map, and then never be touched again.

Alternatively I could be mistaken and it actually is about contested get(): if so, then ConcurrentHashMap would be the logical next step.
Even if computeIfAbsent() can't yet be used (with 2.x, anyway -- and with 3.0 it might make sense to avoid to minimize block length), it would be more efficient with get/put pair too, I assume.

So I guess results from benchmark would be interesting. :)