Emit performance warning on redefining optimised core method

spec/ruby/core/warning/performance_warning_spec.rb

@@ -0,0 +1,28 @@
+require_relative '../../spec_helper'
+
+
+describe "Performance warnings" do
+  guard -> { ruby_version_is("3.4") || RUBY_ENGINE == "truffleruby" } do
+    # Optimising Integer, Float or Symbol methods is kind of implementation detail
+    # but multiple implementations do so. So it seems reasonable to have a test case
+    # for at least one such common method.
+    # See https://bugs.ruby-lang.org/issues/20429
+    context "when redefined optimised methods" do
+      it "emits performance warning for redefining Integer#+" do
+        code = <<~CODE
+          Warning[:performance] = true
+
+          class Integer
+            ORIG_METHOD = instance_method(:+)
+
+            def +(...)
+              ORIG_METHOD.bind(self).call(...)
+            end
+          end
+        CODE
+
+        ruby_exe(code, args: "2>&1").should.include?("warning: Redefining 'Integer#+' disables interpreter and JIT optimizations")
+      end
+    end
+  end
+end

spec/tags/core/warning/performance_warning_tags.txt

@@ -0,0 +1 @@
+slow:Performance warnings when redefined optimised methods emits performance warning for redefining Integer#+

spec/tags/truffle/redefining_optimized_core_methods_tags.txt

@@ -0,0 +1,2 @@
+slow:Redefining optimized core methods emits performance warning for redefining core classes methods
+slow:Prepending a module into a class with optimised methods emits performance warning

spec/truffle/redefining_optimized_core_methods_spec.rb

@@ -0,0 +1,247 @@
+# 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'
+
+# Similar MRI tests are in test/mri/tests/ruby/test_optimization.rb and spec/ruby/core/warning/performance_warning_spec.rb
+describe "Redefining optimized core methods" do
+  it "emits performance warning for redefining core classes methods" do
+    # Only Integer, Float, NilClass and Symbol classes have optimised methods.
+    # They are listed in org.truffleruby.core.inlined.CoreMethodAssumptions.
+
+    prolog = "Warning[:performance] = true"
+
+    code_for_integer_class = <<~CODE
+      class Integer
+        ORIG_METHOD_PLUS = instance_method(:+)
+        def +(...)
+          ORIG_METHOD_PLUS.bind(self).call(...)
+        end
+
+        ORIG_METHOD_MINUS = instance_method(:-)
+        def -(...)
+          ORIG_METHOD_MINUS.bind(self).call(...)
+        end
+
+        ORIG_METHOD_UMINUS = instance_method(:-@)
+        def -@(...)
+          ORIG_METHOD_UMINUS.bind(self).call(...)
+        end
+
+        ORIG_METHOD_MULT = instance_method(:*)
+        def *(...)
+          ORIG_METHOD_MULT.bind(self).call(...)
+        end
+
+        ORIG_METHOD_DIVISION = instance_method(:/)
+        def /(...)
+          ORIG_METHOD_DIVISION.bind(self).call(...)
+        end
+
+        ORIG_METHOD_MODULO = instance_method(:%)
+        def %(...)
+          ORIG_METHOD_MODULO.bind(self).call(...)
+        end
+
+        ORIG_METHOD_COMPARE = instance_method(:<=>)
+        def <=>(...)
+          ORIG_METHOD_COMPARE.bind(self).call(...)
+        end
+
+        ORIG_METHOD_SHIFTRIGHT = instance_method(:>>)
+        def >>(...)
+          ORIG_METHOD_SHIFTRIGHT.bind(self).call(...)
+        end
+
+        ORIG_METHOD_SHIFTLEFTL = instance_method(:<<)
+        def <<(...)
+          ORIG_METHOD_SHIFTLEFTL.bind(self).call(...)
+        end
+
+        ORIG_METHOD_OR = instance_method(:|)
+        def |(...)
+          ORIG_METHOD_OR.bind(self).call(...)
+        end
+
+        ORIG_METHOD_AND = instance_method(:&)
+        def &(...)
+          ORIG_METHOD_AND.bind(self).call(...)
+        end
+
+        ORIG_METHOD_CASECOMPARE = instance_method(:===)
+        def ===(...)
+          ORIG_METHOD_CASECOMPARE.bind(self).call(...)
+        end
+
+        ORIG_METHOD_EQUAL = instance_method(:==)
+        def ==(...)
+          ORIG_METHOD_EQUAL.bind(self).call(...)
+        end
+
+        ORIG_METHOD_LT = instance_method(:<)
+        def <(...)
+          ORIG_METHOD_LT.bind(self).call(...)
+        end
+
+        ORIG_METHOD_LTE = instance_method(:<=)
+        def <=(...)
+          ORIG_METHOD_LTE.bind(self).call(...)
+        end
+
+        ORIG_METHOD_GT = instance_method(:>)
+        def >(...)
+          ORIG_METHOD_GT.bind(self).call(...)
+        end
+
+        ORIG_METHOD_GTE = instance_method(:>=)
+        def >=(...)
+          ORIG_METHOD_GTE.bind(self).call(...)
+        end
+      end
+    CODE
+
+    code_for_float_class = <<~CODE
+      class Float
+        ORIG_METHOD_PLUS = instance_method(:+)
+        def +(...)
+          ORIG_METHOD_PLUS.bind(self).call(...)
+        end
+
+        ORIG_METHOD_MINUS = instance_method(:-)
+        def -(...)
+          ORIG_METHOD_MINUS.bind(self).call(...)
+        end
+
+        ORIG_METHOD_UMINUS = instance_method(:-@)
+        def -@(...)
+          ORIG_METHOD_UMINUS.bind(self).call(...)
+        end
+
+        ORIG_METHOD_MULT = instance_method(:*)
+        def *(...)
+          ORIG_METHOD_MULT.bind(self).call(...)
+        end
+
+        ORIG_METHOD_DIVISION = instance_method(:/)
+        def /(...)
+          ORIG_METHOD_DIVISION.bind(self).call(...)
+        end
+
+        ORIG_METHOD_MODULO = instance_method(:%)
+        def %(...)
+          ORIG_METHOD_MODULO.bind(self).call(...)
+        end
+
+        ORIG_METHOD_COMPARE = instance_method(:<=>)
+        def <=>(...)
+          ORIG_METHOD_COMPARE.bind(self).call(...)
+        end
+
+        ORIG_METHOD_EQUAL = instance_method(:==)
+        def ==(...)
+          ORIG_METHOD_EQUAL.bind(self).call(...)
+        end
+
+        ORIG_METHOD_LT = instance_method(:<)
+        def <(...)
+          ORIG_METHOD_LT.bind(self).call(...)
+        end
+
+        ORIG_METHOD_LTE = instance_method(:<=)
+        def <=(...)
+          ORIG_METHOD_LTE.bind(self).call(...)
+        end
+
+        ORIG_METHOD_GT = instance_method(:>)
+        def >(...)
+          ORIG_METHOD_GT.bind(self).call(...)
+        end
+
+        ORIG_METHOD_GTE = instance_method(:>=)
+        def >=(...)
+          ORIG_METHOD_GTE.bind(self).call(...)
+        end
+      end
+    CODE
+
+    code_for_nil_class = <<~CODE
+      class NilClass
+        ORIG_METHOD_NIL = instance_method(:nil?)
+
+        def nil?(...)
+          ORIG_METHOD_NIL.bind(self).call(...)
+        end
+      end
+    CODE
+
+    code_for_symbol_class = <<~CODE
+      class Symbol
+        ORIG_METHOD_TO_PROC = instance_method(:to_proc)
+
+        def to_proc(...)
+          ORIG_METHOD_TO_PROC.bind(self).call(...)
+        end
+      end
+    CODE
+
+    code = [prolog, code_for_integer_class, code_for_float_class, code_for_nil_class, code_for_symbol_class].join("\n")
+    output = ruby_exe(code, args: "2>&1")
+
+    output.should.include?("warning: Redefining 'Integer#+' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#-' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#-@' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#*' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#/' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#%' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#<=>' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#>>' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#<<' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#|' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#&' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#===' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#==' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#<' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#<=' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#>' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Integer#>=' disables interpreter and JIT optimizations")
+
+    output.should.include?("warning: Redefining 'Float#+' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Float#-' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Float#-@' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Float#*' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Float#/' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Float#%' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Float#<=>' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Float#==' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Float#<' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Float#<=' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Float#>' disables interpreter and JIT optimizations")
+    output.should.include?("warning: Redefining 'Float#>=' disables interpreter and JIT optimizations")
+
+    output.should.include?("warning: Redefining 'NilClass#nil?' disables interpreter and JIT optimizations")
+
+    output.should.include?("warning: Redefining 'Symbol#to_proc' disables interpreter and JIT optimizations")
+  end
+end
+
+describe "Prepending a module into a class with optimised methods" do
+  it "emits performance warning" do
+    code = <<~CODE
+      Warning[:performance] = true
+
+      module M
+      end
+
+      class Integer
+        prepend M
+      end
+    CODE
+
+    ruby_exe(code, args: "2>&1").should.include?("warning: Prepending a module to Integer disables interpreter and JIT optimizations")
+  end
+end

src/main/java/org/truffleruby/core/module/ModuleFields.java

@@ -40,6 +40,7 @@
 import org.truffleruby.core.string.StringUtils;
 import org.truffleruby.core.symbol.RubySymbol;
 import org.truffleruby.language.Nil;
+import org.truffleruby.language.PerformanceWarningNode;
 import org.truffleruby.language.RubyConstant;
 import org.truffleruby.language.RubyDynamicObject;
 import org.truffleruby.language.RubyGuards;
@@ -325,7 +326,7 @@ public void include(RubyContext context, Node currentNode, RubyModule module) {
 
         performIncludes(inclusionPoint, modulesToInclude);
 
-        newHierarchyVersion();
+        newHierarchyVersion(context, currentNode);
     }
 
     private void performIncludes(ModuleChain inclusionPoint, Deque<RubyModule> moduleAncestors) {
@@ -401,9 +402,9 @@ public void prepend(RubyContext context, Node currentNode, RubyModule module) {
         }
 
         // If there were already prepended modules, invalidate the first of them
-        newHierarchyVersion();
+        newHierarchyVersion(context, currentNode);
 
-        invalidateBuiltinsAssumptions();
+        invalidateBuiltinsAssumptions(context, currentNode);
     }
 
     private List<RubyModule> getPrependedModulesAndSelf() {
@@ -555,9 +556,9 @@ public void addMethod(RubyContext context, Node currentNode, InternalMethod meth
             }
 
             // invalidate assumptions to not use an AST-inlined methods
-            changedMethod(method.getName());
+            changedMethod(context, method.getName(), currentNode);
             if (refinedModule != null) {
-                refinedModule.fields.changedMethod(method.getName());
+                refinedModule.fields.changedMethod(context, method.getName(), currentNode);
             }
         }
 
@@ -585,7 +586,7 @@ public void addMethod(RubyContext context, Node currentNode, InternalMethod meth
     }
 
     @TruffleBoundary
-    public boolean removeMethod(String methodName) {
+    public boolean removeMethod(RubyContext context, String methodName, Node currentNode) {
         final InternalMethod method = getMethod(methodName);
         if (method == null) {
             return false;
@@ -596,7 +597,7 @@ public boolean removeMethod(String methodName) {
             removedEntry.invalidate(rubyModule, methodName);
         }
 
-        changedMethod(methodName);
+        changedMethod(context, methodName, currentNode);
         return true;
     }
 
@@ -841,13 +842,13 @@ public String toString() {
         return super.toString() + "(" + getName() + ")";
     }
 
-    public void newHierarchyVersion() {
+    public void newHierarchyVersion(RubyContext context, Node currentNode) {
         if (!isClass()) {
             hierarchyUnmodifiedAssumption.invalidate(getName());
         }
 
         if (isRefinement()) {
-            getRefinedModule().fields.invalidateBuiltinsAssumptions();
+            getRefinedModule().fields.invalidateBuiltinsAssumptions(context, currentNode);
         }
     }
 
@@ -1159,18 +1160,31 @@ public void registerAssumption(String methodName, Assumption assumption) {
         assert old == null;
     }
 
-    private void changedMethod(String name) {
-        Assumption assumption = inlinedBuiltinsAssumptions.get(name);
+    private void changedMethod(RubyContext context, String methodName, Node currentNode) {
+        Assumption assumption = inlinedBuiltinsAssumptions.get(methodName);
         if (assumption != null) {
             assumption.invalidate();
+
+            PerformanceWarningNode.warn(
+                    context,
+                    StringUtils.format("Redefining '%s#%s' disables interpreter and JIT optimizations", getName(),
+                            methodName),
+                    currentNode);
         }
     }
 
-    private void invalidateBuiltinsAssumptions() {
+    private void invalidateBuiltinsAssumptions(RubyContext context, Node currentNode) {
         if (!inlinedBuiltinsAssumptions.isEmpty()) {
             for (Assumption assumption : inlinedBuiltinsAssumptions.values()) {
                 assumption.invalidate();
             }
+
+            PerformanceWarningNode.warn(
+                    context,
+                    StringUtils.format("Prepending a module to %s disables interpreter and JIT optimizations",
+                            getName()),
+                    currentNode);
         }
     }
+
 }