RFR: 8362394: C2: Repeated stacked string concatenation fails with "Hit MemLimit" and other resourcing errors
Roberto Castañeda Lozano
rcastanedalo at openjdk.org
Mon Aug 11 08:27:13 UTC 2025
On Fri, 8 Aug 2025 06:10:56 GMT, Daniel Skantz <dskantz at openjdk.org> wrote:
> This PR addresses a bug in the stringopts phase. During string concatenation, repeated stacking of concatenations can lead to excessive compilation resource use and generation of questionable code as the merging of two StringBuilder-append-toString links sc1 and sc2 can result in a new StringBuilder with the size sc1->num_arguments() * sc2->num_arguments().
>
> In the attached test, the size of the successively merged StringBuilder doubles on each merge -- there's 24 of them -- as the toString result of the first component is used twice in the second component [1], etc. Not only does the compiler hang on this test case, but the string concat optimization seems to give an arbitrary amount of back-to-back stores in the generated code depending on the number of stacked concatenations.
>
> The proposed solution is to put an upper bound on the size of a merged concatenation, which guards against this case of repeated concatenations on the same string variable, and potentially other edge cases. 100 seems like a generous limit, and higher limits could be insufficient as each argument corresponds to about 20 new nodes later in replace_string_concat [2].
>
> [1] https://github.com/openjdk/jdk/blob/0ceb366dc26e2e4f6252da9dd8930b016a5d46ba/src/hotspot/share/opto/stringopts.cpp#L303
>
> [2] https://github.com/openjdk/jdk/blob/0ceb366dc26e2e4f6252da9dd8930b016a5d46ba/src/hotspot/share/opto/stringopts.cpp#L1806
>
> Testing: T1-4.
>
> Extra testing: verified that no method in T1-4 is being compiled with a merged concat candidate exceeding the suggested limit of 100 aguments, regardless of whether or not the later checks verify_control_flow() and verify_mem_flow pass.
Good catch, Daniel! I have a few comments, mostly about the test.
src/hotspot/share/opto/stringopts.cpp line 687:
> 685: // sc->num_arguments() * other->num_arguments(),
> 686: // which is a problem in the case of repeated stacked concats.
> 687: // Put a limit at 100 arguments to guard against excessive resource use.
Could you extract this limit into a constant and give it a meaningful name?
test/hotspot/jtreg/ProblemList.txt line 1:
> 1: #
Did you consider running the test with `-XX:-OptoScheduling`, as an alternative to problem listing? This should simply avoid the problematic assertion and would be more robust w.r.t. other platforms.
test/hotspot/jtreg/compiler/stringopts/TestStackedConcatsMany.java line 42:
> 40: for (int i = 0; i < 10; i++) {
> 41: String s = f(" ");
> 42: }
This could be simplified into:
Suggestion:
new StringBuilder(); // Trigger loading of the StringBuilder class.
String s = f(" ");
Then you can narrow down the CompileOnly command to `-XX:CompileOnly=compiler.stringopts.TestStackedConcatsMany::f`.
-------------
Changes requested by rcastanedalo (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/26685#pullrequestreview-3104581918
PR Review Comment: https://git.openjdk.org/jdk/pull/26685#discussion_r2265964055
PR Review Comment: https://git.openjdk.org/jdk/pull/26685#discussion_r2266001174
PR Review Comment: https://git.openjdk.org/jdk/pull/26685#discussion_r2265968794
More information about the hotspot-compiler-dev
mailing list