lazy secure random #3723
lazy secure random #3723
Conversation
…ocation
shaves off 10-15% of Thread.new/start :
```
Rehearsal ----------------------------------------------------------
Thread.new [100000x] 12.800000 6.850000 19.650000 ( 10.392600)
Thread.start [100000x] 12.670000 5.680000 18.350000 ( 9.625846)
------------------------------------------------ total: 38.000000sec
user system total real
Thread.new [100000x] 12.780000 5.760000 18.540000 ( 9.620000)
Thread.start [100000x] 12.490000 5.810000 18.300000 ( 9.524975)
```
```
Rehearsal ----------------------------------------------------------
Thread.new [100000x] 11.040000 5.560000 16.600000 ( 8.708792)
Thread.start [100000x] 10.740000 5.520000 16.260000 ( 8.400990)
------------------------------------------------ total: 32.860000sec
user system total real
Thread.new [100000x] 10.840000 5.620000 16.460000 ( 8.506339)
Thread.start [100000x] 10.460000 5.560000 16.020000 ( 8.323819)
```
|
We may want to make the field Otherwise, 👍. |
|
Oh, it might also be interesting to see how it looks when entropy is not being fed or the system is in an entropy-starved situation. Most people I know don't have entropy-feeding services installed, so that would be a more typical case. Note also that Fiber and Enumerator.next instantiation both require ThreadContext instantiation. Enumerator#next could be greatly improved by reducing TC init costs. |
|
@headius will try to starve my /dev/random although I only seen those issues in cloud installs. does a ThreadContext really need a volatile marker - its accessed by a single thread 99% of time, when its not the non-owning thread won't try to read any fields (mostly seen |
|
@kares Good point about TC being thread-local anyway; I guess volatility is not really a requirement then. |
|
tried depleting |
| } catch (Exception e) { | ||
| trySHA1PRNG = false; | ||
| sr = new SecureRandom(); | ||
| public SecureRandom getSecureRandom() { |
There was a problem hiding this comment.
It looks like you're relying on ThreadContext effectively being thread-local to avoid any locking. It might be good to note why this method lacks any synchronization, since it looks like a bug upon first inspection.
There was a problem hiding this comment.
thanks, its basically the nature of the whole class being assumed to be "single-threaded".
it has some (all non-final) non-volatile fields with non-synchronized getters/setters already.
There was a problem hiding this comment.
It's also not a concern unless it's a bad thing to get extra instantiations here. My concern about volatility was just to force SecureRandom state has been published; if two get created, it's not a big deal.
There was a problem hiding this comment.
Yeah, fair enough. I meant just having a comment so others don't think it's problematic. But if that's the nature of the class, that's fine too.
ThreadContext.secureRandomfield was introduced as an optimization for theSecureRandomLibrary... however as its a final field initialized on
new ThreadContextall Ruby thread allocations potentially pay the cost of creating a secure random generator. which under some circumstances might get slow as they all seed themselves using a shared seeder (from a single source of entropy e.g. /dev/random under *nix).numbers show a noticeable improvement on Ubuntu (rng-pack installed thus I rarely deplete /dev/random) :
100_000 threads BEFORE :
100_000 threads AFTER :
with 10_000 threads :