Closed mdouglass closed 10 months ago
I was able to run git bisect
between 1.0.2 and HEAD of main using a modified version of my test.sh. The first failed commit is this one:
6c066b82708eb7e7ca0ca7f89737df48aa534a6c is the first bad commit
commit 6c066b82708eb7e7ca0ca7f89737df48aa534a6c
Author: Jordan Harband <ljharb@gmail.com>
Date: Fri Nov 10 22:10:43 2023 -0800
[Refactor] build up a string instead of an array + join
index.js | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
bisect found first bad commit
At a first glance that makes sense to me -- the creation of lots of temporary strings as it builds up the final JSON output vs the single-pass. For our use case, I'd lean towards reverting that, but I'm not sure what the motivator for the change was.
Interesting, that makes sense but I wouldn't have thought of it.
I assume you can confirm that v1.1.0, with that commit reverted, fixes the problem? If so, I'll put up a PR (that's not just a straight revert), and you can try that one before I merge and release it.
Yep, that seems to work.
Version | w/o --max-old-space-size | --max-old-space-size=2048 |
---|---|---|
1.0.2 | 4237, 4419, 4328 ms | 4363, 4429, 4414 ms |
1.1.0 | 10456, 10685, 11513 ms | OOM, OOM, OOM |
1.1.0 + revert | 3809, 3927, 3769 ms | 3884, 3752, 3885 ms |
awesome, even faster than v1.0.2 :-) I'll get a fix for this out tonight.
ty!
@mdouglass if you're able to test out #10, just to make absolutely sure it'll solve the problem, then I can merge and release it :-)
lgtm, thank you again :+1:
Version | w/o --max-old-space-size | --max-old-space-size=2048 |
---|---|---|
1.0.2 | 4237, 4419, 4328 ms | 4363, 4429, 4414 ms |
1.1.0 | 10456, 10685, 11513 ms | OOM, OOM, OOM |
1.1.0 + revert | 3809, 3927, 3769 ms | 3884, 3752, 3885 ms |
#10 | 3851, 3934, 3813 ms | 3884, 3785, 3686 |
We upgraded our project which deals with large JSON files (~250MiB) to 1.1.0. Using 1.1.0 we are seeing two performance-related regressions depending on whether we have a max-old-space-size setting in place.
node.js v20.7.0 json-stable-stringify 1.0.2 vs 1.1.0
index.mjs
test script
output of running test.sh
The JSON input file I used for this run is 267,437,008 bytes. I am not sure sure if you'll need it or if any sample file in this size range would be able to demonstrate the problem.