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: add max depth limit and max edges limit #56

Merged
merged 1 commit into from Sep 7, 2021

Conversation

simoneb
Copy link
Contributor

@simoneb simoneb commented Sep 6, 2021

This supersedes #51 by updating the branch to integrate the latest master changes and undoing some unnecessary changes.

See original PR for implementation details

@mcollina
Copy link
Collaborator

mcollina commented Sep 7, 2021

Is there any performance regression in landing this?

@simoneb
Copy link
Contributor Author

simoneb commented Sep 7, 2021

Benchmark results look good to me ✔️

On this branch

simone@X86 MINGW64 /n/os/fast-safe-stringify (limit-option)
$ npm run benchmark

> fast-safe-stringify@2.0.8 benchmark N:\os\fast-safe-stringify
> node benchmark.js

util.inspect:          simple object                  x 207,141 ops/sec ±2.31% (89 runs sampled)
util.inspect:          circular                       x 88,762 ops/sec ±4.45% (83 runs sampled)
util.inspect:          circular getters               x 98,280 ops/sec ±1.05% (91 runs sampled)
util.inspect:          deep                           x 7,326 ops/sec ±1.23% (89 runs sampled)
util.inspect:          deep circular                  x 7,267 ops/sec ±1.44% (91 runs sampled)
util.inspect:          large deep circular getters    x 5,054 ops/sec ±2.82% (87 runs sampled)
util.inspect:          deep non-conf circular getters x 7,338 ops/sec ±1.05% (90 runs sampled)

json-stringify-safe:   simple object                  x 205,854 ops/sec ±2.88% (82 runs sampled)
json-stringify-safe:   circular                       x 104,300 ops/sec ±1.74% (92 runs sampled)
json-stringify-safe:   circular getters               x 102,861 ops/sec ±3.75% (90 runs sampled)
json-stringify-safe:   deep                           x 8,423 ops/sec ±0.99% (91 runs sampled)
json-stringify-safe:   deep circular                  x 8,028 ops/sec ±2.18% (89 runs sampled)
json-stringify-safe:   large deep circular getters    x 293 ops/sec ±2.83% (81 runs sampled)
json-stringify-safe:   deep non-conf circular getters x 8,027 ops/sec ±0.85% (91 runs sampled)

fast-safe-stringify:   simple object                  x 1,042,603 ops/sec ±2.35% (87 runs sampled)
fast-safe-stringify:   circular                       x 434,144 ops/sec ±0.98% (86 runs sampled)
fast-safe-stringify:   circular getters               x 433,646 ops/sec ±1.25% (89 runs sampled)
fast-safe-stringify:   deep                           x 23,648 ops/sec ±3.62% (88 runs sampled)
fast-safe-stringify:   deep circular                  x 24,564 ops/sec ±1.18% (90 runs sampled)
fast-safe-stringify:   large deep circular getters    x 1,047 ops/sec ±1.35% (91 runs sampled)
fast-safe-stringify:   deep non-conf circular getters x 11,314 ops/sec ±1.20% (85 runs sampled)

Fastest is
fast-safe-stringify:   simple object

On master

simone@X86 MINGW64 /n/os/fast-safe-stringify (master)
$ npm run benchmark

> fast-safe-stringify@2.0.8 benchmark N:\os\fast-safe-stringify
> node benchmark.js

util.inspect:          simple object                  x 208,739 ops/sec ±1.87% (88 runs sampled)
util.inspect:          circular                       x 91,703 ops/sec ±2.86% (85 runs sampled)
util.inspect:          circular getters               x 88,827 ops/sec ±4.73% (81 runs sampled)
util.inspect:          deep                           x 7,226 ops/sec ±1.69% (88 runs sampled)
util.inspect:          deep circular                  x 7,072 ops/sec ±1.27% (91 runs sampled)
util.inspect:          large deep circular getters    x 5,091 ops/sec ±1.57% (87 runs sampled)
util.inspect:          deep non-conf circular getters x 7,299 ops/sec ±1.39% (89 runs sampled)

json-stringify-safe:   simple object                  x 218,630 ops/sec ±1.53% (90 runs sampled)
json-stringify-safe:   circular                       x 92,882 ops/sec ±2.86% (80 runs sampled)
json-stringify-safe:   circular getters               x 94,241 ops/sec ±2.27% (83 runs sampled)
json-stringify-safe:   deep                           x 5,024 ops/sec ±8.40% (60 runs sampled)
json-stringify-safe:   deep circular                  x 6,733 ops/sec ±4.80% (81 runs sampled)
json-stringify-safe:   large deep circular getters    x 280 ops/sec ±2.73% (77 runs sampled)
json-stringify-safe:   deep non-conf circular getters x 5,987 ops/sec ±4.08% (71 runs sampled)

fast-safe-stringify:   simple object                  x 848,946 ops/sec ±3.60% (74 runs sampled)
fast-safe-stringify:   circular                       x 320,895 ops/sec ±4.62% (72 runs sampled)
fast-safe-stringify:   circular getters               x 390,152 ops/sec ±3.34% (81 runs sampled)
fast-safe-stringify:   deep                           x 21,731 ops/sec ±2.64% (81 runs sampled)
fast-safe-stringify:   deep circular                  x 18,851 ops/sec ±4.60% (73 runs sampled)
fast-safe-stringify:   large deep circular getters    x 640 ops/sec ±5.48% (67 runs sampled)
fast-safe-stringify:   deep non-conf circular getters x 9,186 ops/sec ±4.92% (77 runs sampled)

Fastest is
fast-safe-stringify:   simple object

Copy link
Collaborator

@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 ce7db51 into davidmarkclements:master Sep 7, 2021
@simoneb simoneb deleted the limit-option branch September 7, 2021 11:47
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.

None yet

3 participants