feat: improved bytesToUuid func #434

awwit · 2020-05-03T14:44:28Z

I replaced the conversion of bytes to uuid with a more efficient way (and without inflating the size of the resulting string).

Here's what I got in terms of performance.

Benchmark master:

uuidv1() x 1,439,640 ops/sec ±0.91% (93 runs sampled)
uuidv1() fill existing array x 7,449,049 ops/sec ±0.55% (90 runs sampled)
uuidv4() x 349,506 ops/sec ±0.88% (90 runs sampled)
uuidv4() fill existing array x 404,741 ops/sec ±1.62% (90 runs sampled)
uuidv3() x 141,296 ops/sec ±0.79% (90 runs sampled)
uuidv5() x 142,066 ops/sec ±0.87% (92 runs sampled)

Benchmark this branch:

uuidv1() x 1,638,366 ops/sec ±1.07% (92 runs sampled)
uuidv1() fill existing array x 7,837,067 ops/sec ±0.68% (92 runs sampled)
uuidv4() x 394,752 ops/sec ±1.09% (90 runs sampled)
uuidv4() fill existing array x 412,690 ops/sec ±1.30% (91 runs sampled)
uuidv3() x 155,735 ops/sec ±0.72% (91 runs sampled)
uuidv5() x 152,023 ops/sec ±0.90% (89 runs sampled)

ctavan · 2020-05-03T21:00:53Z

Great optimization, thank you!

Interestingly the string concatenation seems to be faster on V8 (Node.js/Chrome), but slower on JavaScriptCore (Safari) and SpiderMonkey (Firefox).

Since this is definitely a perf improvement for Node.js, I will still merge it since performance should only ever be a real concern in serverside use cases, not in the browser.

For the record:

Safari 13.1

Master:

[Log] Starting. Tests take ~1 minute to run ... (benchmark.js, line 9)
[Log] uuidv1() x 373,296 ops/sec ±2.65% (49 runs sampled) (benchmark.js, line 34)
[Log] uuidv1() fill existing array x 589,196 ops/sec ±4.61% (49 runs sampled) (benchmark.js, line 34)
[Log] uuidv4() x 839,177 ops/sec ±1.15% (62 runs sampled) (benchmark.js, line 34)
[Log] uuidv4() fill existing array x 1,366,066 ops/sec ±1.40% (57 runs sampled) (benchmark.js, line 34)
[Log] uuidv3() x 22,352 ops/sec ±0.81% (59 runs sampled) (benchmark.js, line 34)
[Log] uuidv5() x 13,163 ops/sec ±2.27% (56 runs sampled) (benchmark.js, line 34)
[Log] Fastest is uuidv4() fill existing array (benchmark.js, line 37)

Branch:

[Log] Starting. Tests take ~1 minute to run ... (benchmark.js, line 9)
[Log] uuidv1() x 316,734 ops/sec ±4.59% (47 runs sampled) (benchmark.js, line 39)
[Log] uuidv1() fill existing array x 516,869 ops/sec ±3.99% (45 runs sampled) (benchmark.js, line 39)
[Log] uuidv4() x 653,217 ops/sec ±2.61% (56 runs sampled) (benchmark.js, line 39)
[Log] uuidv4() fill existing array x 1,218,314 ops/sec ±1.52% (55 runs sampled) (benchmark.js, line 39)
[Log] uuidv3() x 14,437 ops/sec ±1.20% (52 runs sampled) (benchmark.js, line 39)
[Log] uuidv5() x 10,993 ops/sec ±2.21% (49 runs sampled) (benchmark.js, line 39)
[Log] Fastest is uuidv4() fill existing array (benchmark.js, line 42)

Firefox 75

Master:

Starting. Tests take ~1 minute to run ... benchmark.js:9:9
uuidv1() x 858,098 ops/sec ±2.46% (54 runs sampled) benchmark.js:34:13
uuidv1() fill existing array x 3,341,471 ops/sec ±1.99% (59 runs sampled) benchmark.js:34:13
uuidv4() x 555,726 ops/sec ±1.51% (57 runs sampled) benchmark.js:34:13
uuidv4() fill existing array x 1,007,543 ops/sec ±1.13% (61 runs sampled) benchmark.js:34:13
uuidv3() x 49,755 ops/sec ±3.55% (31 runs sampled) benchmark.js:34:13
uuidv5() x 33,969 ops/sec ±1.27% (57 runs sampled) benchmark.js:34:13
Fastest is uuidv1() fill existing array benchmark.js:37:13

This Branch:

Starting. Tests take ~1 minute to run ... benchmark.js:9:9
uuidv1() x 284,785 ops/sec ±1.38% (56 runs sampled) benchmark.js:39:13
uuidv1() fill existing array x 2,814,427 ops/sec ±2.47% (54 runs sampled) benchmark.js:39:13
uuidv4() x 275,961 ops/sec ±1.89% (55 runs sampled) benchmark.js:39:13
uuidv4() fill existing array x 1,037,837 ops/sec ±0.44% (61 runs sampled) benchmark.js:39:13
uuidv3() x 26,069 ops/sec ±1.21% (22 runs sampled) benchmark.js:39:13
uuidv5() x 24,065 ops/sec ±2.95% (52 runs sampled) benchmark.js:39:13
Fastest is uuidv1() fill existing array

ctavan · 2020-05-03T21:05:45Z

src/bytesToUuid.js

@@ -14,28 +14,28 @@ function bytesToUuid(buf, offset) {
  const bth = byteToHex;

  // join used to fix memory issue caused by concatenation: https://bugs.chromium.org/p/v8/issues/detail?id=3175#c4


The issue linked above is still unresolved.

@awwit have you tried the examples in that issue in recent Node.js or Chrome versions to see if the issue still exists?

@broofa can you shed some light on this? I saw the discussion in #267 but cannot judge if this is still an issue?

Oh, I just saw #267 (comment) which I must have missed while reading the other issue initially.

Seems like .toLowerCase() is also a workaround for the memory issue? If this turns out to be true (@broofa / @sava-smith can you confirm) then we should at least update the comment here to reflect the new hack?

@ctavan I tried different ways to fix this bug. It still exists in the V8 engine. (Did not check for other engines).

The fastest option was ".toLowerCase()" on V8 engine. It also forms a normal-sized string like .join('').

There is still a compromise option (' ' + str).slice(1) which works even faster, but consumes a little more memory for the result string.

ctavan · 2020-05-03T21:06:39Z

I should correct myself: I'm happy to merge this if we can ensure that the issue discussed in #267 no longer exists.

awwit · 2020-05-03T21:51:05Z

@ctavan

I should correct myself: I'm happy to merge this if we can ensure that the issue discussed in #267 no longer exists.

This problem still exists. But it can be solved by .toLowerString().

I am not sure that the results in the browser can be trusted. Every time I get new data. And the results are getting worse with each next run…

This is probably due to the way the browser works with JS (throttling execution to save device energy, or something like that)

broofa

267 does a good job capturing what went into the current form of this function. 'Looks like you've taken all that into account.

I agree with @ctavan that v8 (Node) perf on the server trumps browser perf. I wouldn't worry too much about browser perf as long as there aren't glaring regressions.

Please update the comment in bytesToUuid as follows...

// Note: Be careful editing this code!  It's been tuned for performance
// and works in ways you may not expect. See https://github.com/uuidjs/uuid/pull/434

I see no reason not to take this PR. (Aside from the fact very few people will notice this change unless something breaks. 😝 )

broofa · 2020-05-04T16:38:45Z

src/bytesToUuid.js

-  ].join('');
+  // Note: Be careful editing this code!  It's been tuned for performance
+  // and works in ways you may not expect. See https://github.com/uuidjs/uuid/pull/434
+  // `toLowerCase` used to fix memory issue caused by concatenation: https://bugs.chromium.org/p/v8/issues/detail?id=3175#c4


Referring to this PR should suffice. Readers can get from here to 267 and the Chromium issue if they want to dive that deep.

Suggested change

// `toLowerCase` used to fix memory issue caused by concatenation: https://bugs.chromium.org/p/v8/issues/detail?id=3175#c4

Ok, as you say…

ctavan

👏 🚀

feat: improved bytesToUuid func

2136152

ctavan reviewed May 3, 2020

View reviewed changes

broofa reviewed May 4, 2020

View reviewed changes

style: added comment to bytesToUuid func

149adf7

awwit requested a review from broofa May 4, 2020 16:20

broofa requested changes May 4, 2020

View reviewed changes

style: remove one line of comment

a660ec5

awwit requested a review from broofa May 4, 2020 16:52

broofa approved these changes May 4, 2020

View reviewed changes

ctavan approved these changes May 4, 2020

View reviewed changes

ctavan merged commit e156415 into uuidjs:master May 4, 2020

kkntl mentioned this pull request Oct 11, 2023

perf: remove superfluous call to toLowerCase #677

Merged

This was referenced Apr 4, 2024

renovate/37.279.3 package update wolfi-dev/os#16355

Closed

renovate/37.279.4 package update wolfi-dev/os#16373

Closed

renovate/37.280.0 package update wolfi-dev/os#16439

Merged

broofa mentioned this pull request Jun 8, 2024

chore(release): 10.0.0 #760

Merged

This was referenced Jun 29, 2024

renovate/37.421.0 package update wolfi-dev/os#22887

Closed

renovate/37.421.1 package update wolfi-dev/os#22898

Closed

renovate/37.421.2 package update wolfi-dev/os#22900

Closed

uuidbot mentioned this pull request Oct 27, 2024

chore(main): release 11.0.0 #808

Merged

This was referenced Oct 27, 2024

chore(main): release 11.0.0 #809

Merged

chore(main): release 11.0.0 #810

Closed

chore(main): release 11.0.0 #813

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: improved bytesToUuid func #434

feat: improved bytesToUuid func #434

awwit commented May 3, 2020

ctavan commented May 3, 2020

ctavan May 3, 2020

ctavan May 3, 2020

awwit May 3, 2020

ctavan commented May 3, 2020

awwit commented May 3, 2020 •

edited

Loading

broofa left a comment •

edited

Loading

broofa May 4, 2020 •

edited

Loading

awwit May 4, 2020

ctavan left a comment

		@@ -14,28 +14,28 @@ function bytesToUuid(buf, offset) {
		const bth = byteToHex;

		// join used to fix memory issue caused by concatenation: https://bugs.chromium.org/p/v8/issues/detail?id=3175#c4

feat: improved bytesToUuid func #434

feat: improved bytesToUuid func #434

Conversation

awwit commented May 3, 2020

ctavan commented May 3, 2020

Safari 13.1

Firefox 75

ctavan May 3, 2020

Choose a reason for hiding this comment

ctavan May 3, 2020

Choose a reason for hiding this comment

awwit May 3, 2020

Choose a reason for hiding this comment

ctavan commented May 3, 2020

awwit commented May 3, 2020 • edited Loading

broofa left a comment • edited Loading

Choose a reason for hiding this comment

broofa May 4, 2020 • edited Loading

Choose a reason for hiding this comment

awwit May 4, 2020

Choose a reason for hiding this comment

ctavan left a comment

Choose a reason for hiding this comment

awwit commented May 3, 2020 •

edited

Loading

broofa left a comment •

edited

Loading

broofa May 4, 2020 •

edited

Loading