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

Reduce floating point errors in decoding #66

Merged
merged 1 commit into from
Oct 18, 2021

Conversation

kusano
Copy link
Contributor

@kusano kusano commented Oct 17, 2021

Before:

1> Json = jsone:encode(123.0).
<<"1.23000000000000000000e+02">>
2> jsone:decode(Json).
123.00000000000001

After:

1> Json = jsone:encode(123.0).
<<"1.23000000000000000000e+02">>
2> jsone:decode(Json).
123.0

Although floating point numbers may have errors, numbers which can be represented without errors should be treated without errors. This patch reduce floating point errors by avoiding the use of decimals.

1> 123000000000000000000.0 * 1.0e-18.
123.00000000000001
2> 123000000000000000000.0 / 1.0e+18.
123.0

In spite of that 123000000000000000000 > 253, 123000000000000000000 can be exactly represented in IEEE 754 format since it can be divided by 2 many times.

My concern is performance. Division may be slower than multiplication.

My coworker wrote this patch and he has agreed to post it here.

@sile
Copy link
Owner

sile commented Oct 18, 2021

Thanks for your PR! LGTM.

For your performance concern, I tried running a simple benchmark as follows:

% Common Setup
> Input = [123.0 || X<-lists:seq(1, 100000)].
> Json = jsone:encode(Input).

% `master` branch
> timer:tc(fun jsone:decode/1, [Json]).
{66039, % 66ms
 [123.00000000000001,123.00000000000001,123.00000000000001,
  123.00000000000001,123.00000000000001,123.00000000000001,
  123.00000000000001,123.00000000000001,123.00000000000001,
  123.00000000000001,123.00000000000001,123.00000000000001,
  123.00000000000001,123.00000000000001,123.00000000000001,
  123.00000000000001,123.00000000000001,123.00000000000001,
  123.00000000000001,123.00000000000001,123.00000000000001,
  123.00000000000001,123.00000000000001,123.00000000000001,
  123.00000000000001,123.00000000000001,123.00000000000001|...]}

% PR branch
18> timer:tc(fun jsone:decode/1, [Json]).
{65655, % 65ms
 [123.0,123.0,123.0,123.0,123.0,123.0,123.0,123.0,123.0,
  123.0,123.0,123.0,123.0,123.0,123.0,123.0,123.0,123.0,123.0,
  123.0,123.0,123.0,123.0,123.0,123.0,123.0,123.0|...]}

There are no significant performance changes. So I think it's okay to merge this PR.

@sile sile merged commit 6ba5456 into sile:master Oct 18, 2021
@sile
Copy link
Owner

sile commented Oct 18, 2021

Note that I'll release the next version including this patch within this month.

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

Successfully merging this pull request may close these issues.

2 participants