Closed facundominguez closed 4 years ago
Here are the results of the benchmarks:
benchmarking References/delete global reference (attached)
time 5.822 μs (704.3 ns .. 12.74 μs)
0.066 R² (0.002 R² .. 0.209 R²)
mean 150.9 μs (70.16 μs .. 385.0 μs)
std dev 176.3 μs (58.25 μs .. 257.1 μs)
variance introduced by outliers: 99% (severely inflated)
benchmarking References/delete global reference (not attached)
time 155.5 μs (153.9 μs .. 157.0 μs)
0.997 R² (0.996 R² .. 0.998 R²)
mean 154.4 μs (152.2 μs .. 156.3 μs)
std dev 6.982 μs (5.515 μs .. 10.29 μs)
variance introduced by outliers: 45% (moderately inflated)
benchmarking References/delete global reference (separate thread)
time 305.5 ns (299.5 ns .. 312.2 ns)
0.993 R² (0.988 R² .. 0.996 R²)
mean 311.7 ns (300.1 ns .. 342.1 ns)
std dev 57.76 ns (22.54 ns .. 115.9 ns)
variance introduced by outliers: 97% (severely inflated)
It looks to me that having a separate thread do the deletion is the way to go.
Here are the results of the benchmarks:
benchmarking References/delete global reference (attached) time 5.822 μs (704.3 ns .. 12.74 μs) 0.066 R² (0.002 R² .. 0.209 R²) mean 150.9 μs (70.16 μs .. 385.0 μs) std dev 176.3 μs (58.25 μs .. 257.1 μs) variance introduced by outliers: 99% (severely inflated) benchmarking References/delete global reference (not attached) time 155.5 μs (153.9 μs .. 157.0 μs) 0.997 R² (0.996 R² .. 0.998 R²) mean 154.4 μs (152.2 μs .. 156.3 μs) std dev 6.982 μs (5.515 μs .. 10.29 μs) variance introduced by outliers: 45% (moderately inflated) benchmarking References/delete global reference (separate thread) time 305.5 ns (299.5 ns .. 312.2 ns) 0.993 R² (0.988 R² .. 0.996 R²) mean 311.7 ns (300.1 ns .. 342.1 ns) std dev 57.76 ns (22.54 ns .. 115.9 ns) variance introduced by outliers: 97% (severely inflated)
It looks to me that having a separate thread do the deletion is the way to go.
Note that the results of the third benchmark do not measure the time needed to delete a reference, but only the overhead of sending the references to a dedicated thread, which does the actual deleting. It still is a way more efficient process than attaching the thread to the jvm, then detaching it (+5% vs. +3000%).
commit b250314 fix: use main thread class loader when attaching thread as daemon
is not necessarily related to this PR, or the underlying issue, but solves a bug that was dormant until now.
It may be relevant to move it to its own PR instead
I squashed commits. The old tip of the branch is 42bc342.
I undid the provision to set the context class loader when attaching threads. I'm using loadJavaWrappers in test to load the classes ahead of calling JNI in other threads. Also, I added a troubleshooting section to the readme, to discuss this and other problems.
The naive solution of attaching a thread to the JVM before calling deleteGlobalRefNonFinalized, then detaching it afterward, has been rejecting due to adding too much overhead:
The extra cost of 25 μs led us to try to have a thread, attached to the JVM startup, which would be dedicated to deleting global references. A preliminary, still nonfunctional version, is currently on commit 8c4a7f4