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: consistently handle string in array #666

Merged
merged 2 commits into from
Dec 26, 2023
Merged

Conversation

qwelias
Copy link
Contributor

@qwelias qwelias commented Dec 8, 2023

Closes #662

Checklist

Notes

  • changed buildTest to compare parsed-FJS vs parsed-JSON because some things (like Date, and in general anything with .toJSON) cannot be parsed back after stringification
  • removed !(${input} instanceof Date check beacuse earlier we already do || instanceof Date
  • toString check is giga questionable, real JSON.stringify behavior ignores toString and instead uses toJSON, see:
    • MDN
    • try in REPL: JSON.stringify({toString:() => 'json'}) vs JSON.stringify({toJSON:() => 'json'})

@ivan-tymoshenko ivan-tymoshenko self-requested a review December 8, 2023 23:32
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

@Uzlopak
Copy link
Contributor

Uzlopak commented Dec 9, 2023

How are the benchmarks?

@qwelias
Copy link
Contributor Author

qwelias commented Dec 11, 2023

On my machine

the branch:

FJS creation x 7,971 ops/sec ±2.16% (90 runs sampled)
CJS creation x 222,214 ops/sec ±0.26% (95 runs sampled)
AJV Serialize creation x 99,653,679 ops/sec ±0.15% (97 runs sampled)
JSON.stringify array x 6,123 ops/sec ±0.32% (94 runs sampled)
fast-json-stringify array default x 9,499 ops/sec ±0.18% (97 runs sampled)
fast-json-stringify array json-stringify x 10,286 ops/sec ±0.38% (96 runs sampled)
compile-json-stringify array x 10,646 ops/sec ±0.58% (94 runs sampled)
AJV Serialize array x 10,249 ops/sec ±0.22% (95 runs sampled)
JSON.stringify large array x 287 ops/sec ±0.51% (90 runs sampled)
fast-json-stringify large array default x 154 ops/sec ±0.49% (44 runs sampled)
fast-json-stringify large array json-stringify x 298 ops/sec ±0.32% (89 runs sampled)
compile-json-stringify large array x 500 ops/sec ±0.12% (84 runs sampled)
AJV Serialize large array x 180 ops/sec ±0.20% (83 runs sampled)
JSON.stringify long string x 11,912 ops/sec ±0.82% (87 runs sampled)
fast-json-stringify long string x 12,163 ops/sec ±0.78% (90 runs sampled)
compile-json-stringify long string x 11,692 ops/sec ±0.78% (87 runs sampled)
AJV Serialize long string x 29,725 ops/sec ±0.30% (96 runs sampled)
JSON.stringify short string x 14,207,086 ops/sec ±0.67% (96 runs sampled)
fast-json-stringify short string x 49,478,443 ops/sec ±0.24% (96 runs sampled)
compile-json-stringify short string x 40,336,032 ops/sec ±2.34% (82 runs sampled)
AJV Serialize short string x 39,007,070 ops/sec ±0.60% (92 runs sampled)
JSON.stringify obj x 3,775,135 ops/sec ±0.90% (94 runs sampled)
fast-json-stringify obj x 14,007,933 ops/sec ±0.10% (94 runs sampled)
compile-json-stringify obj x 25,083,170 ops/sec ±1.34% (93 runs sampled)
AJV Serialize obj x 14,928,476 ops/sec ±0.46% (93 runs sampled)
JSON stringify date x 1,107,942 ops/sec ±0.45% (95 runs sampled)
fast-json-stringify date format x 1,653,662 ops/sec ±0.32% (96 runs sampled)
compile-json-stringify date format x 1,106,712 ops/sec ±0.48% (94 runs sampled)

base commit:

FJS creation x 8,502 ops/sec ±1.95% (87 runs sampled)
CJS creation x 219,254 ops/sec ±0.19% (98 runs sampled)
AJV Serialize creation x 99,928,845 ops/sec ±0.36% (96 runs sampled)
JSON.stringify array x 6,164 ops/sec ±0.17% (96 runs sampled)
fast-json-stringify array default x 9,777 ops/sec ±0.70% (94 runs sampled)
fast-json-stringify array json-stringify x 9,841 ops/sec ±0.31% (96 runs sampled)
compile-json-stringify array x 9,816 ops/sec ±0.35% (93 runs sampled)
AJV Serialize array x 9,107 ops/sec ±0.31% (95 runs sampled)
JSON.stringify large array x 300 ops/sec ±0.26% (89 runs sampled)
fast-json-stringify large array default x 155 ops/sec ±0.14% (87 runs sampled)
fast-json-stringify large array json-stringify x 299 ops/sec ±0.50% (89 runs sampled)
compile-json-stringify large array x 462 ops/sec ±0.25% (87 runs sampled)
AJV Serialize large array x 161 ops/sec ±0.69% (82 runs sampled)
JSON.stringify long string x 13,013 ops/sec ±0.33% (93 runs sampled)
fast-json-stringify long string x 13,204 ops/sec ±0.24% (96 runs sampled)
compile-json-stringify long string x 13,228 ops/sec ±0.12% (96 runs sampled)
AJV Serialize long string x 29,119 ops/sec ±0.32% (96 runs sampled)
JSON.stringify short string x 13,938,511 ops/sec ±0.08% (93 runs sampled)
fast-json-stringify short string x 48,387,569 ops/sec ±0.26% (93 runs sampled)
compile-json-stringify short string x 46,779,630 ops/sec ±0.22% (97 runs sampled)
AJV Serialize short string x 47,391,463 ops/sec ±0.60% (94 runs sampled)
JSON.stringify obj x 3,911,040 ops/sec ±0.23% (95 runs sampled)
fast-json-stringify obj x 13,328,137 ops/sec ±0.27% (93 runs sampled)
compile-json-stringify obj x 25,501,630 ops/sec ±0.25% (93 runs sampled)
AJV Serialize obj x 12,396,970 ops/sec ±0.71% (94 runs sampled)
JSON stringify date x 1,086,867 ops/sec ±0.16% (98 runs sampled)
fast-json-stringify date format x 1,673,385 ops/sec ±0.35% (92 runs sampled)
compile-json-stringify date format x 1,141,811 ops/sec ±0.18% (97 runs sampled)

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 c300641 into fastify:master Dec 26, 2023
19 checks passed
@qwelias qwelias deleted the date-tuple branch January 2, 2024 10:51
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.

Tuple of date-time does not allow Date
4 participants