Skip to content

Commit cb60ddb

Browse files
committed
Emit performance warnings regardless of compilation and add spec for Regexp inline caches
* Much easier to test and reproduce that way. * Fix PerformanceWarningNode & NotOptimizedWarningNode to check $VERBOSE and to call Warning.warn.
1 parent 51ae78b commit cb60ddb

File tree

12 files changed

+139
-34
lines changed

12 files changed

+139
-34
lines changed

spec/tags/truffle/splitting_tags.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
slow:Critical methods whic must split are under 100 AST nodes
2+
slow:Critical methods which must split are under 100 AST nodes

spec/truffle/splitting_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
require_relative '../ruby/spec_helper'
1010

11-
describe 'Critical methods whic must split' do
11+
describe 'Critical methods which must split' do
1212
it 'are under 100 AST nodes' do
1313
code = <<-'EOF'
1414
require 'strscan'

src/main/java/org/truffleruby/core/CoreLibrary.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ public final class CoreLibrary {
202202
public final RubyModule truffleRegexpOperationsModule;
203203
public final RubyModule truffleRandomOperationsModule;
204204
public final RubyModule truffleThreadOperationsModule;
205+
public final RubyModule truffleWarningOperationsModule;
205206
public final RubyClass encodingCompatibilityErrorClass;
206207
public final RubyClass encodingUndefinedConversionErrorClass;
207208
public final RubyClass methodClass;
@@ -508,6 +509,7 @@ public CoreLibrary(RubyContext context, RubyLanguage language) {
508509
defineModule(truffleModule, "ReadlineHistory");
509510
truffleRandomOperationsModule = defineModule(truffleModule, "RandomOperations");
510511
truffleThreadOperationsModule = defineModule(truffleModule, "ThreadOperations");
512+
truffleWarningOperationsModule = defineModule(truffleModule, "WarningOperations");
511513
defineModule(truffleModule, "WeakRefOperations");
512514
handleClass = defineClass(truffleModule, objectClass, "Handle");
513515
warningModule = defineModule("Warning");

src/main/java/org/truffleruby/core/regexp/InterpolatedRegexpNode.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
import org.truffleruby.core.encoding.RubyEncoding;
1616
import org.truffleruby.core.regexp.InterpolatedRegexpNodeFactory.RegexpBuilderNodeGen;
1717
import org.truffleruby.core.string.TStringWithEncoding;
18-
import org.truffleruby.language.NotOptimizedWarningNode;
18+
import org.truffleruby.language.PerformanceWarningNode;
1919
import org.truffleruby.language.RubyBaseNode;
2020
import org.truffleruby.language.RubyContextSourceNode;
2121
import org.truffleruby.language.RubyNode;
@@ -103,8 +103,9 @@ Object fast(TStringWithEncoding[] parts,
103103

104104
@Specialization(replaces = "fast")
105105
Object slow(TStringWithEncoding[] parts,
106-
@Cached NotOptimizedWarningNode notOptimizedWarningNode) {
107-
notOptimizedWarningNode.warn("unstable interpolated regexps are not optimized");
106+
@Cached PerformanceWarningNode performanceWarningNode) {
107+
performanceWarningNode.warn(
108+
"unstable interpolated regexps cause deoptimization loops which hurt performance significantly, avoid creating regexps dynamically where possible or cache them to fix this");
108109
return createRegexp(parts);
109110
}
110111

src/main/java/org/truffleruby/core/regexp/RegexpNodes.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.Iterator;
1414

1515
import com.oracle.truffle.api.dsl.Bind;
16+
import com.oracle.truffle.api.dsl.NeverDefault;
1617
import com.oracle.truffle.api.nodes.Node;
1718
import com.oracle.truffle.api.profiles.InlinedBranchProfile;
1819
import com.oracle.truffle.api.strings.TruffleString;
@@ -32,7 +33,7 @@
3233
import org.truffleruby.core.string.RubyString;
3334
import org.truffleruby.core.symbol.RubySymbol;
3435
import org.truffleruby.annotations.Visibility;
35-
import org.truffleruby.language.NotOptimizedWarningNode;
36+
import org.truffleruby.language.PerformanceWarningNode;
3637
import org.truffleruby.language.control.DeferredRaiseException;
3738
import org.truffleruby.language.control.RaiseException;
3839
import org.truffleruby.language.library.RubyStringLibrary;
@@ -60,6 +61,7 @@ public abstract static class QuoteNode extends CoreMethodArrayArgumentsNode {
6061

6162
public abstract RubyString execute(Object raw);
6263

64+
@NeverDefault
6365
public static QuoteNode create() {
6466
return RegexpNodesFactory.QuoteNodeFactory.create(null);
6567
}
@@ -76,10 +78,11 @@ RubyString quoteSymbol(RubySymbol raw) {
7678
}
7779

7880
@Fallback
79-
RubyString quote(Object raw,
81+
static RubyString quote(Object raw,
8082
@Cached ToStrNode toStrNode,
81-
@Cached QuoteNode recursive) {
82-
return recursive.execute(toStrNode.execute(this, raw));
83+
@Cached QuoteNode recursive,
84+
@Bind("this") Node node) {
85+
return recursive.execute(toStrNode.execute(node, raw));
8386
}
8487

8588
}
@@ -192,9 +195,9 @@ RubyRegexp slowCompiling(Object pattern, int options,
192195
@Cached InlinedBranchProfile errorProfile,
193196
@Cached @Shared TruffleString.AsTruffleStringNode asTruffleStringNode,
194197
@Cached @Shared RubyStringLibrary libPattern,
195-
@Cached NotOptimizedWarningNode notOptimizedWarningNode) {
196-
notOptimizedWarningNode.warn(
197-
"unbounded creation of regexps causes deoptimization loops which hurts performance significantly, avoid creating regexps dynamically where possible or cache them to fix this");
198+
@Cached PerformanceWarningNode performanceWarningNode) {
199+
performanceWarningNode.warn(
200+
"unbounded creation of regexps causes deoptimization loops which hurt performance significantly, avoid creating regexps dynamically where possible or cache them to fix this");
198201
return compile(pattern, options, this, libPattern, asTruffleStringNode, errorProfile);
199202
}
200203

src/main/java/org/truffleruby/core/regexp/TruffleRegexpNodes.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
import org.truffleruby.core.string.StringUtils;
7272
import org.truffleruby.interop.TranslateInteropExceptionNode;
7373
import org.truffleruby.language.LazyWarnNode;
74-
import org.truffleruby.language.NotOptimizedWarningNode;
74+
import org.truffleruby.language.PerformanceWarningNode;
7575
import org.truffleruby.language.RubyBaseNode;
7676
import org.truffleruby.language.RubyGuards;
7777
import org.truffleruby.language.WarnNode;
@@ -311,11 +311,10 @@ static Object fastUnion(VirtualFrame frame, RubyString str, Object sep, Object[]
311311

312312
@Specialization(replaces = "fastUnion")
313313
Object slowUnion(RubyString str, Object sep, Object[] args,
314-
@Cached NotOptimizedWarningNode notOptimizedWarningNode,
314+
@Cached PerformanceWarningNode performanceWarningNode,
315315
@Cached InlinedBranchProfile errorProfile) {
316-
notOptimizedWarningNode.warn(
317-
"unbounded creation of regexps causes deoptimization loops which hurts performance significantly, " +
318-
"avoid creating regexps dynamically where possible or cache them to fix this");
316+
performanceWarningNode.warn(
317+
"unbounded creation of regexps causes deoptimization loops which hurt performance significantly, avoid creating regexps dynamically where possible or cache them to fix this");
319318

320319
return buildUnion(str, sep, args, errorProfile);
321320
}

src/main/java/org/truffleruby/extra/TruffleGraalNodes.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,14 @@ RubyProc copyCapturedLocals(RubyProc proc) {
144144

145145
}
146146

147+
@Primitive(name = "compiled?")
148+
public abstract static class IsCompiledNode extends PrimitiveNode {
149+
@Specialization
150+
boolean isCompiled() {
151+
return CompilerDirectives.inCompiledCode();
152+
}
153+
}
154+
147155
@Primitive(name = "assert_compilation_constant")
148156
public abstract static class AssertCompilationConstantNode extends PrimitiveArrayArgumentsNode {
149157

@@ -163,7 +171,7 @@ private void notConstantBoundary() {
163171
}
164172

165173
@Primitive(name = "assert_not_compiled")
166-
public abstract static class AssertNotCompilationConstantNode extends PrimitiveNode {
174+
public abstract static class AssertNotCompiledNode extends PrimitiveNode {
167175

168176
@Specialization
169177
Object assertNotCompiled() {

src/main/java/org/truffleruby/language/NotOptimizedWarningNode.java

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,36 +11,32 @@
1111

1212
import java.util.Set;
1313
import java.util.concurrent.ConcurrentHashMap;
14-
import java.util.logging.Level;
1514

16-
import com.oracle.truffle.api.dsl.NeverDefault;
17-
import org.truffleruby.RubyLanguage;
15+
import com.oracle.truffle.api.strings.TruffleString;
16+
import org.truffleruby.RubyContext;
1817

1918
import com.oracle.truffle.api.CompilerAsserts;
2019
import com.oracle.truffle.api.CompilerDirectives;
2120
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
22-
import com.oracle.truffle.api.dsl.GenerateUncached;
2321
import com.oracle.truffle.api.dsl.Specialization;
2422
import com.oracle.truffle.api.nodes.ControlFlowException;
2523
import com.oracle.truffle.api.source.SourceSection;
24+
import org.truffleruby.core.encoding.Encodings;
25+
import org.truffleruby.language.globals.ReadSimpleGlobalVariableNode;
2626

2727
/** Displays a warning when code is compiled that will compile successfully but is very low performance. We don't want
2828
* 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
2929
* benchmark.
3030
*
3131
* Ideally you should not use this node, and instead you should optimise the code which would use it. */
32-
@GenerateUncached
3332
public abstract class NotOptimizedWarningNode extends RubyBaseNode {
3433

35-
@NeverDefault
36-
public static NotOptimizedWarningNode create() {
37-
return NotOptimizedWarningNodeGen.create();
38-
}
39-
4034
@SuppressWarnings("serial")
4135
protected static final class Warned extends ControlFlowException {
4236
}
4337

38+
@Child ReadSimpleGlobalVariableNode readVerboseNode = ReadSimpleGlobalVariableNode.create("$VERBOSE");
39+
4440
public final void warn(String message) {
4541
executeWarn(message);
4642
}
@@ -50,13 +46,18 @@ public final void warn(String message) {
5046
@Specialization(rewriteOn = Warned.class)
5147
void warnOnce(String message) throws Warned {
5248
// The message should be a constant, because we don't want to do anything expensive to create it
53-
CompilerAsserts.compilationConstant(message);
49+
CompilerAsserts.partialEvaluationConstant(message);
5450

5551
// If we're in the interpreter then don't warn
5652
if (CompilerDirectives.inInterpreter()) {
5753
return;
5854
}
5955

56+
// Do not warn if $VERBOSE is nil
57+
if (readVerboseNode.execute() == nil) {
58+
return;
59+
}
60+
6061
// Only warn if Warning[:performance] is true
6162
if (!getContext().getWarningCategoryPerformance().get()) {
6263
return;
@@ -78,12 +79,14 @@ private void log(String message) {
7879
.getTopMostUserSourceSection(getEncapsulatingSourceSection());
7980

8081
final String displayedWarning = String.format(
81-
"%s: %s",
82+
"%s: warning: %s\n",
8283
getContext().fileLine(userSourceSection),
8384
message);
8485

8586
if (DISPLAYED_WARNINGS.add(displayedWarning)) {
86-
RubyLanguage.LOGGER.log(Level.WARNING, displayedWarning);
87+
var warningString = createString(TruffleString.FromJavaStringNode.getUncached(), displayedWarning,
88+
Encodings.US_ASCII);
89+
RubyContext.send(this, coreLibrary().truffleWarningOperationsModule, "performance_warning", warningString);
8790
}
8891
}
8992

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
* Copyright (c) 2018, 2024 Oracle and/or its affiliates. All rights reserved. This
3+
* code is released under a tri EPL/GPL/LGPL license. You can use it,
4+
* redistribute it and/or modify it under the terms of the:
5+
*
6+
* Eclipse Public License version 2.0, or
7+
* GNU General Public License version 2, or
8+
* GNU Lesser General Public License version 2.1.
9+
*/
10+
package org.truffleruby.language;
11+
12+
import java.util.Set;
13+
import java.util.concurrent.ConcurrentHashMap;
14+
15+
import org.truffleruby.RubyContext;
16+
import org.truffleruby.core.encoding.Encodings;
17+
import org.truffleruby.language.globals.ReadSimpleGlobalVariableNode;
18+
19+
import com.oracle.truffle.api.CompilerAsserts;
20+
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
21+
import com.oracle.truffle.api.dsl.Specialization;
22+
import com.oracle.truffle.api.nodes.ControlFlowException;
23+
import com.oracle.truffle.api.source.SourceSection;
24+
import com.oracle.truffle.api.strings.TruffleString;
25+
26+
public abstract class PerformanceWarningNode extends RubyBaseNode {
27+
28+
@SuppressWarnings("serial")
29+
protected static final class Warned extends ControlFlowException {
30+
}
31+
32+
@Child ReadSimpleGlobalVariableNode readVerboseNode = ReadSimpleGlobalVariableNode.create("$VERBOSE");
33+
34+
public final void warn(String message) {
35+
executeWarn(message);
36+
}
37+
38+
protected abstract void executeWarn(String message);
39+
40+
@Specialization(rewriteOn = Warned.class)
41+
void warnOnce(String message) throws Warned {
42+
// The message should be a constant, because we don't want to do anything expensive to create it
43+
CompilerAsserts.partialEvaluationConstant(message);
44+
45+
// Do not warn if $VERBOSE is nil
46+
if (readVerboseNode.execute() == nil) {
47+
return;
48+
}
49+
50+
// Only warn if Warning[:performance] is true
51+
if (!getContext().getWarningCategoryPerformance().get()) {
52+
return;
53+
}
54+
55+
log(message);
56+
throw new Warned();
57+
}
58+
59+
@Specialization(replaces = "warnOnce")
60+
void doNotWarn(String message) {
61+
// do nothing
62+
}
63+
64+
@TruffleBoundary
65+
private void log(String message) {
66+
// We want the topmost user source section, as otherwise lots of warnings will come from the same core methods
67+
final SourceSection userSourceSection = getContext().getCallStack()
68+
.getTopMostUserSourceSection(getEncapsulatingSourceSection());
69+
70+
final String displayedWarning = String.format(
71+
"%s: warning: %s\n",
72+
getContext().fileLine(userSourceSection),
73+
message);
74+
75+
if (DISPLAYED_WARNINGS.add(displayedWarning)) {
76+
var warningString = createString(TruffleString.FromJavaStringNode.getUncached(), displayedWarning,
77+
Encodings.US_ASCII);
78+
RubyContext.send(this, coreLibrary().truffleWarningOperationsModule, "performance_warning", warningString);
79+
}
80+
}
81+
82+
// The remembered set of displayed warnings is global to the VM
83+
private static final Set<String> DISPLAYED_WARNINGS = ConcurrentHashMap.newKeySet();
84+
}

src/main/java/org/truffleruby/language/WarnNode.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ public static WarnNode create() {
3939
}
4040

4141
public boolean shouldWarn() {
42-
final Object verbosity = readVerboseNode.execute();
43-
return verbosity != nil;
42+
final Object verbose = readVerboseNode.execute();
43+
return verbose != nil;
4444
}
4545

4646
public final boolean shouldWarnForDeprecation() {

0 commit comments

Comments
 (0)