RFR: 8328473: StringTable and SymbolTable statistics delay time to safepoint [v2]

[Aleksey Shipilev]

On Mon, 24 Feb 2025 18:41:28 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This change adds a safepoint poll to gathering statistics for the Symbol and String tables, using the ConcurrentHashTableTasks to chunk up the walk.  The stringTable and symbolTable is similar, like the GrowTask and DeleteTask code.  Maybe this can be cleaned up but I don't have a good idea about that yet that doesn't involve yet another level of templated functions and code. This is already pretty highly templatized.
>> Tested with tier1-4 and runThese internal test with JFR and failure injection to verify that we do try to safepoint while gathering statistics.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fxi typo.

Marked as reviewed by shade (Reviewer).

I realized my `jcmd` suggestion would not work, because it _itself_ runs in `VMThread`, so we miss an opportunity to stall another pending (GC) safepoint. 

Easiest way to reproduce this is to go to `lib/jfr/default.jfc` and set the low period:


    <event name="jdk.StringTableStatistics">
      <setting name="enabled">true</setting>
      <setting name="period">100ms</setting>
    </event>


Then run this:


public class GC {
	static final int COUNT = 5_000_000;
        static String[] STRS = new String[COUNT];
	public static void main(String... args) throws Exception {
                for (int c = 0; c < COUNT; c++) {
			STRS[c] = "String" + c;
			STRS[c].intern();
                }
		while (true) {
			System.gc();
			Thread.sleep(100);
		}
	}
}

$ build/linux-x86_64-server-release/images/jdk/bin/java -Xmx1g -XX:+UseParallelGC -XX:StartFlightRecording=filename=100us.jfr -Xlog:safepoint GC.java


Before the patch:


[28.594s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100409233 ns, Reaching safepoint: 63522678 ns, At safepoint: 35075399 ns, Total: 98598077 ns
[28.830s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100408313 ns, Reaching safepoint: 99500774 ns, At safepoint: 35990128 ns, Total: 135490902 ns
[29.064s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100403672 ns, Reaching safepoint: 99545475 ns, At safepoint: 34358513 ns, Total: 133903988 ns
[29.259s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100392112 ns, Reaching safepoint: 60340691 ns, At safepoint: 33913938 ns, Total: 94254629 ns
[29.405s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100350133 ns, Reaching safepoint: 3830 ns, At safepoint: 45870373 ns, Total: 45874203 ns
[29.540s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100391972 ns, Reaching safepoint: 4240 ns, At safepoint: 34334753 ns, Total: 34338993 ns
[29.739s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100378902 ns, Reaching safepoint: 64600069 ns, At safepoint: 34248220 ns, Total: 98848289 ns
[29.985s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100398184 ns, Reaching safepoint: 99533655 ns, At safepoint: 46089494 ns, Total: 145623149 ns
[30.232s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100396804 ns, Reaching safepoint: 99533335 ns, At safepoint: 47408568 ns, Total: 146941903 ns
[30.407s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100339512 ns, Reaching safepoint: 39161103 ns, At safepoint: 35173080 ns, Total: 74334183 ns


After the patch:


[29.159s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100415121 ns, Reaching safepoint: 3470 ns, At safepoint: 34602873 ns, Total: 34606343 ns
[29.294s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100364263 ns, Reaching safepoint: 77200 ns, At safepoint: 34833386 ns, Total: 34910586 ns
[29.429s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100424341 ns, Reaching safepoint: 77051 ns, At safepoint: 34416483 ns, Total: 34493534 ns
[29.564s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100380481 ns, Reaching safepoint: 76160 ns, At safepoint: 34529093 ns, Total: 34605253 ns
[29.699s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100446433 ns, Reaching safepoint: 6670 ns, At safepoint: 34552893 ns, Total: 34559563 ns
[29.834s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100372811 ns, Reaching safepoint: 79071 ns, At safepoint: 34600173 ns, Total: 34679244 ns
[29.968s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100372983 ns, Reaching safepoint: 3560 ns, At safepoint: 34080189 ns, Total: 34083749 ns
[30.104s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100361950 ns, Reaching safepoint: 78221 ns, At safepoint: 35596762 ns, Total: 35674983 ns
[30.240s][info][safepoint  ] Safepoint "ParallelGCCollect", Time since last: 100448723 ns, Reaching safepoint: 6560 ns, At safepoint: 34830036 ns, Total: 34836596 ns


I think it shows the current claiming strategy is good enough to mitigate TTSP stalls.

-------------

PR Review: https://git.openjdk.org/jdk/pull/23750#pullrequestreview-2642038463
PR Comment: https://git.openjdk.org/jdk/pull/23750#issuecomment-2682902510