-
Notifications
You must be signed in to change notification settings - Fork 72
Cache JsonProvider.provider() result as per javadoc recommendation #78
Conversation
rogersnm
commented
Apr 6, 2018
+1, I'm serialising a large object and it takes about 2000ms on my machine currently. Switching to this implementation drops it down to 50ms.
See jakartaee/jsonp-api#80 it seems there already is a similar PR in Jakarta EE. I doubt that this repository results in any new updates or releases.
@lukasj @bravehorsie Recently there were some similar PRs in different projects. I am not 100% convinced that it's needed. Folks, I want you to review it before letting it go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently this would actually decrease performance with default implementation if JsonProvider is cached by client application (as it is supposed to). See
When JsonProvider instance is cached like this buffer pool exists only once. Because it uses ConcurrentLinkedQueue it will decrease performance in multithreaded environment because of synchronization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I certainly see dangers making it final. That way for the lifetime of the Json class in a particular JVM it would remain the same even if the underlying provider may change (less likely without OSGi but possible for some implementations)
Keeping a static cache, maybe, as long as there's at least a protected or similar way to reset it, see how we did in JSR 363 with ServiceProvider or CDI.
I am not convinced we still need this in Java EE 8, maybe leave to a future (Jakarta EE) version of JSON-P?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comments as in jakartaee/jsonp-api#80 apply here
I got through this because I found a performance hostpot in calling Json methods (about 500 microsec per call on a i5)