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

Add support in TokenBuffer for lazily decoded (big) numbers #3730

Closed
cowtowncoder opened this issue Jan 14, 2023 · 5 comments
Closed

Add support in TokenBuffer for lazily decoded (big) numbers #3730

cowtowncoder opened this issue Jan 14, 2023 · 5 comments
Milestone

Comments

@cowtowncoder
Copy link
Member

(note: for background see #3721)

There are a few issues with TokenBuffer regarding number coercions, mostly due to eager buffering sometimes needed, which forces decision on use of type (double vs BigDecimal in particular) before knowing target type.
There are also concerns about overhead in some cases, preventing efficient skipping of big number values in cases where buffering is needed: for example, when @JsonCreator use requires preservation of content that would ultimately be skipped.

An idea that came to mind is that since TokenBuffer stores all buffered content using 2 elements -- JsonToken for type, and opaque Object (depending on type) -- it would be possible to augment handling to use hypothetical BufferedNumber (or LazyNumber) type, which could:

  1. Store more information on actual NumberType (to preserve distinction between finer grain JsonParser exposes)
  2. Allow deferred decoding esp. for "big" numbers (BigInteger, all floating-point types)

This would require support from jackson-core for:

  • New type itself (BufferedNumber) as well as implementation(s) if not concrete
  • New accessor(s) from JsonParser -- either specific "always get buffered" or hybrid (buffered if not decoded; Number if decoded)

There are probably other subtleties too but this is the general idea. If anyone plans to work on this, another issue needs to be created on jackson-core side.

@cowtowncoder cowtowncoder added the to-evaluate Issue that has been received but not yet evaluated label Jan 14, 2023
@pjfanning
Copy link
Member

pjfanning commented Jan 14, 2023

@cowtowncoder I added lazy nums in #3727

In the test case that shows the unnecessary parseBigInteger call now doesn't end up parsing the big int - the LazyBigInteger is stored but the number is never parsed.

Unfortunately, not many existing tests lead to lazy nums being actually parsed - so I can't yet guarantee that the lazy code actually works all the way through.

@cowtowncoder cowtowncoder removed the to-evaluate Issue that has been received but not yet evaluated label Jan 14, 2023
@cowtowncoder
Copy link
Member Author

cowtowncoder commented Jan 28, 2023

Another potential mechanism to use: FasterXML/jackson-core#902 (assuming I get to implement it).

EDIT: Ok it was easier than I thought; getNumberValueDeferred() PR is here:

https://github.com/FasterXML/jackson-core/pull/903

ready for review.

@cowtowncoder
Copy link
Member Author

Ok, instead I went with:

FasterXML/jackson-core#902

with adds JsonParser.getNumberValueDeferred(). I hope to work on this soon, but @pjfanning if you have time and interest don't let me block your progress.

One thing I need to figure out has to do with testing, how to verify expected handling improvements both wrt skipping of unused contents (need mocking, maybe?) and wrt retaining of exact representation through buffering.

@pjfanning
Copy link
Member

@cowtowncoder #3761 is open

@cowtowncoder cowtowncoder added this to the 2.15.0 milestone Jan 31, 2023
@cowtowncoder
Copy link
Member Author

Merged #3761 so this is now implemented.

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