Skip to content

Add comprehensive benchmarking suite#527

Draft
dvmhmdsd wants to merge 14 commits into
ljharb:mainfrom
dvmhmdsd:main
Draft

Add comprehensive benchmarking suite#527
dvmhmdsd wants to merge 14 commits into
ljharb:mainfrom
dvmhmdsd:main

Conversation

@dvmhmdsd

Copy link
Copy Markdown

Motivation

As discussed here #526 for prformance optimiztions, that there should be a benchmarking infrastructure:

  • Run before and after changes to measure impact
    • Execute on every Node.js version in the test matrix
    • Prevent future performance regressions
    • Provide data-driven evidence for performance claims

@socket-security

socket-security Bot commented Jun 20, 2025

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedobject.fromentries@​2.0.8671008151100
Addedobject.entries@​1.1.9671008552100
Addedstring.prototype.repeat@​1.0.0671007952100
Addedarray.from@​1.1.6571009353100
Addedarray-includes@​3.1.96610010055100
Addedcolors@​=1.4.010010010077100
Addedbenchmark@​2.1.41001009879100
Addedes6-shim@​0.35.810010010079100

View full report

@ljharb ljharb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

the root package.json's publishConfig.ignore will need "benchmark" added in package.json as well.

Also, can we add a GHA workflow that runs these on every push/PR, and fails if there's a big enough slowdown?

Comment thread benchmark/package.json Outdated
Comment thread benchmark/README.md Outdated
@dvmhmdsd

Copy link
Copy Markdown
Author

I added most of the comments, will find a way for the GHA @ljharb

@ljharb ljharb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this seems to be only running benchmarks in node 20.

Comment thread .github/workflows/benchmark.yml Outdated
Comment thread .github/workflows/benchmark.yml Outdated
@dvmhmdsd

Copy link
Copy Markdown
Author

this seems to be only running benchmarks in node 20.

what versions should it run on ?

@ljharb

ljharb commented Jun 21, 2025

Copy link
Copy Markdown
Owner

Execute on every Node.js version in the test matrix

all of them

@dvmhmdsd

dvmhmdsd commented Jun 22, 2025

Copy link
Copy Markdown
Author

Currently, the benchmarking scripts use some modern features (e.g., Array.from, async/await) and rely on tools that are not compatible with 0.8 like Benchmark.js which I use for benchmarking, which only supports node 0.10. So, is compatibility with Node 0.8 for the benchmarking infrastructure is a hard requirement ? @ljharb

@ljharb

ljharb commented Jun 22, 2025

Copy link
Copy Markdown
Owner

I can definitely live with only going down to 0.10. https://npmjs.com/array.from should cover that one, and async/await can be done with Promises instead, which are polyfillable.

Comment thread .github/workflows/benchmark.yml Outdated
dvmhmdsd added 8 commits June 25, 2025 12:55
- Implemented parsing benchmarks for various query string formats including simple, nested, and array queries.
- Added stringifying benchmarks for different object structures and array formats.
- Created a runner to manage benchmark execution and results reporting.
- Introduced fixtures for generating test data patterns for benchmarking.
- Added functionality to compare current results against baseline benchmarks.
- Implemented options for saving current results as baseline and generating summary reports.
- Enhanced command-line interface for running benchmarks with various options.
Comment thread benchmark/polyfills.js Outdated
Comment thread benchmark/polyfills.js Outdated
Comment thread benchmark/polyfills.js Outdated
Comment thread benchmark/polyfills.js Outdated
Comment thread benchmark/polyfills.js Outdated
Comment thread benchmark/runner.js Outdated
Comment thread benchmark/runner.js Outdated
Comment thread benchmark/stringify.js Outdated
Comment thread package.json Outdated
Comment thread package.json Outdated
- Deleted outdated benchmark result files to clean up the repository.
- Updated the `generateQuery` function in fixtures to handle default size parameter more gracefully.
- Added new dependencies for polyfills to support ES6+ features in older Node.js versions.
- Removed custom polyfills for Array.from, String.prototype.repeat, Object.fromEntries, and Object.entries, replacing them with well-maintained npm packages.
- Updated benchmark runner to improve code readability and maintainability.
- Adjusted benchmark commands in package.json for consistency and clarity.
- Added new benchmark result file with updated performance metrics for various parsing scenarios.
Comment thread package.json Outdated
Comment thread README.md Outdated
@dvmhmdsd

Copy link
Copy Markdown
Author

Hi @ljharb
Just following up on this PR — I’ve addressed all the comments and happy to make any further changes if needed. Please let me know if there's anything else blocking it from being merged. Thanks.

@ljharb

ljharb commented Apr 27, 2026

Copy link
Copy Markdown
Owner

Sorry for the long delay. If you could rebase this and resolve the conflicts, I can give it a new review.

@ljharb ljharb marked this pull request as draft April 27, 2026 18:31
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