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

Optimize hexWrite with an array lookup table #343

Merged
merged 7 commits into from
Feb 1, 2024

Conversation

chjj
Copy link
Contributor

@chjj chjj commented Jan 23, 2024

Should fix the slowdown introduced by the hash table lookups in #303

My own benchmarks seem to indicate a 10x speedup.

@chjj
Copy link
Contributor Author

chjj commented Jan 23, 2024

Wrote a proper benchmark. Here are the numbers:

Without this PR:

$ node perf/write-hex.js
BrowserBuffer#write(4096, "hex") x 5,697 ops/sec ±0.29% (91 runs sampled)
NodeBuffer#write(4096, "hex") x 105,578 ops/sec ±0.04% (90 runs sampled)
Fastest is NodeBuffer#write(4096, "hex")

With this PR:

$ node perf/write-hex.js
BrowserBuffer#write(4096, "hex") x 34,100 ops/sec ±0.07% (93 runs sampled)
NodeBuffer#write(4096, "hex") x 105,699 ops/sec ±0.04% (92 runs sampled)
Fastest is NodeBuffer#write(4096, "hex")

Though, on my local machine I was getting an even larger speedup.

@dcousens dcousens self-assigned this Jan 23, 2024
index.js Show resolved Hide resolved
index.js Show resolved Hide resolved
Copy link
Collaborator

@dcousens dcousens left a comment

Choose a reason for hiding this comment

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

LGTM

@chjj
Copy link
Contributor Author

chjj commented Feb 1, 2024

I added more cases to the test. It's a bit spammy now. Let me know if you want me to reduce that.

@dcousens dcousens merged commit 54885d4 into feross:master Feb 1, 2024
6 checks passed
@dcousens
Copy link
Collaborator

dcousens commented Feb 1, 2024

Thanks for your work @chjj

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