Cache formatter with offset parsed #120
Conversation
| @@ -59,6 +62,7 @@ public JacksonJodaDateFormat(JacksonJodaDateFormat base, | |||
| { | |||
| super(base, useTimestamp); | |||
| _formatter = base._formatter; | |||
| _formatterWithOffsetParsed = _formatter.withOffsetParsed(); | |||
There was a problem hiding this comment.
This probably could just be base._formatterWithOffsetParsed?
There was a problem hiding this comment.
The reason for this vanilla copy & paste is just in case _formatter is changed to something else in the future, so that you don't have to also maintain the value at _formatterWithOffsetParsed
The idea is, whatever is assigned to _formatter, create a with offset parsed version of exactly that.
Do let me know if the reason I stated justifies it, if you still don't agree I will change it to what you have suggested.
There was a problem hiding this comment.
I don't have strong feelings here -- seems like avoiding another operation would be nice, but then again this is not in the hot codepath (as (de)serializer that uses it is cached). So I'll trust your judgment with it.
| @@ -104,6 +111,7 @@ protected JacksonJodaDateFormat(JacksonJodaDateFormat base, | |||
| { | |||
| super(base); | |||
| _formatter = base._formatter; | |||
| _formatterWithOffsetParsed = _formatter.withOffsetParsed(); | |||
There was a problem hiding this comment.
Same reason as my previous comment.
|
Sounds good, thank you for PR! One small request: could you rebase this against either 2.11 or 2.12? |
guidomedina
force-pushed
the
cache-formatter-with-offset-parsed
branch
from
4d94a11
to
5f89426
Compare
|
I have rebased and change the base to |
|
Looks good, happy to merge. Just one thing: I am not 100% sure if I had asked for this before, but if not, a CLA would be needed before the first contribution (but just once so if you did send one at an earlier point that'd be fine -- same in future for other contributions). Document is here: https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf and the usual way is to print it, fill & sign, scan/photo, email to Looking forward to merging this in 2.12, thank you for the contribution! |
Cache formatter with offset parsed which can create lots of allocation when de-serialising Json objects with many date/time when the time/zone is preserved.
We have observed this is big GC hotspot, the solution is simple; to cache it, when no locale is used and the feature
DeserializationFeature.ADJUST_DATES_TO_CONTEXT_TIME_ZONEis disabled a new formatter is always instantiated.