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.
The fix for JDK-8225670 failed due to performance regressions and failures including SPARC (see linked bugs) and therefore needs to be re-worked.
Comments
Fix request [11u]
I downport this for parity with 11.0.13-oracle.
Small risk as the change only resets a field.
Wrt. Rolands comment above: as it is in 11.0.13-oracle now, I think we should take this, too.
Actually, we found several similar changes helped with the stability of the JVM, as JDK-8210236
that applies to the same function.
Nightly testing passes.
11u backport: this doesn't seem to be required for 11. Concurrent class loading (which is mentioned in the review thread as possibly triggering this) is not in 11 and the tests that cause the failure are special in that they make use of the whitebox API. The tests are not in 11 either.
25-03-2020
The following test case shows the benefit of not clearing the counter with clear_row() as done in JDK-8225670 which caused regressions:
public void test() {
String s = "test";
for (int i = 0; i < 1_000_000; i++ ) {
s.replace("e", "t"); // Just a big enough method that does not get inlined by default
}
}
If we use clear_row(), then String::replace is not inlined:
org.sample.ClearRow::test @ 5 (27 bytes) made not entrant
@ 16 java.lang.String::replace (255 bytes) too big
The MDO counter for String::replace is reset to zero. This profile information is passed to ok_to_inline [1] and the zeroed counter is eventually read at [2]. The following frequency check [3] fails and thus InlineTree::should_inline returns false because the method is too big (and wrongly assumed to be never called).
However, if we just use set_receiver(row, NULL) instead of clear_row(), which does not reset the counter, then call_site_count is big number greater than zero at [2] and the check at [3] succeeds. The maximum inline size is increased to FreqInlineSize (325 by default). Therefore String::replace is inlined:
org.sample.ClearRow::test @ 5 (27 bytes) made not entrant
@ 16 java.lang.String::replace (255 bytes) inline (hot)
We can already see in a local JMH benchmark that not clearing the counter is beneficial:
Using set_receiver(row, NULL) as in the current fix:
Benchmark Mode Cnt Score Error Units
ClearRow.test thrpt 5 42.810 ± 1.572 ops/s
Using clear_row() as in JDK-8225670:
Benchmark Mode Cnt Score Error Units
ClearRow.test thrpt 5 32.658 ± 1.267 ops/s
There are other cases, for example by using HashMap::put instead, in which call_does_dispatch is true [4]. It then skips directly to the check at [5]. The check succeeds having a non reset counter and the speculative_receiver_type can be used to inline the method.
The String class is not unloaded. In the previous fix for JDK-8225670 [6] clear_row() was called if either the klass 'k' was NULL or it was unloaded. This means that in this test case the MDO contains the String klass at row index 0 but the index 1 is empty, i.e. the klass is NULL. As a result, that counter was cleared with clear_row() by mistake when processing the row index 1 (since 'k' is NULL). The new fix will only set the klass to NULL with set_receiver(row, NULL) and will not clear the counter anymore.
So I guess we've seen large performance regressions due to many such missed inline opportunities due to lost/wrong profiling information.
[1] http://hg.openjdk.java.net/jdk/jdk/file/18c246ad2ff9/src/hotspot/share/opto/doCall.cpp#l171
[2] http://hg.openjdk.java.net/jdk/jdk/file/18c246ad2ff9/src/hotspot/share/opto/bytecodeInfo.cpp#l163
[3] http://hg.openjdk.java.net/jdk/jdk/file/18c246ad2ff9/src/hotspot/share/opto/bytecodeInfo.cpp#l170
[4] http://hg.openjdk.java.net/jdk/jdk/file/18c246ad2ff9/src/hotspot/share/opto/doCall.cpp#l167
[5] http://hg.openjdk.java.net/jdk/jdk/file/18c246ad2ff9/src/hotspot/share/opto/doCall.cpp#l205
[6] http://cr.openjdk.java.net/~thartmann/8225670/webrev.00/