diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e80c939377..fe090b635e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ Bug fixes: * Fix `rb_global_variable()` for `Float` and bignum values during the `Init_` function (#3478, @eregon). +Performance: +* Fix inline caching for Regexp creation from Strings (#3492, @andrykonchin, @eregon). + # 24.0.0 New features: diff --git a/spec/tags/truffle/splitting_tags.txt b/spec/tags/truffle/splitting_tags.txt index 74b1e281162..d9d526eadc4 100644 --- a/spec/tags/truffle/splitting_tags.txt +++ b/spec/tags/truffle/splitting_tags.txt @@ -1 +1,2 @@ slow:Critical methods whic must split are under 100 AST nodes +slow:Critical methods which must split are under 100 AST nodes diff --git a/spec/truffle/regexp/inline_caching_spec.rb b/spec/truffle/regexp/inline_caching_spec.rb new file mode 100644 index 00000000000..4b935c6b900 --- /dev/null +++ b/spec/truffle/regexp/inline_caching_spec.rb @@ -0,0 +1,340 @@ +# truffleruby_primitives: true + +# Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved. This +# code is released under a tri EPL/GPL/LGPL license. You can use it, +# redistribute it and/or modify it under the terms of the: +# +# Eclipse Public License version 2.0, or +# GNU General Public License version 2, or +# GNU Lesser General Public License version 2.1. + +require_relative '../../ruby/spec_helper' + +# This test requires splitting (--engine.Splitting) which is only available with the OptimizedTruffleRuntime. +# It fails under --experimental-engine-caching because CallInternalMethodNode does not have cached specializations for +# !isSingleContext() and so ends up using an IndirectCallNode which prevents splitting. +guard -> { TruffleRuby.jit? && !Truffle::Boot.get_option('experimental-engine-caching') && Truffle::Boot.get_option("default-cache") != 0 } do + describe "Inline caching for dynamically-created Regexp works for" do + before :each do + @performance_warnings, Warning[:performance] = Warning[:performance], true + end + + after :each do + Warning[:performance] = @performance_warnings + end + + it "Regexp.new" do + # Check that separate call sites with fixed input does not warn + -> { + Regexp.new("a") + Regexp.new("b") + Regexp.new("c") + Regexp.new("d") + Regexp.new("e") + Regexp.new("f") + Regexp.new("g") + Regexp.new("h") + Regexp.new("i") + Regexp.new("j") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |pattern| + Regexp.new(pattern) + end + }.should complain(/unbounded creation of regexps/) + end + + it "Regexp.union with 1 argument" do + # Check that separate call sites with fixed input do not warn + -> { + Regexp.union("a") + Regexp.union("b") + Regexp.union("c") + Regexp.union("d") + Regexp.union("e") + Regexp.union("f") + Regexp.union("g") + Regexp.union("h") + Regexp.union("i") + Regexp.union("j") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |pattern| + Regexp.union(pattern) + end + }.should complain(/unbounded creation of regexps/) + end + + it "Regexp.union with multiple arguments" do + # Check that separate call sites with fixed input do not warn + -> { + Regexp.union("h", "a") + Regexp.union("h", "b") + Regexp.union("h", "c") + Regexp.union("h", "d") + Regexp.union("h", "e") + Regexp.union("h", "f") + Regexp.union("h", "g") + Regexp.union("h", "h") + Regexp.union("h", "i") + Regexp.union("h", "j") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |pattern| + Regexp.union("h", pattern) + end + }.should complain(/unbounded creation of regexps/) + end + + it "interpolated Regexp" do + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |pattern| + /#{pattern}/ + end + }.should complain(/unstable interpolated regexps/) + end + + it "String#scan" do + # Check that separate call sites with fixed input do not warn + -> { + "zzz".scan("a") + "zzz".scan("b") + "zzz".scan("c") + "zzz".scan("d") + "zzz".scan("e") + "zzz".scan("f") + "zzz".scan("g") + "zzz".scan("h") + "zzz".scan("i") + "zzz".scan("j") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + # "a".."z" and not just "a".."j" because there can be some late heuristic megamorphic splitting by TRegex (ExecCompiledRegexNode) + ("a".."z").each do |pattern| + "zzz".scan(pattern) + end + }.should complain(/unbounded creation of regexps/) + end + + it "String#sub" do + # Don't use String explicitly to trigger Truffle::Type.coerce_to_regexp. String argument is handled with + # Primitive.matchdata_create_single_group and isn't converted to Regexp immediately. + pattern = Class.new do + def initialize(string) = @string = string + def to_str = @string + end + + # Check that separate call sites with fixed input do not warn + -> { + "zzz".sub(pattern.new("a"), "replacement") + "zzz".sub(pattern.new("b"), "replacement") + "zzz".sub(pattern.new("c"), "replacement") + "zzz".sub(pattern.new("d"), "replacement") + "zzz".sub(pattern.new("e"), "replacement") + "zzz".sub(pattern.new("f"), "replacement") + "zzz".sub(pattern.new("g"), "replacement") + "zzz".sub(pattern.new("h"), "replacement") + "zzz".sub(pattern.new("i"), "replacement") + "zzz".sub(pattern.new("j"), "replacement") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |s| + "zzz".sub(pattern.new(s), "replacement") + end + }.should complain(/unbounded creation of regexps/) + end + + it "String#sub!" do + # Don't use String explicitly to trigger Truffle::Type.coerce_to_regexp. String argument is handled with + # Primitive.matchdata_create_single_group and isn't converted to Regexp immediately. + pattern = Class.new do + def initialize(string) = @string = string + def to_str = @string + end + + # Check that separate call sites with fixed input do not warn + -> { + "zzz".sub!(pattern.new("a"), "replacement") + "zzz".sub!(pattern.new("b"), "replacement") + "zzz".sub!(pattern.new("c"), "replacement") + "zzz".sub!(pattern.new("d"), "replacement") + "zzz".sub!(pattern.new("e"), "replacement") + "zzz".sub!(pattern.new("f"), "replacement") + "zzz".sub!(pattern.new("g"), "replacement") + "zzz".sub!(pattern.new("h"), "replacement") + "zzz".sub!(pattern.new("i"), "replacement") + "zzz".sub!(pattern.new("j"), "replacement") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |s| + "zzz".sub!(pattern.new(s), "replacement") + end + }.should complain(/unbounded creation of regexps/) + end + + it "String#gsub" do + # Don't use String explicitly to trigger Truffle::Type.coerce_to_regexp. String argument is handled with + # Primitive.matchdata_create_single_group and isn't converted to Regexp immediately. + pattern = Class.new do + def initialize(string) = @string = string + def to_str = @string + end + + # Check that separate call sites with fixed input do not warn + -> { + "zzz".gsub(pattern.new("a"), "replacement") + "zzz".gsub(pattern.new("b"), "replacement") + "zzz".gsub(pattern.new("c"), "replacement") + "zzz".gsub(pattern.new("d"), "replacement") + "zzz".gsub(pattern.new("e"), "replacement") + "zzz".gsub(pattern.new("f"), "replacement") + "zzz".gsub(pattern.new("g"), "replacement") + "zzz".gsub(pattern.new("h"), "replacement") + "zzz".gsub(pattern.new("i"), "replacement") + "zzz".gsub(pattern.new("j"), "replacement") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |s| + "zzz".gsub(pattern.new(s), "replacement") + end + }.should complain(/unbounded creation of regexps/) + end + + it "String#gsub!" do + # Don't use String explicitly to trigger Truffle::Type.coerce_to_regexp. String argument is handled with + # Primitive.matchdata_create_single_group and isn't converted to Regexp immediately. + pattern = Class.new do + def initialize(string) = @string = string + def to_str = @string + end + + # Check that separate call sites with fixed input do not warn + -> { + "zzz".gsub!(pattern.new("a"), "replacement") + "zzz".gsub!(pattern.new("b"), "replacement") + "zzz".gsub!(pattern.new("c"), "replacement") + "zzz".gsub!(pattern.new("d"), "replacement") + "zzz".gsub!(pattern.new("e"), "replacement") + "zzz".gsub!(pattern.new("f"), "replacement") + "zzz".gsub!(pattern.new("g"), "replacement") + "zzz".gsub!(pattern.new("h"), "replacement") + "zzz".gsub!(pattern.new("i"), "replacement") + "zzz".gsub!(pattern.new("j"), "replacement") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |s| + "zzz".gsub!(pattern.new(s), "replacement") + end + }.should complain(/unbounded creation of regexps/) + end + + it "String#match" do + # Check that separate call sites with fixed input do not warn + -> { + "zzz".match("a") + "zzz".match("b") + "zzz".match("c") + "zzz".match("d") + "zzz".match("e") + "zzz".match("f") + "zzz".match("g") + "zzz".match("h") + "zzz".match("i") + "zzz".match("j") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |pattern| + "zzz".match(pattern) + end + }.should complain(/unbounded creation of regexps/) + end + + it "String#match?" do + # Check that separate call sites with fixed input do not warn + -> { + "zzz".match?("a") + "zzz".match?("b") + "zzz".match?("c") + "zzz".match?("d") + "zzz".match?("e") + "zzz".match?("f") + "zzz".match?("g") + "zzz".match?("h") + "zzz".match?("i") + "zzz".match?("j") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |pattern| + "zzz".match?(pattern) + end + }.should complain(/unbounded creation of regexps/) + end + + it "Symbol#match" do + # Check that separate call sites with fixed input do not warn + -> { + :zzz.match("a") + :zzz.match("b") + :zzz.match("c") + :zzz.match("d") + :zzz.match("e") + :zzz.match("f") + :zzz.match("g") + :zzz.match("h") + :zzz.match("i") + :zzz.match("j") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |pattern| + :zzz.match(pattern) + end + }.should complain(/unbounded creation of regexps/) + end + + it "Symbol#match?" do + # Check that separate call sites with fixed input do not warn + -> { + :zzz.match?("a") + :zzz.match?("b") + :zzz.match?("c") + :zzz.match?("d") + :zzz.match?("e") + :zzz.match?("f") + :zzz.match?("g") + :zzz.match?("h") + :zzz.match?("i") + :zzz.match?("j") + }.should_not complain + + # Check that calling it with many different inputs has the warning + -> { + ("a".."z").each do |pattern| + :zzz.match?(pattern) + end + }.should complain(/unbounded creation of regexps/) + end + end +end diff --git a/spec/truffle/splitting_spec.rb b/spec/truffle/splitting_spec.rb index 15899c63c15..bbdf443d67b 100644 --- a/spec/truffle/splitting_spec.rb +++ b/spec/truffle/splitting_spec.rb @@ -8,7 +8,7 @@ require_relative '../ruby/spec_helper' -describe 'Critical methods whic must split' do +describe 'Critical methods which must split' do it 'are under 100 AST nodes' do code = <<-'EOF' require 'strscan' diff --git a/src/main/java/org/truffleruby/core/CoreLibrary.java b/src/main/java/org/truffleruby/core/CoreLibrary.java index cd34556b094..a07cb19e290 100644 --- a/src/main/java/org/truffleruby/core/CoreLibrary.java +++ b/src/main/java/org/truffleruby/core/CoreLibrary.java @@ -202,6 +202,7 @@ public final class CoreLibrary { public final RubyModule truffleRegexpOperationsModule; public final RubyModule truffleRandomOperationsModule; public final RubyModule truffleThreadOperationsModule; + public final RubyModule truffleWarningOperationsModule; public final RubyClass encodingCompatibilityErrorClass; public final RubyClass encodingUndefinedConversionErrorClass; public final RubyClass methodClass; @@ -508,6 +509,7 @@ public CoreLibrary(RubyContext context, RubyLanguage language) { defineModule(truffleModule, "ReadlineHistory"); truffleRandomOperationsModule = defineModule(truffleModule, "RandomOperations"); truffleThreadOperationsModule = defineModule(truffleModule, "ThreadOperations"); + truffleWarningOperationsModule = defineModule(truffleModule, "WarningOperations"); defineModule(truffleModule, "WeakRefOperations"); handleClass = defineClass(truffleModule, objectClass, "Handle"); warningModule = defineModule("Warning"); diff --git a/src/main/java/org/truffleruby/core/regexp/InterpolatedRegexpNode.java b/src/main/java/org/truffleruby/core/regexp/InterpolatedRegexpNode.java index a1791ef4362..9788efcf933 100644 --- a/src/main/java/org/truffleruby/core/regexp/InterpolatedRegexpNode.java +++ b/src/main/java/org/truffleruby/core/regexp/InterpolatedRegexpNode.java @@ -15,7 +15,7 @@ import org.truffleruby.core.encoding.RubyEncoding; import org.truffleruby.core.regexp.InterpolatedRegexpNodeFactory.RegexpBuilderNodeGen; import org.truffleruby.core.string.TStringWithEncoding; -import org.truffleruby.language.NotOptimizedWarningNode; +import org.truffleruby.language.PerformanceWarningNode; import org.truffleruby.language.RubyBaseNode; import org.truffleruby.language.RubyContextSourceNode; import org.truffleruby.language.RubyNode; @@ -103,8 +103,9 @@ Object fast(TStringWithEncoding[] parts, @Specialization(replaces = "fast") Object slow(TStringWithEncoding[] parts, - @Cached NotOptimizedWarningNode notOptimizedWarningNode) { - notOptimizedWarningNode.warn("unstable interpolated regexps are not optimized"); + @Cached PerformanceWarningNode performanceWarningNode) { + performanceWarningNode.warn( + "unstable interpolated regexps cause deoptimization loops which hurt performance significantly, avoid creating regexps dynamically where possible or cache them to fix this"); return createRegexp(parts); } diff --git a/src/main/java/org/truffleruby/core/regexp/RegexpNodes.java b/src/main/java/org/truffleruby/core/regexp/RegexpNodes.java index b094af4be43..ae467344750 100644 --- a/src/main/java/org/truffleruby/core/regexp/RegexpNodes.java +++ b/src/main/java/org/truffleruby/core/regexp/RegexpNodes.java @@ -13,11 +13,13 @@ import java.util.Iterator; import com.oracle.truffle.api.dsl.Bind; +import com.oracle.truffle.api.dsl.NeverDefault; import com.oracle.truffle.api.nodes.Node; import com.oracle.truffle.api.profiles.InlinedBranchProfile; import com.oracle.truffle.api.strings.TruffleString; import org.joni.NameEntry; import org.truffleruby.annotations.CoreMethod; +import org.truffleruby.annotations.Split; import org.truffleruby.builtins.CoreMethodArrayArgumentsNode; import org.truffleruby.annotations.CoreModule; import org.truffleruby.annotations.Primitive; @@ -25,21 +27,23 @@ import org.truffleruby.core.array.RubyArray; import org.truffleruby.core.cast.ToStrNode; import org.truffleruby.core.encoding.Encodings; +import org.truffleruby.core.encoding.RubyEncoding; import org.truffleruby.core.encoding.TStringUtils; import org.truffleruby.core.klass.RubyClass; import org.truffleruby.core.string.ATStringWithEncoding; +import org.truffleruby.core.string.StringHelperNodes; import org.truffleruby.core.string.TStringWithEncoding; import org.truffleruby.core.string.RubyString; import org.truffleruby.core.symbol.RubySymbol; import org.truffleruby.annotations.Visibility; +import org.truffleruby.language.PerformanceWarningNode; import org.truffleruby.language.control.DeferredRaiseException; import org.truffleruby.language.control.RaiseException; import org.truffleruby.language.library.RubyStringLibrary; -import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.dsl.Cached; -import com.oracle.truffle.api.dsl.Fallback; +import com.oracle.truffle.api.dsl.Cached.Shared; import com.oracle.truffle.api.dsl.Specialization; @CoreModule(value = "Regexp", isClass = true) @@ -54,41 +58,67 @@ int hash(RubyRegexp regexp) { } } - @CoreMethod(names = { "quote", "escape" }, onSingleton = true, required = 1) + @CoreMethod(names = { "quote", "escape" }, onSingleton = true, required = 1, split = Split.ALWAYS) public abstract static class QuoteNode extends CoreMethodArrayArgumentsNode { - @Child private QuoteNode quoteNode; - public abstract RubyString execute(Object raw); + @NeverDefault public static QuoteNode create() { return RegexpNodesFactory.QuoteNodeFactory.create(null); } - @Specialization(guards = "libRaw.isRubyString(raw)", limit = "1") + @SuppressWarnings("truffle-static-method") + @Specialization( + guards = { + "libString.isRubyString(raw)", + "equalNode.execute(node, libString, raw, cachedString, cachedEnc)" }, + limit = "getDefaultCacheLimit()") + RubyString quoteStringCached(Object raw, + @Cached @Shared RubyStringLibrary libString, + @Cached("asTruffleStringUncached(raw)") TruffleString cachedString, + @Cached("libString.getEncoding(raw)") RubyEncoding cachedEnc, + @Cached StringHelperNodes.EqualSameEncodingNode equalNode, + @Bind("this") Node node, + @Cached("quote(libString, raw)") TStringWithEncoding quotedString) { + return createString(quotedString); + } + + @Specialization(replaces = "quoteStringCached", guards = "libString.isRubyString(raw)") RubyString quoteString(Object raw, - @Cached RubyStringLibrary libRaw) { - return createString(ClassicRegexp.quote19(new ATStringWithEncoding(libRaw, raw))); + @Cached @Shared RubyStringLibrary libString) { + return createString(quote(libString, raw)); } - @Specialization + @Specialization(guards = "raw == cachedSymbol", limit = "getDefaultCacheLimit()") + RubyString quoteSymbolCached(RubySymbol raw, + @Cached("raw") RubySymbol cachedSymbol, + @Cached("quote(cachedSymbol)") TStringWithEncoding quotedString) { + return createString(quotedString); + } + + @Specialization(replaces = "quoteSymbolCached") RubyString quoteSymbol(RubySymbol raw) { - return doQuoteString(createString(raw.tstring, raw.encoding)); + return createString(quote(raw)); } - @Fallback - RubyString quote(Object raw, - @Cached ToStrNode toStrNode) { - return doQuoteString(toStrNode.execute(this, raw)); + @Specialization(guards = { "!libString.isRubyString(raw)", "!isRubySymbol(raw)" }) + static RubyString quoteGeneric(Object raw, + @Cached @Shared RubyStringLibrary libString, + @Cached ToStrNode toStrNode, + @Cached QuoteNode recursive, + @Bind("this") Node node) { + return recursive.execute(toStrNode.execute(node, raw)); } - private RubyString doQuoteString(Object raw) { - if (quoteNode == null) { - CompilerDirectives.transferToInterpreterAndInvalidate(); - quoteNode = insert(QuoteNode.create()); - } - return quoteNode.execute(raw); + TStringWithEncoding quote(RubyStringLibrary strings, Object string) { + return ClassicRegexp.quote19(new ATStringWithEncoding(strings, string)); } + + TStringWithEncoding quote(RubySymbol symbol) { + return ClassicRegexp.quote19(new ATStringWithEncoding(symbol.tstring, symbol.encoding)); + } + } @CoreMethod(names = "source") @@ -180,12 +210,39 @@ boolean fixedEncoding(RubyRegexp regexp) { @Primitive(name = "regexp_compile", lowerFixnum = 1) public abstract static class RegexpCompileNode extends PrimitiveArrayArgumentsNode { - @Specialization(guards = "libPattern.isRubyString(pattern)", limit = "1") - static RubyRegexp initialize(Object pattern, int options, + static final InlinedBranchProfile UNCACHED_BRANCH_PROFILE = InlinedBranchProfile.getUncached(); + + @Specialization( + guards = { + "libPattern.isRubyString(pattern)", + "patternEqualNode.execute(node, libPattern, pattern, cachedPattern, cachedPatternEnc)", + "options == cachedOptions" }, + limit = "getDefaultCacheLimit()") + static RubyRegexp fastCompiling(Object pattern, int options, + @Cached @Shared TruffleString.AsTruffleStringNode asTruffleStringNode, + @Cached @Shared RubyStringLibrary libPattern, + @Cached("asTruffleStringUncached(pattern)") TruffleString cachedPattern, + @Cached("libPattern.getEncoding(pattern)") RubyEncoding cachedPatternEnc, + @Cached("options") int cachedOptions, + @Cached StringHelperNodes.EqualSameEncodingNode patternEqualNode, + @Bind("this") Node node, + @Cached("compile(pattern, options, node, libPattern, asTruffleStringNode, UNCACHED_BRANCH_PROFILE)") RubyRegexp regexp) { + return regexp; + } + + @Specialization(replaces = "fastCompiling", guards = "libPattern.isRubyString(pattern)") + RubyRegexp slowCompiling(Object pattern, int options, @Cached InlinedBranchProfile errorProfile, - @Cached TruffleString.AsTruffleStringNode asTruffleStringNode, - @Cached RubyStringLibrary libPattern, - @Bind("this") Node node) { + @Cached @Shared TruffleString.AsTruffleStringNode asTruffleStringNode, + @Cached @Shared RubyStringLibrary libPattern, + @Cached PerformanceWarningNode performanceWarningNode) { + performanceWarningNode.warn( + "unbounded creation of regexps causes deoptimization loops which hurt performance significantly, avoid creating regexps dynamically where possible or cache them to fix this"); + return compile(pattern, options, this, libPattern, asTruffleStringNode, errorProfile); + } + + public RubyRegexp compile(Object pattern, int options, Node node, RubyStringLibrary libPattern, + TruffleString.AsTruffleStringNode asTruffleStringNode, InlinedBranchProfile errorProfile) { var encoding = libPattern.getEncoding(pattern); try { return RubyRegexp.create( diff --git a/src/main/java/org/truffleruby/core/regexp/TruffleRegexpNodes.java b/src/main/java/org/truffleruby/core/regexp/TruffleRegexpNodes.java index bd334526280..60e979545fd 100644 --- a/src/main/java/org/truffleruby/core/regexp/TruffleRegexpNodes.java +++ b/src/main/java/org/truffleruby/core/regexp/TruffleRegexpNodes.java @@ -45,6 +45,7 @@ import org.truffleruby.RubyContext; import org.truffleruby.RubyLanguage; import org.truffleruby.annotations.CoreMethod; +import org.truffleruby.annotations.Split; import org.truffleruby.builtins.CoreMethodArrayArgumentsNode; import org.truffleruby.annotations.CoreModule; import org.truffleruby.annotations.Primitive; @@ -69,6 +70,7 @@ import org.truffleruby.core.string.StringUtils; import org.truffleruby.interop.TranslateInteropExceptionNode; import org.truffleruby.language.LazyWarnNode; +import org.truffleruby.language.PerformanceWarningNode; import org.truffleruby.language.RubyBaseNode; import org.truffleruby.language.RubyGuards; import org.truffleruby.language.WarnNode; @@ -283,7 +285,7 @@ private static Regex makeRegexpForEncoding(RubyContext context, RubyRegexp regex } } - @CoreMethod(names = "union", onSingleton = true, required = 2, rest = true) + @CoreMethod(names = "union", onSingleton = true, required = 2, rest = true, split = Split.ALWAYS) public abstract static class RegexpUnionNode extends CoreMethodArrayArgumentsNode { static final InlinedBranchProfile UNCACHED_BRANCH_PROFILE = InlinedBranchProfile.getUncached(); @@ -307,7 +309,11 @@ static Object fastUnion(VirtualFrame frame, RubyString str, Object sep, Object[] @Specialization(replaces = "fastUnion") Object slowUnion(RubyString str, Object sep, Object[] args, + @Cached PerformanceWarningNode performanceWarningNode, @Cached InlinedBranchProfile errorProfile) { + performanceWarningNode.warn( + "unbounded creation of regexps causes deoptimization loops which hurt performance significantly, avoid creating regexps dynamically where possible or cache them to fix this"); + return buildUnion(str, sep, args, errorProfile); } diff --git a/src/main/java/org/truffleruby/extra/TruffleGraalNodes.java b/src/main/java/org/truffleruby/extra/TruffleGraalNodes.java index 99f2c98b838..ad9e46f6e30 100644 --- a/src/main/java/org/truffleruby/extra/TruffleGraalNodes.java +++ b/src/main/java/org/truffleruby/extra/TruffleGraalNodes.java @@ -163,7 +163,7 @@ private void notConstantBoundary() { } @Primitive(name = "assert_not_compiled") - public abstract static class AssertNotCompilationConstantNode extends PrimitiveNode { + public abstract static class AssertNotCompiledNode extends PrimitiveNode { @Specialization Object assertNotCompiled() { diff --git a/src/main/java/org/truffleruby/language/NotOptimizedWarningNode.java b/src/main/java/org/truffleruby/language/PerformanceWarningNode.java similarity index 66% rename from src/main/java/org/truffleruby/language/NotOptimizedWarningNode.java rename to src/main/java/org/truffleruby/language/PerformanceWarningNode.java index e318fed2dde..4a459220946 100644 --- a/src/main/java/org/truffleruby/language/NotOptimizedWarningNode.java +++ b/src/main/java/org/truffleruby/language/PerformanceWarningNode.java @@ -11,36 +11,26 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.logging.Level; -import com.oracle.truffle.api.dsl.NeverDefault; -import org.truffleruby.RubyLanguage; +import org.truffleruby.RubyContext; +import org.truffleruby.core.encoding.Encodings; +import org.truffleruby.language.globals.ReadSimpleGlobalVariableNode; import com.oracle.truffle.api.CompilerAsserts; -import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; -import com.oracle.truffle.api.dsl.GenerateUncached; import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.nodes.ControlFlowException; import com.oracle.truffle.api.source.SourceSection; +import com.oracle.truffle.api.strings.TruffleString; -/** Displays a warning when code is compiled that will compile successfully but is very low performance. We don't want - * to bail out, as this isn't visible to users - we want them to see if they're using code like this in something like a - * benchmark. - * - * Ideally you should not use this node, and instead you should optimise the code which would use it. */ -@GenerateUncached -public abstract class NotOptimizedWarningNode extends RubyBaseNode { - - @NeverDefault - public static NotOptimizedWarningNode create() { - return NotOptimizedWarningNodeGen.create(); - } +public abstract class PerformanceWarningNode extends RubyBaseNode { @SuppressWarnings("serial") protected static final class Warned extends ControlFlowException { } + @Child ReadSimpleGlobalVariableNode readVerboseNode = ReadSimpleGlobalVariableNode.create("$VERBOSE"); + public final void warn(String message) { executeWarn(message); } @@ -50,10 +40,10 @@ public final void warn(String message) { @Specialization(rewriteOn = Warned.class) void warnOnce(String message) throws Warned { // The message should be a constant, because we don't want to do anything expensive to create it - CompilerAsserts.compilationConstant(message); + CompilerAsserts.partialEvaluationConstant(message); - // If we're in the interpreter then don't warn - if (CompilerDirectives.inInterpreter()) { + // Do not warn if $VERBOSE is nil + if (readVerboseNode.execute() == nil) { return; } @@ -74,15 +64,18 @@ void doNotWarn(String message) { @TruffleBoundary private void log(String message) { // We want the topmost user source section, as otherwise lots of warnings will come from the same core methods - final SourceSection userSourceSection = getContext().getCallStack().getTopMostUserSourceSection(); + final SourceSection userSourceSection = getContext().getCallStack() + .getTopMostUserSourceSection(getEncapsulatingSourceSection()); final String displayedWarning = String.format( - "%s: %s", + "%s: warning: %s%n", getContext().fileLine(userSourceSection), message); if (DISPLAYED_WARNINGS.add(displayedWarning)) { - RubyLanguage.LOGGER.log(Level.WARNING, displayedWarning); + var warningString = createString(TruffleString.FromJavaStringNode.getUncached(), displayedWarning, + Encodings.US_ASCII); + RubyContext.send(this, coreLibrary().truffleWarningOperationsModule, "performance_warning", warningString); } } diff --git a/src/main/java/org/truffleruby/language/WarnNode.java b/src/main/java/org/truffleruby/language/WarnNode.java index d5fe8bec974..173136e762f 100644 --- a/src/main/java/org/truffleruby/language/WarnNode.java +++ b/src/main/java/org/truffleruby/language/WarnNode.java @@ -39,8 +39,8 @@ public static WarnNode create() { } public boolean shouldWarn() { - final Object verbosity = readVerboseNode.execute(); - return verbosity != nil; + final Object verbose = readVerboseNode.execute(); + return verbose != nil; } public final boolean shouldWarnForDeprecation() { diff --git a/src/main/java/org/truffleruby/language/WarningNode.java b/src/main/java/org/truffleruby/language/WarningNode.java index f8b791fdc01..25683bf9e93 100644 --- a/src/main/java/org/truffleruby/language/WarningNode.java +++ b/src/main/java/org/truffleruby/language/WarningNode.java @@ -23,8 +23,8 @@ public static WarningNode create() { @Override public boolean shouldWarn() { - final Object verbosity = readVerboseNode.execute(); - return verbosity == Boolean.TRUE; + final Object verbose = readVerboseNode.execute(); + return verbose == Boolean.TRUE; } @DenyReplace diff --git a/src/main/ruby/truffleruby/core/regexp.rb b/src/main/ruby/truffleruby/core/regexp.rb index b97cbd10b0f..7027cd16552 100644 --- a/src/main/ruby/truffleruby/core/regexp.rb +++ b/src/main/ruby/truffleruby/core/regexp.rb @@ -110,7 +110,7 @@ def self.union(*patterns) else converted = Truffle::Type.rb_check_convert_type(pattern, Regexp, :to_regexp) if Primitive.nil? converted - Regexp.new(Regexp.quote(pattern)) + Primitive.regexp_compile(Regexp.quote(pattern), 0) else converted end @@ -182,6 +182,7 @@ def self.new(pattern, opts = undefined, encoding = nil) Primitive.regexp_compile pattern, opts # may be overridden by subclasses end + Truffle::Graal.always_split(method(:new)) class << self alias_method :compile, :new diff --git a/src/main/ruby/truffleruby/core/string.rb b/src/main/ruby/truffleruby/core/string.rb index ff4b375c9f4..d199c394536 100644 --- a/src/main/ruby/truffleruby/core/string.rb +++ b/src/main/ruby/truffleruby/core/string.rb @@ -185,7 +185,7 @@ def grapheme_clusters(&block) if block_given? each_grapheme_cluster(&block) else - regex = Regexp.new('\X'.encode(encoding)) + regex = Primitive.regexp_compile('\X'.encode(encoding), 0) scan(regex) end end @@ -315,6 +315,7 @@ def scan(pattern, &block) Primitive.regexp_last_match_set(Primitive.caller_special_variables, last_match) ret end + Truffle::Graal.always_split(instance_method(:scan)) def split(pattern = nil, limit = undefined, &block) Truffle::Splitter.split(Primitive.dup_as_string_instance(self), pattern, limit, &block) @@ -374,7 +375,7 @@ def tr_s(source, replacement) def each_grapheme_cluster return to_enum(:each_grapheme_cluster) { size } unless block_given? - regex = Regexp.new('\X'.encode(encoding)) + regex = Primitive.regexp_compile('\X'.encode(encoding), 0) # scan(regex, &block) would leak the $ vars in the user block which is probably unwanted scan(regex) { |e| yield e } self @@ -653,6 +654,7 @@ def sub(pattern, replacement = undefined, &block) Primitive.string_replace(s, res) if res s end + Truffle::Graal.always_split(instance_method(:sub)) def sub!(pattern, replacement = undefined, &block) if Primitive.undefined?(replacement) && !block_given? @@ -671,6 +673,7 @@ def sub!(pattern, replacement = undefined, &block) nil end end + Truffle::Graal.always_split(instance_method(:sub!)) def slice!(one, two = undefined) Primitive.check_mutable_string self @@ -898,6 +901,7 @@ def gsub(pattern, replacement = undefined, &block) Primitive.string_replace(s, res) if res s end + Truffle::Graal.always_split(instance_method(:gsub)) def gsub!(pattern, replacement = undefined, &block) if Primitive.undefined?(replacement) && !block_given? @@ -916,6 +920,7 @@ def gsub!(pattern, replacement = undefined, &block) nil end end + Truffle::Graal.always_split(instance_method(:gsub!)) def match(pattern, pos = 0) pattern = Truffle::Type.coerce_to_regexp(pattern) unless Primitive.is_a?(pattern, Regexp) @@ -930,11 +935,13 @@ def match(pattern, pos = 0) Primitive.regexp_last_match_set(Primitive.caller_special_variables, $~) result end + Truffle::Graal.always_split(instance_method(:match)) def match?(pattern, pos = 0) pattern = Truffle::Type.coerce_to_regexp(pattern) unless Primitive.is_a?(pattern, Regexp) pattern.match? self, pos end + Truffle::Graal.always_split(instance_method(:match?)) def scrub(replace = nil, &block) return Primitive.dup_as_string_instance(self) if valid_encoding? diff --git a/src/main/ruby/truffleruby/core/symbol.rb b/src/main/ruby/truffleruby/core/symbol.rb index 35fde177b1c..5ebd4eb2546 100644 --- a/src/main/ruby/truffleruby/core/symbol.rb +++ b/src/main/ruby/truffleruby/core/symbol.rb @@ -88,6 +88,7 @@ def length def match(*args, &block) to_s.match(*args, &block) end + Truffle::Graal.always_split(instance_method(:match)) def =~(pattern) str = to_s @@ -108,6 +109,7 @@ def match?(pattern, pos = 0) pattern = Truffle::Type.coerce_to_regexp(pattern) unless Primitive.is_a?(pattern, Regexp) pattern.match? to_s, pos end + Truffle::Graal.always_split(instance_method(:match?)) def encoding Primitive.encoding_get_object_encoding self diff --git a/src/main/ruby/truffleruby/core/truffle/string_operations.rb b/src/main/ruby/truffleruby/core/truffle/string_operations.rb index ea7645c1e03..6c38f1cbfc6 100644 --- a/src/main/ruby/truffleruby/core/truffle/string_operations.rb +++ b/src/main/ruby/truffleruby/core/truffle/string_operations.rb @@ -76,6 +76,7 @@ def self.gsub_internal_matches(global, orig, pattern) gsub_other_matches(global, orig, pattern) end end + Truffle::Graal.always_split(method(:gsub_internal_matches)) def self.gsub_new_offset(orig, match) start_pos = Primitive.match_data_byte_begin(match, 0) @@ -127,6 +128,7 @@ def self.gsub_other_matches(global, orig, pattern) end res end + Truffle::Graal.always_split(method(:gsub_other_matches)) def self.gsub_internal_yield_matches(orig, matches) return nil if matches.empty? diff --git a/src/main/ruby/truffleruby/core/truffle/warning_operations.rb b/src/main/ruby/truffleruby/core/truffle/warning_operations.rb index f2cec9282aa..bcd18494d2a 100644 --- a/src/main/ruby/truffleruby/core/truffle/warning_operations.rb +++ b/src/main/ruby/truffleruby/core/truffle/warning_operations.rb @@ -15,5 +15,9 @@ def self.check_category(category) raise ArgumentError, "unknown category: #{category}" end + + def self.performance_warning(message) + ::Warning.warn(message, category: :performance) + end end end diff --git a/src/main/ruby/truffleruby/core/type.rb b/src/main/ruby/truffleruby/core/type.rb index c8b442fb0d8..1f9adfe391b 100644 --- a/src/main/ruby/truffleruby/core/type.rb +++ b/src/main/ruby/truffleruby/core/type.rb @@ -407,9 +407,10 @@ def self.coerce_to_regexp(pattern, quote = false) else pattern = StringValue(pattern) pattern = Regexp.quote(pattern) if quote - Regexp.new(pattern) + Primitive.regexp_compile pattern, 0 end end + Truffle::Graal.always_split(method(:coerce_to_regexp)) def self.coerce_to_encoding(obj) case obj