-
-
Notifications
You must be signed in to change notification settings - Fork 795
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 StreamReadFeature.USE_FAST_BIG_NUMBER_PARSER
to enable faster BigDecimal
, BigInteger
parsing
#851
Conversation
pjfanning
commented
Nov 30, 2022
•
edited
Loading
edited
- So far, this one feature also affects BigInteger parsing, but I could introduce a 2nd feature if that was preferred
- The fastdoubleparser Big Integer parser allows hex numbers but jackson-core's JSON parsers block the hex numbers anyway.
- I've got an open issue with fastdoubleparser team and will circle back when I know if they will address this
Quick note/question: would this actually use multiple processing threads? (wrt references to parallel processing) |
It uses fork-join tasks to do the parallel processing - I haven't delved too much into the details - maybe there is a ForkJoinPool to run the tasks. Edit: It uses the common ForkJoinPool - see wrandelshofer/FastDoubleParser#24 (comment) The parallel support is only enabled if you enable 2 separate features. |
Ok. I feel quite strongly that a library should not be controlling parallelism here so I think anything that uses thread pools, executors etc should be an extension (not part of core by default): otherwise this could cause issues with things like, say, async processing. So I definitely want to make sure to disable processing done using any other model than single main thread; because the alternative is that threading/parallelism aspects would need to be externally configurable. |
Ok - I can remove the feature to support parallelism |
@pjfanning Let's do that for now. Having code that would allow that is fine (esp. assuming it'd be hassle to remove it), and we could consider it as a later add-on. Maybe my thinking evolves here; for now there are some limits I like in libraries, like Jackson being pretty much single-threaded linear, no-surprises component (with minor deviation with use of |
@cowtowncoder is it ok to merge this? There is a minor issue in fastdoubleparser (see issue description) but I don't think this has a big impact. I'd like to try this change out in jackson-databind, etc. It's ages util the actual v2.15.0 release so I think it's ok to live with the potential that we will need to upgrade fastdoubleparser jar later. |
@pjfanning Ok I could merge this but have couple of minor things to change. Will add notes before -- was first thinking of merging, asking for change but with 2.x->3.0 merge it's probably better to get changes in at once. On hex numbers, that's I guess mostly not-a-problem as tokenization won't allow those: but would allow conversion from "Stringified" numbers. I guess I'm ok with that too. We have some other places where "from String" conversion allows some non-standard representations just out of convenience of using JDK parsing for Integer and Longs etc. |
StreamReadFeature.USE_FAST_BIG_DECIMAL_PARSER
to enable faster BigDecimal
, BigInteger
parsing
StreamReadFeature.USE_FAST_BIG_DECIMAL_PARSER
to enable faster BigDecimal
, BigInteger
parsingStreamReadFeature.USE_FAST_BIG_NUMBER_PARSER
to enable faster BigDecimal
, BigInteger
parsing