Closed jchambers closed 6 years ago
Let me check.
@jchambers implementations seems fine, however new method is slower, seems like because of
long mostSignificantBits = random.nextLong();
long leastSignificantBits = random.nextLong();
new method is slower
Slower than UUID.randomUUID
, you mean? Will see what I can work out there. Thanks for checking, and sorry I don't have a benchmark-appropriate setup on me to check for myself right now :\
@jchambers correct. I played a bit with your code and used code from UUID class itself
private UUID(byte[] data) {
long msb = 0;
long lsb = 0;
assert data.length == 16 : "data must be 16 bytes in length";
for (int i=0; i<8; i++)
msb = (msb << 8) | (data[i] & 0xff);
for (int i=8; i<16; i++)
lsb = (lsb << 8) | (data[i] & 0xff);
this.mostSigBits = msb;
this.leastSigBits = lsb;
}
And got the same performance. Look like either scalar replacement take place for UUID
instance or some intristics are used.
I played a bit with your code and used code from UUID class itself
Sorry to say that, but UUID class itself is under GPL and not MIT as this library ((
Sorry to say that, but UUID class itself is under GPL and not MIT as this library
Sure—I think we're all on the same page there. I don't think anybody intends to literally use the existing implementation, but we should definitely make sure we're actually making things better. I think the main message is that we need to do more to catch up with the JDK implementation.
I've revised and added some benchmarks. This isn't as much of an obvious win as some other changes, but here's what I'm getting for benchmarks now:
Benchmark | Throughput |
---|---|
UUID#randomUUID() |
520,187.487 ± 12,039.213 UUIDs/second |
FastUUID#randomUUID(Random) |
526,677.776 ± 13,826.630 UUIDs/second |
FastUUID.toString(FastUUID.randomUUID(Random)) |
506,551.001 ± 8,531.199 Strings/second |
FastUUID.randomUUIDString(Random) |
526,072.839 ± 10,159.912 Strings/second |
From a throughput perspective, this is only sliiiiiiightly faster than generating a new UUID and then calling toString
(time spent generating random numbers for the UUID dominates the throughput here). We're only avoiding one object allocation, too, and so the gains are pretty small.
What do you folks think? Is it worth the added complexity? I have to admit I'm on the fence.
@jchambers could you please tell us what JVM and version do you use? I played today a bit with your code and JDK code and JDK UUID.toString()
was always faster (Oracle Hotspot VM 9.0.1).
could you please tell us what JVM and version do you use? I played today a bit with your code and JDK code and JDK UUID.toString() was always faster (Oracle Hotspot VM 9.0.1).
You're right, but I think you need to rebase. In Open JDK 9, they use a fancy native method for UUID#toString()
. We'll just use that if we're running in Java 9 or newer as of 035c71b081a817d36a5b3608c28c89c5f705b839. I updated the README to set expectations for performance in Java 9 and pre-Java-9 in 277d9073bdcf43ad245dee93141b7db8c19bba8f.
Wait. This is my branch. I need to rebase, not you ;)
Aha, you did fallback for Java 9. Nice. Anyway, your code was very helpful for me as in my production code I do:
UUID.randomUUID().toString().replace("-", "")
that is 6 times slower than modified FastUUID.toString
that creates string without -
and I have no need in replace
operation.
So, to call the question, @doom369 it sounds like you've found a very specific adaptation of this that's working from you specifically because you were removing the hyphens. In the general case, though, it seems like this isn't offering much benefit. I think I'm going to close this pull request for now, but I remain open to arguments that we should ship it.
Please let me know if you disagree!
@jchambers no problem. Thanks for your efforts.
As requested in #1, this introduces a method for generating random UUID strings without an intermediate UUID instance.
@doom369 seem legit?