Bug ID: JDK-8136469 OptimizeStringConcat fails on pre-sized StringBuilder shapes

Type: Bug
Component: hotspot
Sub-Component: compiler
Affected Version: 9

Priority: P2
Status: Closed
Resolution: Fixed

Submitted: 2015-09-14
Updated: 2017-08-07
Resolved: 2016-01-12

JDK 9
9 b105Fixed

A sample benchmark:

    @Benchmark
    @CompilerControl(CompilerControl.Mode.DONT_INLINE)
    public String explicitSized() throws Exception {
        String time = String.valueOf(System.nanoTime());
        String thread = Thread.currentThread().getName();
        String m = method;
        return new StringBuilder(63 + time.length() + thread.length() + m.length())
                .append("[")
                .append(time)
                .append("] ")
                .append(thread)
                .append(":")
                .append("Calling an application method \"")
                .append(m)
                .append("\" without fear and prejudice.")
                .toString();
    }

This benchmark regresses severely with Compact Strings. Turning OptimizeStringConcat off does not degrade performance further, which means it is broken with Compact Strings to begin with. The disassembly confirms it. Also see the (broken) workaround patch below:

# JDK 9 Baseline
LogLineBench.explicitSized:     83.267 ��   3.021   ns/op
LogLineBench.explicitSized:��gc.alloc.rate.norm:   384.000 ��   0.001    B/op

# JDK 9 Baseline, -XX:-OptimizeStringConcat
LogLineBench.explicitSized:   190.090 ��   5.893   ns/op
LogLineBench.explicitSized:��gc.alloc.rate.norm:  1296.000 ��   0.001    B/op

# JDK 9 Compact Strings
LogLineBench.explicitSized:   148.536 ��   5.032   ns/op
LogLineBench.explicitSized:��gc.alloc.rate.norm:   704.000 ��   0.001    B/op

# JDK 9 Compact Strings, -XX:-OptimizeStringConcat
LogLineBench.explicitSized:   143.668 ��   7.654   ns/op
LogLineBench.explicitSized:��gc.alloc.rate.norm:   704.000 ��   0.001    B/op

Fix verified by manual inspection with attached test.
07-08-2017
I was able to reproduce this with the latest JDK9 version (see "TestPresizedStringBuilder.java"). The problem is that the initial size of the StringBuilder depends on a static final boolean that is initialized to true at runtime. Therefore the string concatenation control flow chain contains an IfNode with a ConI (1) as input instead of the expected BoolNode and StringConcat::validate_control_flow() silently bails out (see "graph.png").
17-09-2015
I checked again, and can again confirm this is Compact Strings-specific issue: http://cr.openjdk.java.net/~shade/8136469/jdk9-vs-density.txt These benchmarks are part of our usual Compact Strings benchmark suite. Note the original issue happens on code path that bails without PrintOptimizeStringConcat message -- normally you would have PrintOptimizeStringConcat debug block before "fail = true", this is another thing that should be fixed. Therefore the thing you observe in your minimized case is something else.
16-09-2015
I simplified the benchmark to the following method public static String explicitSized(String a) { return new StringBuilder(a.length()).append("a").toString(); } Compiling this with -XX:+PrintOptimizeStringConcat bails out with considering toString call in Test::explicitSized @ bci:16 fusion would fail for 26 Allocate === 5 6 7 8 1 ( 24 22 23 1 10 ) [[ 27 28 29 36 37 38 ]] rawptr:NotNull ( int:>=0, java/lang/Object:NotNull , bool, top ) Test::explicitSized @ bci:0 !jvms: Test::explicitSized @ bci:0 69 CatchProj === 68 [[ 81 ]] #0@bci -1 !jvms: Test::explicitSized @ bci:5 68 Catch === 62 63 [[ 69 70 ]] !jvms: Test::explicitSized @ bci:5 62 Proj === 46 [[ 68 ]] #0 !jvms: Test::explicitSized @ bci:5 46 CallStaticJava === 53 37 25 8 1 ( 61 1 43 43 ) [[ 62 63 64 66 ]] # Static java.lang.String::length int ( java/lang/String:NotNull:exact ) Test::explicitSized @ bci:5 !jvms: Test::explicitSized @ bci:5 This clearly is a bug because (as you said) it does not happen when using a local variable for the length. However, this happens both with the baseline JDK9 and with CompactStrings. Am I missing something?
16-09-2015
Thanks for the information, Aleksey. I'll have a look.
15-09-2015
This issue nils when we extract the length computation into local variable: int len = 63 + time.length() + thread.length() + m.length(); return new StringBuilder(len)
14-09-2015
It would seem validate_control_flow() does not expect ConI node instead of Bool node here: http://cr.openjdk.java.net/~shade/8136469/poc.patch
14-09-2015

Blocks :	JDK-8146629 - Make phase->is_IterGVN() accessible from Node::Identity and Node::Value
Blocks :	JDK-8085796 - JEP 280: Indify String Concatenation