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

Concurrency issue with DeserializerCache (due to Map deserializers not getting cached) #604

Closed
ShawnTuatara opened this issue Nov 7, 2014 · 6 comments
Milestone

Comments

@ShawnTuatara
Copy link

Running some load tests on a Spring Boot based service whereby I am trying to reach 2000 requests per second. The service was responding fine up to 1000 rps but now trying to push 2000 is causing issues. Reviewing the code path with AppDynamics the thread is getting stuck in the DeserializerCache._createAndCacheValueDeserializer() method.

jackson-synchronization-issue

I have multiple stack traces that show the same method having over 200ms execution time on that method. Reviewing some of the old (2012) mailing lists posts shows that there was some issues around this but nothing concrete to fix / get around the issue. This is using the latest 2.4.3 version of jackson-databind.

Anyone able to provide recommendations on how to resolve this issue?

@cowtowncoder
Copy link
Member

There are no known open issues regarding concurrency or performance of DeserializerCache that I am aware of.

But looking at line in question, I suspect this is not a problem with Jackson as much as problem with caller somehow ending up not getting a cached instance. Two scenarios would be:

  1. ObjectMapper is not being reused, but re-created for new requests (which is a big performance problem on its own)
  2. Construction of deserializer somehow throwing an exception

Otherwise, code should not get into this part of DeserializerCache. So question is not so much hot spot you see, but rather that why calls would end up getting there, not finding deserializer.
And the very first thing is to make 100% sure that ObjectMappers are being reused. If not, I wouldn't be surprised to see the hot spot.

Another question is as to why "canDeserialize()" is being called at all -- it is often unnecessary, and it'd be better to simply call readValue(). There are few cases where "canDeserialize()" would probide actual information on failing cases. But if it is called, it makes sense to call variant that takes two arguments; second being AtomicReference that will be set to Throwable that caused failure.
If ObjectMapper is properly reused, I would check to see if an exception is causing failure of deserializer construction.

@ShawnTuatara
Copy link
Author

Cross posted to the spring-boot project as that is the component that is creating the ObjectMapper and objects that call the ObjectMapper. Hopefully they can provide some insight into the questions you are asking.

@cowtowncoder
Copy link
Member

Thanks! Historically there have been a few cases where high contention has led to finding other root causes, so this is not totally unusual finding. So I hope it turns out to be something simple.

I also think that later versions of Spring started using canDeserialize() variant that obtains underlying Exception, to give more information on failure. So if you are not using the latest version, make sure to upgrade and/or check list of fixes in later versions.

@ShawnTuatara
Copy link
Author

Looks like we resolved the issue in the other thread. It was because I was using a Map and that deserializer isn't cached so it kept calling into that method to check. Now I am curious why the Map deserializer doesn't get cached.

@cowtowncoder
Copy link
Member

Hmmh. Could be historical thing; assumption being that introspection needed for POJOs is expensive, but one for Maps less so. Although over time, cost of handling Map types has increased a lot too, so is probably incorrect assumption.

Sounds like I should check into caching aspects to ensure that these deserializers (and ones for Collections too) will get cached as well.

@cowtowncoder cowtowncoder changed the title Concurrency issue with DeserializerCache Concurrency issue with DeserializerCache (due to Map deserializers not getting cached) Nov 8, 2014
@cowtowncoder cowtowncoder added this to the 2.4.4 milestone Nov 8, 2014
@cowtowncoder
Copy link
Member

Ok: I changed 2.4 branch so that 2.4.4 should be caching Map deserializers. For 2.5 I will probably change baseline so that more deserializers (at least all collection types) are cached.

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