Skip to content

improve performance of safeParse #81

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

Merged
merged 4 commits into from
Jul 29, 2022
Merged

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Jul 29, 2022

before:
JSON.parse x 1,711,391 ops/sec ±0.86% (87 runs sampled)
JSON.parse proto x 1,077,100 ops/sec ±1.18% (90 runs sampled)
secure-json-parse parse x 1,430,832 ops/sec ±1.00% (91 runs sampled)
secure-json-parse parse proto x 1,576,912 ops/sec ±1.14% (84 runs sampled)
secure-json-parse safeParse x 1,420,410 ops/sec ±1.06% (87 runs sampled)
secure-json-parse safeParse proto x 139,151 ops/sec ±0.97% (90 runs sampled)
JSON.parse reviver x 497,298 ops/sec ±0.83% (90 runs sampled)

after:
JSON.parse x 1,738,271 ops/sec ±0.84% (88 runs sampled)
JSON.parse proto x 1,100,147 ops/sec ±1.23% (89 runs sampled)
secure-json-parse parse x 1,441,809 ops/sec ±0.79% (91 runs sampled)
secure-json-parse parse proto x 1,592,567 ops/sec ±0.82% (82 runs sampled)
secure-json-parse safeParse x 1,405,980 ops/sec ±1.30% (90 runs sampled)
secure-json-parse safeParse proto x 917,157 ops/sec ±0.81% (92 runs sampled)
JSON.parse reviver x 501,752 ops/sec ±0.78% (88 runs sampled)

Checklist

@mcollina
Copy link
Member

linting is failing

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

Is this a semver-major change?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jul 29, 2022

@mcollina

Should not be backwards breaking. I just corrected the typings of scan.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 3938d11 into fastify:master Jul 29, 2022
@mcollina
Copy link
Member

Can you send a PR to fastify to use this instead? It should improve things in case of a failure.

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