JDK-8207359 : Make SymbolTable increment_refcount disallow zero
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 12
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2018-07-16
  • Updated: 2021-05-25
  • Resolved: 2018-07-20
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 12
12 b04Fixed
Related Reports
Blocks :  
JDK-8195100 - Use a low latency hashtable for SymbolTable
Relates :  
JDK-8239347 - Refactor Symbol to make _length a standalone field again
Relates :  
JDK-8208084 - Windows build failure - "'snprintf': identifier not found"
Description
SymbolTable::increment_refcount() should check for and disallow incrementing the refcount to observe a zero value.

This can happen if another thread is decrementing the refcount while we are incrementing it.   If the other thread decrements the refcount to zero, we cannot return this symbol.  This is only possible during insertion into the concurrent hashtable.
Comments
JDK 11 does not have the concurrent hashtable, which enables the situation that we're defending against with this change, so it shouldn't be backported to JDK 11.
25-05-2021
We observe a significant performance regression after backporting this patch to JDK11, I've attached the flame graphs.. -- java -jar /home/joeylee.lz/jmh/target/benchmarks.jar benchmarks.profiling.openjdk.java.lang.ClassForName.test1 -t 10 Before: 3485.681 ��(99.9%) 903.834 ns/op After revert : 2796.066 ��(99.9%) 123.088 ns/op
25-05-2021
We could have two threads doing TempNewSymbol. The decrement would be in the destructor of TempNewSymbol, while the other thread is trying to increment it. If the other thread increments the symbol from zero to 1, that was ok with the stop the world symbol table, but is not okay with a concurrently cleaning symbol table. The gtest I added does this. You're right, the NOT_PRODUCT is redundant. We can fix that in a future change.
30-07-2018
It is very unclear to me how we can be trying to increment the refcount and decrement it to zero at the same time. This would suggest that the decrement is being done asynchronously with respect to the actual use of the symbol - which then begs the question as to how the code can know to do the decrement? If we only increment a live symbol then the symbol is live to the thread doing the increment, so how can another thread be decrementing as if it held the last reference to the symbol ?? A description of the symbol lifecycle, in the context of this problem, we be very informative. Aside, in: +#ifndef PRODUCT + if (refcount() != PERM_REFCOUNT) { // not a permanent symbol + NOT_PRODUCT(Atomic::inc(&_total_count);) + } +#endif the NOT_PRODUCT is superfluous.
30-07-2018