Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8076758

new StringBuilder().append(String).toString() should be recognized by OptimizeStringConcat

    Details

    • Type: Enhancement
    • Status: Resolved
    • Priority: P4
    • Resolution: Fixed
    • Affects Version/s: 9
    • Fix Version/s: 9
    • Component/s: hotspot
    • Labels:
      None
    • Subcomponent:
    • Resolved In Build:
      b82

      Backports

        Description

        There is a corner case in String optimizations that is missing. It's a questionable programming practice, since the code sequence should just be replaced by "new String(s)", or even "s" assuming the new identity is not required. However, it might be trivial to support in OptimizeStringConcat?

        @State(Scope.Benchmark)
        @BenchmarkMode(Mode.AverageTime)
        @OutputTimeUnit(TimeUnit.NANOSECONDS)
        @Warmup(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
        @Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
        @Fork(3)
        public class ConcatSimpleBench {

            @Param({"1", "64", "4096"})
            int size;

            private String s;

            @Setup
            public void setup() {
                s = "";
                for (int c = 0; c < size; c++) {
                    s += "a";
                }
            }

            @Benchmark
            @CompilerControl(CompilerControl.Mode.DONT_INLINE)
            public String base() {
                return new String(s);
            }

            @Benchmark
            @CompilerControl(CompilerControl.Mode.DONT_INLINE)
            public String sb() {
                return new StringBuilder().append(s).toString();
            }
        }

        Yields:

        Benchmark (size) Mode Cnt Score Error Units
        ConcatSimpleBench.base 1 avgt 15 4.971 ± 0.992 ns/op
        ConcatSimpleBench.base 64 avgt 15 4.972 ± 1.044 ns/op
        ConcatSimpleBench.base 4096 avgt 15 5.249 ± 1.045 ns/op
        ConcatSimpleBench.sb 1 avgt 15 11.267 ± 2.628 ns/op
        ConcatSimpleBench.sb 64 avgt 15 16.219 ± 0.296 ns/op
        ConcatSimpleBench.sb 4096 avgt 15 814.078 ± 12.745 ns/op

        "base" tests experiences constant-time performance (the char[] array is just shared). "sb" test experiences close to O(n), since it copies the backing char[] storage.

          Issue Links

            Activity

            Show
            shade Aleksey Shipilev added a comment - Benchmark:   http://cr.openjdk.java.net/~shade/8076758/ConcatSimpleBench.java Executable JAR:   http://cr.openjdk.java.net/~shade/8076758/benchmarks.jar
            Hide
            shade Aleksey Shipilev added a comment -
            Proof-of-concept patch that solves the issue in a benchmark:
              http://cr.openjdk.java.net/~shade/8076758/webrev.00/

            I'm not entirely sure it works in a face of deoptimization.
            Show
            shade Aleksey Shipilev added a comment - Proof-of-concept patch that solves the issue in a benchmark:    http://cr.openjdk.java.net/~shade/8076758/webrev.00/ I'm not entirely sure it works in a face of deoptimization.
            Hide
            shade Aleksey Shipilev added a comment - - edited
            This issue becomes important for Indify String Concat (JDK-8085796) work, that may routinely produce expressions like this, as the intermediate computation.
            Show
            shade Aleksey Shipilev added a comment - - edited This issue becomes important for Indify String Concat ( JDK-8085796 ) work, that may routinely produce expressions like this, as the intermediate computation.
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/36208147039b
            User: thartmann
            Date: 2015-09-01 11:45:36 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/36208147039b User: thartmann Date: 2015-09-01 11:45:36 +0000
            Hide
            hgupdate HG Updates added a comment -
            URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/36208147039b
            User: lana
            Date: 2015-09-16 20:45:12 +0000
            Show
            hgupdate HG Updates added a comment - URL: http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/36208147039b User: lana Date: 2015-09-16 20:45:12 +0000

              People

              • Assignee:
                shade Aleksey Shipilev
                Reporter:
                shade Aleksey Shipilev
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: