search

michalmuskala / jason

A blazing fast JSON parser and generator in pure Elixir.
Other
1.62k stars 169 forks source link

Support :decimals mode #73

Closed kamenlitchev closed 5 years ago

kamenlitchev commented 5 years ago

Introduce :decimals decoding option. Allows to produce decoded maps where integers/floats are represented with a Decimal struct. Supported options are:

Implementation notes: I've selected to use function argument matching to get the desired result where I had to propagate one argument (num_decode_mode) throughout the code. The other approach was to propagate two functions (float and integer parsing), but I simply liked the first approach more.

On performance impact, I am not sure how much of a change there really is. My dev machine was all over the place for both original and new implementation.

kamenlitchev commented 5 years ago

I ran the old and the new implementations two times each on my PC (all apps closed, nothing else active). Iterations per second are relatively consistent (at least from my non-statistical point of view :) ). Linux-on-Windows (Ubuntu 18.04.02 LTS), Intel i7-8550U, 32GB RAM.

Results in Iterations per second for mix bench.decode of old vs new implementation (with no decimals: decoding option set - meaning decimals: :none):

  Old (Run 1) Old (Run 2) New (Run 1) New (Run 2)
Blockchain 2.75 K 2.64 K 2.72 K 2,71 K
Giphy 240.01 270.34 273.07 256.99
GitHub 891.46 906.41 915.05 906.22
GovTrack 8.40 8.37 8.38 8.24
Issue 90 8.99 8.84 9.03 9.04
JSON Generator 347.59 339.79 346.78 342.33
Json Generator (Pretty) 289.00 280.96 287.36 284.11
Pokedex 538.70 507.46 534.43 516.36
UTF-8 escaped 1.02 K 1.03 K 1.02 K 1.02 K
UTF-8 unescaped 5.97 K 5.98 K 5.95 K 5.99 K

Attached are the full results: Decimal_Performance.zip

kamenlitchev commented 5 years ago

BTW, this is also related to Issue #71 - with this pull request Jason can be one of the parsers that does not break on receiving unquoted float-incompatible numbers :)

kamenlitchev commented 5 years ago

Regarding the :decimals option parameters themselves - :all, :none - these are all to my liking.

Initially I coded to allow :integers, too. But then I thought of no use case when one would need Decimal precision on integers only.

However, I am not really sure that :floats is needed, too. Specifically, if one has that flag set this means that any value within the JSON can be either integer or Decimal, thus rendering potential arithmetic operations to always take value data type in mind.

With all that said, maybe a simple decimals :all | :none approach is better? Or even decimals: :true | :false? Providing those two flags only will simplify the code by removing the guard clauses and will protect people from bugs (when they use decimals: :floats and test with integers, but a real world client sends them a float instead and their math logic breaks).

kamenlitchev commented 5 years ago

Since there is no interest into accepting the commit or feedback if something is wrong with it, I will be closing it

conradfr commented 3 years ago

FWIW I would have loved to have that option, sad it didn't make it.