-
-
Notifications
You must be signed in to change notification settings - Fork 456
Replace GPL licensed Random with MIT licensed PCG random #4664
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
Replace GPL licensed Random with MIT licensed PCG random #4664
Conversation
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
ee747ae | 405.43 ms | 485.70 ms | 80.28 ms |
3699cd5 | 423.60 ms | 495.52 ms | 71.92 ms |
ee747ae | 400.46 ms | 423.61 ms | 23.15 ms |
ee747ae | 554.98 ms | 611.50 ms | 56.52 ms |
ee747ae | 374.71 ms | 455.18 ms | 80.47 ms |
ee747ae | 382.73 ms | 435.41 ms | 52.68 ms |
ee747ae | 396.82 ms | 441.67 ms | 44.86 ms |
ee747ae | 358.21 ms | 389.41 ms | 31.20 ms |
7314dbe | 437.83 ms | 505.64 ms | 67.81 ms |
ee747ae | 415.92 ms | 470.15 ms | 54.23 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
3699cd5 | 1.58 MiB | 2.10 MiB | 533.45 KiB |
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
7314dbe | 1.58 MiB | 2.10 MiB | 533.45 KiB |
ee747ae | 1.58 MiB | 2.10 MiB | 530.95 KiB |
…icensed_pcg_random
bits = ((l << 32) + (int) j >>> 1); | ||
val = bits % n; | ||
} while (bits - val + (n - 1) < 0); | ||
return val; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Input Validation Flaws in Random Number Methods
The nextInt(int n)
method is missing validation for n <= 0
, which can cause an ArithmeticException
(for n=0
) or lead to unpredictable behavior and potential infinite loops (for n < 0
). Similarly, nextLong(long n)
only validates n == 0
; negative n
values can also cause unpredictable behavior or infinite loops. A stray semicolon at line 183 serves no purpose.
Additional Locations (1)
nextseed = (oldseed * multiplier + addend) & mask; | ||
} while (!seed.compareAndSet(oldseed, nextseed)); | ||
return (int) (nextseed >>> (48 - bits)); | ||
// state = (state * MULT_64) + inc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Random Class Regression Causes Thread Safety Issues
The Random
class introduced a thread safety regression. It switched from AtomicLong
and synchronized methods to unsynchronized plain long
fields (state
, inc
). This creates race conditions, which can corrupt the internal state and degrade random number quality in concurrent use. The setSeed
method, previously synchronized, now also lacks protection, as indicated by a TODO
.
📜 Description
Replace GPL licensed Random with MIT licensed PCG random
Here's benchmark results for generating UUIDs with different versions of Random:
This shows the new PCG based
Random
is similar in single threaded performance tojava.util.Random
which the old implementation was a subset of.💡 Motivation and Context
Avoid GPL licensed code in SDK
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps