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

Performance optimizations #33

Merged
merged 1 commit into from Jan 12, 2023
Merged

Performance optimizations #33

merged 1 commit into from Jan 12, 2023

Conversation

Cadienvan
Copy link
Contributor

Did some small, yet significant performance improvements, explained below:

  • Using a Buffer.concat and generally reducing the amount of operations made by the baseId function.
  • Used a bitwise operator for the generate -> count variable, as they are generally faster than arithmetic ones.
  • Removed the pad function as its only usage can be converted, as did, to a sliced string.
  • As the '==' padding used in the baseId function is always the same, I brought it out of the function scope preventing a redundant operation being done every time.

All tests seem to pass and benchmarks provide a +1/+5% benefit.

Before updates, on a Macbook Pro 2021 (M1 Max, 32GB RAM):

hyperid - variable length x 14,254,472 ops/sec ±0.62% (88 runs sampled)
hyperid - fixed length x 14,098,171 ops/sec ±0.69% (90 runs sampled)
hyperid - fixed length, url safe x 14,382,843 ops/sec ±0.75% (91 runs sampled)

After updates:

hyperid - variable length x 14,481,182 ops/sec ±0.52% (84 runs sampled)
hyperid - fixed length x 14,268,481 ops/sec ±0.57% (87 runs sampled)
hyperid - fixed length, url safe x 14,797,637 ops/sec ±0.62% (92 runs sampled)

I reached 15M ops/sec in a couple of benchmarks, yet those results are the medians.

…ving pad, storing base64 redunant padding result
Copy link
Owner

@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, good work

@Cadienvan
Copy link
Contributor Author

Hi @mcollina , just to be sure I didn't miss anything, PR is still open, you still waiting something from me or just didn't have time to pull in?

@mcollina mcollina merged commit ec22c2e into mcollina:master Jan 12, 2023
@mcollina
Copy link
Owner

Shipped v3.1.0

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

2 participants