chore: optimize uuid.v4 by 4.3x (430%) (internal stringify by 33x)

[Rush]

It turns out that validation in uuidStringify() is slowing it down by 33x. Since uuidv1() and uuidv4() produce valid buffers by the virtue of their algorithm they can safely use a validation-free version of stringify.

Benchmark changes

uuidv1() - faster by 1.3x
uuidv4() - faster by 4.3x
the gains are due to .unsafeStringify() with no validation (as it is not required) is 33x faster than the version with validation.

The public uuidStringify() has been left unchanged but my recommendation would be to also remove validation from this method in the future and just export the unsafe version.

Benchmark (Core i9-10900K)

uuidStringify() x 2,957,961 ops/sec ±0.78% (86 runs sampled)
uuidParse() x 3,331,671 ops/sec ±0.59% (93 runs sampled)
---

uuidv1() x 3,185,198 ops/sec ±0.54% (91 runs sampled)
uuidv1() fill existing array x 9,987,064 ops/sec ±0.21% (95 runs sampled)
uuidv4() x 6,610,790 ops/sec ±0.77% (94 runs sampled)
uuidv4() fill existing array x 7,085,027 ops/sec ±0.68% (96 runs sampled)
uuidv3() x 385,764 ops/sec ±1.00% (89 runs sampled)
uuidv5() x 424,133 ops/sec ±1.43% (89 runs sampled)
Fastest is uuidv1() fill existing array

Benchmark prior to changes

Starting. Tests take ~1 minute to run ...
uuidStringify() x 2,901,140 ops/sec ±0.61% (93 runs sampled)
uuidParse() x 3,279,020 ops/sec ±0.38% (88 runs sampled)
---

uuidv1() x 2,328,848 ops/sec ±0.60% (90 runs sampled)
uuidv1() fill existing array x 9,944,353 ops/sec ±0.26% (93 runs sampled)
uuidv4() x 1,677,495 ops/sec ±0.35% (90 runs sampled)
uuidv4() fill existing array x 6,861,270 ops/sec ±0.36% (91 runs sampled)
Fastest is uuidv1() fill existing array

[Rush]

I left the version with validation as it may be useful for development. Please let me know if it should be removed.

[LinusU]

The new safeStringify no longer validates that the version is between 0-5, or that version 0 consists only of zeros, as the previous stringify does.

How about keeping the public stringify as it was before, but have an internal unsafeStringify that we use internally from v1/v35/v4? Since there we know that the bytes are valid.

That way this won't be a breaking change either, so it can go out in a regular release.

[Rush]

All right, but I still find the check's usefulness questionable. It is not a very strict check, it's just checking appearances. A more proper test would validate the input buffer for proper length and version even prior to stringifying.

[Rush]

Updated PR according to your suggestions. What's maybe is a missed opportunity is not exposing the validation-free stringify function. Any idea what are the use cases for users using this public .stringify() method?

[Rush]

Is there anything else I should change?

[Rush]

Addressed. Thank you for the review.

[LinusU]

Neat! 👍

[broofa]

@ctavan Do you know why the bundlewatch check isn't running here?

[ctavan]

@ctavan Do you know why the bundlewatch check isn't running here?

The browser tests and bundlewatch both use API keys (for Browserstack and bundlewatch respectively) and therefore don't run on PRs created from branches of forks. They only run on branches of this very repo.

The reason is that it would otherwise be trivial to extract those secrets (just console.log(process.env.API_KEY)).

I haven't checked in a while if there is a solution for this problem by now. But until then we can also just merge and observe the checks on main.

[LinusU]

Bundlewatch report for v8.3.2-14-g577c840 (this PR):

PASS ./examples/browser-rollup/dist/v1-size.js: 802B < 1KB (gzip)
PASS ./examples/browser-rollup/dist/v3-size.js: 1.96KB < 2.1KB (gzip)
PASS ./examples/browser-rollup/dist/v4-size.js: 522B < 716B (gzip)
PASS ./examples/browser-rollup/dist/v5-size.js: 1.36KB < 1.5KB (gzip)
PASS ./examples/browser-webpack/dist/v1-size.js: 859B < 1KB (gzip)
PASS ./examples/browser-webpack/dist/v3-size.js: 1.96KB < 2.1KB (gzip)
PASS ./examples/browser-webpack/dist/v4-size.js: 552B < 716B (gzip)
PASS ./examples/browser-webpack/dist/v5-size.js: 1.35KB < 1.5KB (gzip)

Bundlewatch report for v8.3.2-12-gc9e076c (main):

PASS ./examples/browser-rollup/dist/v1-size.js: 912B < 1KB (gzip)
PASS ./examples/browser-rollup/dist/v3-size.js: 1.99KB < 2.1KB (gzip)
PASS ./examples/browser-rollup/dist/v4-size.js: 624B < 716B (gzip)
PASS ./examples/browser-rollup/dist/v5-size.js: 1.39KB < 1.5KB (gzip)
PASS ./examples/browser-webpack/dist/v1-size.js: 905B < 1KB (gzip)
PASS ./examples/browser-webpack/dist/v3-size.js: 1.99KB < 2.1KB (gzip)
PASS ./examples/browser-webpack/dist/v4-size.js: 589B < 716B (gzip)
PASS ./examples/browser-webpack/dist/v5-size.js: 1.39KB < 1.5KB (gzip)

Summary:

	v8.3.2-12-gc9e076c	v8.3.2-14-g577c840
rollup/v1-size.js	912B	802B
rollup/v3-size.js	1.99KB	1.96KB
rollup/v4-size.js	624B	522B
rollup/v5-size.js	1.39KB	1.36KB
webpack/v1-size.js	905B	859B
webpack/v3-size.js	1.99KB	1.96KB
webpack/v4-size.js	589B	552B
webpack/v5-size.js	1.39KB	1.35KB

Lower bundle sizes overall 🙌

[ctavan]

@broofa are you fine with pulling this in now?

[broofa]

@ctavan

[ctavan]

Thank you for this great contribution, @Rush! 🎉

[Rush]

It's my pleasure. Thank you for creating and maintaining this project. 💯

[andrejewski]

It would be great to expose this unsafeStringify so others can use it on trusted inputs as well; an export is used here but I can't import the code; when I do I get:

node
> require('uuid/dist/stringify')
Uncaught:
Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './dist/stringify' is not defined by "exports" in ~/my-project/node_modules/uuid/package.json

> node -v
v15.14.0

[broofa]

@andrejewski What's your use case for this? Unless stringify() is being called 1M's/second the perf gain of unsafeStringify() is unlikely matter.

[Rush]

Same as calling uuid 1M times per second :) I even think that the unsafe one should be the default.

[andrejewski]

@broofa I happen to have my UUIDs represented as UInt8Array wrapped in a simple class type:

class UUID {
  readonly bytes: Readonly<Uint8Array>
}

When this class is constructed I'm always validating the array is of the right size, so when I go to stringify I don't need to run the additional validation. UUIDs are quite pervasive in my applications, and I definitely have not benchmarked anything, but it is wonky to pay the validation cost twice.

@Rush I disagree that the unsafe version should be the default. String types are very pervasive in JS codebases and it's all too easy to pass an arbitrary string when only a UUID is expected. The type error can catch these bugs sooner. On the flip size, passing around a UUID class instance like above is quite safe and less likely to be misused. For folks that really care about eking out performance gains (ourselves in this case), explicitly opting in and understanding the trade-offs is the better choice.

[broofa]

@andrejewski The more I think about this, the less inclined I am to expose unsafeStringify(). It's generally not a good idea to incorporate these types of perf optimizations into the public API.

Since you already have a UUID class, it might make sense for you to inline your preferred stringify implementation in your class. (Feel free to copy the unsafeStringify() code for that if you'd like)

[Rush]

@andrejewski It's a PITA but since there is a lot of opposition to the idea of exposing the optimized version of stringify maybe you can just copy & paste the stringify function to your codebase and be done with it.

Not sure why we would make a statement that doing stringification a million times per second is only reserved to uuid internals and not to other use cases. I can imagine a dozen use-cases which could benefit from it.

Especially since the optimized version is 30x, yes 30 times faster.

[broofa]

@Rush You asked previously what the use cases for stringify() were. I don't have good data on that. Hence why I'm asking you about your case(s).

For what it's worth, part of my reticence here is that I've been down this path before. If you look at the git history of this project, you'll see code and docs for parse() and unparse() methods that were included in version 1. We ended up removing those from the API, however, because allowing invalid UUIDs to be generated just felt sloppy. Especially given the core premise of this module, which is that it is a rigorous implementation of RFC4122.

Thus, when we added the new methods back in as part of version 8 we made the decision to be strict about enforcing RFC compliance on inputs. Hence why stringify() throws.

Not sure why we would make a statement that doing stringification a million times per second is only reserved to uuid internals and not to other use cases.

It's a maintenance burden thing. As long as this method is internal we're free to change things as we see fit. But once it's made public, we're obligated to support it, even if we find a better solution. For example, we could inline the validation in stringify() with something like this ...

if (arr.length - offset < 16) throw('Not enough values in array')
if ((arr[offset + 6] >>> 4) & 0x07 > 5) throw ('Invalid version')
if (arr[offset+8] & 0xc0 !== 0x80) throw ('Invalid variant')

... and probably get comparable performance to unsafeStringify(). What then? If unsafeStringify() is public, we're stuck having to support it until we have a deprecation strategy. It's a pain in the ass I don't particularly want unless there's a good reason for it.

Especially since the optimized version is 30x, yes 30 times faster.

... but that's only in the tightest possible loop. As soon as you do anything interesting inside that loop - a network request, a db query, DOM manipulation, file system I/O, basically anything that makes a UUID useful - the loop time increases by 100X and that "30X" becomes "1.03X".

In absolute terms, unsafeStringify() is trimming ~0.3 µ-seconds off each stringify() call. Something like that. You'd have to call stringify() at least 33,000 times/second to see a noticeable (read, "> 1%") perf improvement? Any slower and it's just not going to matter.

I can imagine a dozen use-cases which could benefit from it.

Please share details of those, because I can't think of any.

For example, really big, high-traffic systems like Amazon, Walmart, Facebook... even if you're generating a UUID for every user action, every comment, every purchase, you're talking about maybe 100,000-1,000,000 operations/sec. But that load is spread across 100K's of multi-core servers. The per-core rate is going to be ≪ 1,000 ops/second.

It's not just about the ops/second rate. You have to sustain that rate long enough for it to matter. Calling stringify() on 10 million UUIDs (say, as part of a DB migration) is only going to take an extra 3 seconds over unsafeStringify(). 🤷

[Rush]

Am I correct that this hasn't been released yet?