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

fix: jv_number_value should cache the double value of literal numbers #3245

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

itchyny
Copy link
Contributor

@itchyny itchyny commented Feb 2, 2025

The code of jv_number_value is intended to cache the double value of
literal numbers, but it does not work because the function accepts the
jv struct by value. This patch fixes the behavior by checking if the
double value is NaN, which is the initial value. This patch apparently
does not cache the value if the literal is NaN, but improves the
performance of major use cases; e.g. range(1000000) runs 25% faster.

The code of `jv_number_value` is intended to cache the double value of
literal numbers, but it does not work because the function accepts the
`jv` struct by value. This patch fixes the behavior by checking if the
double value is `NaN`, which is the initial value. This patch apparently
does not cache the value if the literal is `NaN`, but improves the
performance of major use cases; e.g. `range(1000000)` runs 25% faster.
Copy link
Member

@wader wader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe comment to future code reads about NaN being used as uncached value?

@itchyny itchyny added this to the 1.8 release milestone Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants