diff --git a/src/hotspot/share/opto/stringopts.cpp b/src/hotspot/share/opto/stringopts.cpp index f867fcf4d23..793145e078d 100644 --- a/src/hotspot/share/opto/stringopts.cpp +++ b/src/hotspot/share/opto/stringopts.cpp @@ -173,9 +173,6 @@ class StringConcat : public ResourceObj { assert(!_control.contains(ctrl), "only push once"); _control.push(ctrl); } - bool has_control(Node* ctrl) { - return _control.contains(ctrl); - } void add_constructor(Node* init) { assert(!_constructors.contains(init), "only push once"); _constructors.push(init); @@ -410,66 +407,7 @@ Node_List PhaseStringOpts::collect_toString_calls() { return string_calls; } -PhaseStringOpts::ProcessAppendResult PhaseStringOpts::process_append_candidate(CallStaticJavaNode* cnode, - StringConcat* sc, - ciMethod* m, - ciSymbol* string_sig, - ciSymbol* int_sig, - ciSymbol* char_sig) { - if (cnode->method() != nullptr && !cnode->method()->is_static() && - cnode->method()->holder() == m->holder() && - cnode->method()->name() == ciSymbols::append_name() && - (cnode->method()->signature()->as_symbol() == string_sig || - cnode->method()->signature()->as_symbol() == char_sig || - cnode->method()->signature()->as_symbol() == int_sig)) { - if (sc->has_control(cnode)) { - return ProcessAppendResult::AppendWasAdded; - } - sc->add_control(cnode); - Node* arg = cnode->in(TypeFunc::Parms + 1); - if (arg == nullptr || arg->is_top()) { -#ifndef PRODUCT - if (PrintOptimizeStringConcat) { - tty->print("giving up because the call is effectively dead"); - cnode->jvms()->dump_spec(tty); - tty->cr(); - } -#endif - return ProcessAppendResult::AbortOptimization; - } - - if (cnode->method()->signature()->as_symbol() == int_sig) { - sc->push_int(arg); - } else if (cnode->method()->signature()->as_symbol() == char_sig) { - sc->push_char(arg); - } else if (arg->is_Proj() && arg->in(0)->is_CallStaticJava()) { - CallStaticJavaNode* csj = arg->in(0)->as_CallStaticJava(); - if (csj->method() != nullptr && - csj->method()->intrinsic_id() == vmIntrinsics::_Integer_toString && - arg->outcnt() == 1) { - // _control is the list of StringBuilder calls nodes which - // will be replaced by new String code after this optimization. - // Integer::toString() call is not part of StringBuilder calls - // chain. It could be eliminated only if its result is used - // only by this SB calls chain. - // Another limitation: it should be used only once because - // it is unknown that it is used only by this SB calls chain - // until all related SB calls nodes are collected. - assert(arg->unique_out() == cnode, "sanity"); - sc->add_control(csj); - sc->push_int(csj->in(TypeFunc::Parms)); - } else { - sc->push_string(arg); - } - } else { - sc->push_string(arg); - } - return ProcessAppendResult::AppendWasAdded; - } - return ProcessAppendResult::CandidateIsNotAppend; -} - -// Recognize fluent-chain and non-fluent uses of StringBuilder/Buffer. They are either explicit usages +// Recognize a fluent-chain of StringBuilder/Buffer. They are either explicit usages // of them or the legacy bytecodes of string concatenation prior to JEP-280. eg. // // String result = new StringBuilder() @@ -478,17 +416,18 @@ PhaseStringOpts::ProcessAppendResult PhaseStringOpts::process_append_candidate(C // .append(123) // .toString(); // "foobar123" // -// Fluent-chains are recognized by walking upwards along the receivers, starting from toString(). -// Once the allocation of the StringBuilder has been reached, DU pairs are examined to find the -// constructor and non-fluent uses of the StringBuilder such as in this example: +// PS: Only a certain subset of constructor and append methods are acceptable. +// The criterion is that the length of argument is easy to work out in this phrase. +// It will drop complex cases such as Object. // +// Since it walks along the receivers of fluent-chain, it will give up if the codeshape is +// not "fluent" enough. eg. // StringBuilder sb = new StringBuilder(); // sb.append("foo"); // sb.toString(); // -// PS: Only a certain subset of constructor and append methods are acceptable. -// The criterion is that the length of argument is easy to work out in this phrase. -// It will drop complex cases such as Object. +// The receiver of toString method is the result of Allocation Node(CheckCastPP). +// The append method is overlooked. It will fail at validate_control_flow() test. // StringConcat* PhaseStringOpts::build_candidate(CallStaticJavaNode* call) { ciMethod* m = call->method(); @@ -527,7 +466,7 @@ StringConcat* PhaseStringOpts::build_candidate(CallStaticJavaNode* call) { if (cnode == nullptr) { alloc = recv->isa_Allocate(); if (alloc == nullptr) { - return nullptr; + break; } // Find the constructor call Node* result = alloc->result_cast(); @@ -539,7 +478,7 @@ StringConcat* PhaseStringOpts::build_candidate(CallStaticJavaNode* call) { alloc->jvms()->dump_spec(tty); tty->cr(); } #endif - return nullptr; + break; } Node* constructor = nullptr; for (SimpleDUIterator i(result); i.has_next(); i.next()) { @@ -550,10 +489,6 @@ StringConcat* PhaseStringOpts::build_candidate(CallStaticJavaNode* call) { use->method()->name() == ciSymbols::object_initializer_name() && use->method()->holder() == m->holder()) { // Matched the constructor. - if (constructor != nullptr) { - // The constructor again. We must only process it once. - continue; - } ciSymbol* sig = use->method()->signature()->as_symbol(); if (sig == ciSymbols::void_method_signature() || sig == ciSymbols::int_void_signature() || @@ -607,16 +542,7 @@ StringConcat* PhaseStringOpts::build_candidate(CallStaticJavaNode* call) { } #endif } - } else if (use != nullptr) { - if (process_append_candidate(use, sc, m, string_sig, int_sig, char_sig) == ProcessAppendResult::AbortOptimization) { - // We must abort if process_append_candidate tells us to... - return nullptr; - } - // ...but we do not care if we really found an append or not: - // - If we found an append, that's perfect. Nothing further to do. - // - If this is a call to an unrelated method, validate_mem_flow() (and validate_control_flow()) - // will later check if this call prevents the optimization. So nothing to do here. - // We will continue to look for the constructor (if not found already) and appends. + break; } } if (constructor == nullptr) { @@ -627,7 +553,7 @@ StringConcat* PhaseStringOpts::build_candidate(CallStaticJavaNode* call) { alloc->jvms()->dump_spec(tty); tty->cr(); } #endif - return nullptr; + break; } // Walked all the way back and found the constructor call so see @@ -642,23 +568,62 @@ StringConcat* PhaseStringOpts::build_candidate(CallStaticJavaNode* call) { } else { return nullptr; } - } else { - ProcessAppendResult result = process_append_candidate(cnode, sc, m, string_sig, int_sig, char_sig); - - if (result == ProcessAppendResult::AbortOptimization) { - return nullptr; - } else if (result == ProcessAppendResult::CandidateIsNotAppend) { - // some unhandled signature + } else if (cnode->method() == nullptr) { + break; + } else if (!cnode->method()->is_static() && + cnode->method()->holder() == m->holder() && + cnode->method()->name() == ciSymbols::append_name() && + (cnode->method()->signature()->as_symbol() == string_sig || + cnode->method()->signature()->as_symbol() == char_sig || + cnode->method()->signature()->as_symbol() == int_sig)) { + sc->add_control(cnode); + Node* arg = cnode->in(TypeFunc::Parms + 1); + if (arg == nullptr || arg->is_top()) { #ifndef PRODUCT if (PrintOptimizeStringConcat) { - tty->print("giving up because encountered unexpected signature "); - cnode->tf()->dump(); - tty->cr(); - cnode->in(TypeFunc::Parms + 1)->dump(); + tty->print("giving up because the call is effectively dead"); + cnode->jvms()->dump_spec(tty); tty->cr(); } #endif - return nullptr; + break; } + if (cnode->method()->signature()->as_symbol() == int_sig) { + sc->push_int(arg); + } else if (cnode->method()->signature()->as_symbol() == char_sig) { + sc->push_char(arg); + } else { + if (arg->is_Proj() && arg->in(0)->is_CallStaticJava()) { + CallStaticJavaNode* csj = arg->in(0)->as_CallStaticJava(); + if (csj->method() != nullptr && + csj->method()->intrinsic_id() == vmIntrinsics::_Integer_toString && + arg->outcnt() == 1) { + // _control is the list of StringBuilder calls nodes which + // will be replaced by new String code after this optimization. + // Integer::toString() call is not part of StringBuilder calls + // chain. It could be eliminated only if its result is used + // only by this SB calls chain. + // Another limitation: it should be used only once because + // it is unknown that it is used only by this SB calls chain + // until all related SB calls nodes are collected. + assert(arg->unique_out() == cnode, "sanity"); + sc->add_control(csj); + sc->push_int(csj->in(TypeFunc::Parms)); + continue; + } + } + sc->push_string(arg); + } + continue; + } else { + // some unhandled signature +#ifndef PRODUCT + if (PrintOptimizeStringConcat) { + tty->print("giving up because encountered unexpected signature "); + cnode->tf()->dump(); tty->cr(); + cnode->in(TypeFunc::Parms + 1)->dump(); + } +#endif + break; } } return nullptr; diff --git a/src/hotspot/share/opto/stringopts.hpp b/src/hotspot/share/opto/stringopts.hpp index 99d554838d7..21be4109c7d 100644 --- a/src/hotspot/share/opto/stringopts.hpp +++ b/src/hotspot/share/opto/stringopts.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2025, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -34,7 +34,7 @@ class IdealVariable; class PhaseStringOpts : public Phase { friend class StringConcat; -private: + private: PhaseGVN* _gvn; // List of dead nodes to clean up aggressively at the end @@ -53,23 +53,6 @@ private: // a single string construction. StringConcat* build_candidate(CallStaticJavaNode* call); - enum class ProcessAppendResult { - // Indicates that the candidate was indeed an append and process_append_candidate processed it - // accordingly (added it to the StringConcat etc.) - AppendWasAdded, - // The candidate turned out not to be an append call. process_append_candidate did not do anything. - CandidateIsNotAppend, - // The candidate is an append call, but circumstances completely preventing string concat - // optimization were detected and the optimization must abort. - AbortOptimization - }; - - // Called from build_candidate. Looks at an "append candidate", a call that might be a call - // to StringBuilder::append. If so, adds it to the StringConcat. - ProcessAppendResult process_append_candidate(CallStaticJavaNode* cnode, StringConcat* sc, - ciMethod* m, ciSymbol* string_sig, ciSymbol* int_sig, - ciSymbol* char_sig); - // Replace all the SB calls in concat with an optimization String allocation void replace_string_concat(StringConcat* concat); @@ -122,13 +105,12 @@ private: unroll_string_copy_length = 6 }; -public: + public: PhaseStringOpts(PhaseGVN* gvn); #ifndef PRODUCT static void print_statistics(); - -private: + private: static uint _stropts_replaced; static uint _stropts_merged; static uint _stropts_total; diff --git a/test/hotspot/jtreg/compiler/stringopts/TestFluidAndNonFluid.java b/test/hotspot/jtreg/compiler/stringopts/TestFluidAndNonFluid.java deleted file mode 100644 index 34fbaa1b6ed..00000000000 --- a/test/hotspot/jtreg/compiler/stringopts/TestFluidAndNonFluid.java +++ /dev/null @@ -1,134 +0,0 @@ -/* - * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - - /* - * @test - * @bug 8341696 - * @summary Allow C2 to also optimize non-fluid string builder calls. - * @library /test/lib / - * @run driver compiler.c2.irTests.stringopts.TestFluidAndNonFluid - */ -package compiler.c2.irTests.stringopts; - -import compiler.lib.ir_framework.*; -import jdk.test.lib.Asserts; - -public class TestFluidAndNonFluid { - - public static int unknown = 1; - - public static void main(String[] args) { - // Dont inline any StringBuilder methods for this IR test to check if string opts are applied or not. - TestFramework.runWithFlags("-XX:CompileCommand=dontinline,java.lang.StringBuilder::*"); - } - - @DontInline - public static void opaque(StringBuilder builder) { - builder.append("Z"); - } - - @Run(test = {"fluid", "nonFluid", "nonFinal", "nonFluidExtraneousVariable", "nonFluidConditional", - "nonFluidOpaqueCall"}) - public void runMethod() { - Asserts.assertEQ("0ac", fluidNoParam()); - Asserts.assertEQ("ac", nonFluidNoParam()); - Asserts.assertEQ("ac", fluid("c")); - Asserts.assertEQ("ac", nonFluid("c")); - Asserts.assertEQ("ac", nonFinal("c")); - Asserts.assertEQ("ac", nonFluidExtraneousVariable("c")); - Asserts.assertEQ("ac", nonFluidConditional("c")); - Asserts.assertEQ("aZ", nonFluidOpaqueCall()); - } - - @Test - @IR(failOn = {IRNode.ALLOC_OF, "StringBuilder", IRNode.CALL_OF_METHOD, "toString", IRNode.INTRINSIC_TRAP}) - public static String fluidNoParam() { - return new StringBuilder("0").append("a").append("c").toString(); - } - - @Test - @IR(failOn = {IRNode.ALLOC_OF, "StringBuilder", IRNode.CALL_OF_METHOD, "toString", IRNode.INTRINSIC_TRAP}) - public static String nonFluidNoParam() { - final StringBuilder sb = new StringBuilder(); - sb.append("a"); - sb.append("c"); - return sb.toString(); - } - - @Test - @IR(failOn = {IRNode.ALLOC_OF, "StringBuilder", IRNode.CALL_OF_METHOD, "toString"}) - public static String fluid(String a) { - return new StringBuilder().append("a").append(a).toString(); - } - - @Test - @IR(failOn = {IRNode.ALLOC_OF, "StringBuilder", IRNode.CALL_OF_METHOD, "toString"}) - public static String nonFluid(String a) { - final StringBuilder sb = new StringBuilder(); - sb.append("a"); - sb.append(a); - return sb.toString(); - } - - @Test - @IR(failOn = {IRNode.ALLOC_OF, "StringBuilder", IRNode.CALL_OF_METHOD, "toString"}) - public static String nonFinal(String a) { - StringBuilder sb = new StringBuilder(); - sb.append("a"); - sb.append(a); - return sb.toString(); - } - - @Test - @IR(failOn = {IRNode.ALLOC_OF, "StringBuilder", IRNode.CALL_OF_METHOD, "toString"}) - public static String nonFluidExtraneousVariable(String a) { - final StringBuilder sb = new StringBuilder(); - final StringBuilder x = sb; - sb.append("a"); - x.append(a); - return sb.toString(); - } - - @Test - @IR(counts = {IRNode.ALLOC_OF, "StringBuilder", "1", IRNode.CALL_OF_METHOD, "toString", "1"}) - @IR(failOn = IRNode.INTRINSIC_TRAP) - static String nonFluidConditional(String a) { - final StringBuilder sb = new StringBuilder(); - sb.append("a"); - if (unknown == 1) { - sb.append(a); - } - return sb.toString(); - } - - @Test - @IR(counts = {IRNode.ALLOC_OF, "StringBuilder", "1", IRNode.CALL_OF_METHOD, "toString", "1"}) - @IR(failOn = IRNode.INTRINSIC_TRAP) - static String nonFluidOpaqueCall() { - final StringBuilder sb = new StringBuilder(); - sb.append("a"); - opaque(sb); - return sb.toString(); - } - -} diff --git a/test/micro/org/openjdk/bench/vm/compiler/FluidSBBench.java b/test/micro/org/openjdk/bench/vm/compiler/FluidSBBench.java deleted file mode 100644 index 794ff768678..00000000000 --- a/test/micro/org/openjdk/bench/vm/compiler/FluidSBBench.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Copyright Amazon.com Inc. or its affiliates. All Rights Reserved. - * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - * - */ - -package org.openjdk.bench.vm.compiler; - -import org.openjdk.jmh.annotations.Benchmark; -import org.openjdk.jmh.annotations.BenchmarkMode; -import org.openjdk.jmh.annotations.Fork; -import org.openjdk.jmh.annotations.Measurement; -import org.openjdk.jmh.annotations.Mode; -import org.openjdk.jmh.annotations.OutputTimeUnit; -import org.openjdk.jmh.annotations.Warmup; -import org.openjdk.jmh.annotations.State; -import org.openjdk.jmh.annotations.Scope; -import java.util.concurrent.TimeUnit; - -@Warmup(iterations = 3, time = 300, timeUnit = TimeUnit.MILLISECONDS) -@Measurement(iterations = 3, time = 300, timeUnit = TimeUnit.MILLISECONDS) -@Fork(value = 1, jvmArgsAppend = {"-XX:+UseParallelGC", "-Xmx1g", "-Xms1g"}) -@BenchmarkMode(Mode.AverageTime) -@OutputTimeUnit(TimeUnit.NANOSECONDS) -@State(Scope.Thread) -public class FluidSBBench { - static final String PREFIX = "a"; - String foo = "aaaaa aaaaa aaaaa aaaaa aaaaa"; - - @Benchmark - public String fluid() { - return new StringBuilder().append(PREFIX).append(foo).toString(); - } - - @Benchmark - public String nonFluid() { - final StringBuilder sb = new StringBuilder(); - sb.append(PREFIX); - sb.append(foo); - return sb.toString(); - } -}