Skip to content

Commit

Permalink
Emit a performance warning when a class reached max variations
Browse files Browse the repository at this point in the history 
[Feature #19538]

This new `peformance` warning category is disabled by default.
It needs to be specifically enabled via `-W:performance` or `Warning[:performance] = true`
  • Loading branch information
@byroot
byroot committed Apr 13, 2023
1 parent 854baee commit ac123f1
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 5 deletions.
2 changes: 2 additions & 0 deletions common.mk
View file
Expand Up @@ -14227,10 +14227,12 @@ shape.$(OBJEXT): $(top_srcdir)/internal/array.h
shape.$(OBJEXT): $(top_srcdir)/internal/basic_operators.h
shape.$(OBJEXT): $(top_srcdir)/internal/class.h
shape.$(OBJEXT): $(top_srcdir)/internal/compilers.h
shape.$(OBJEXT): $(top_srcdir)/internal/error.h
shape.$(OBJEXT): $(top_srcdir)/internal/gc.h
shape.$(OBJEXT): $(top_srcdir)/internal/imemo.h
shape.$(OBJEXT): $(top_srcdir)/internal/serial.h
shape.$(OBJEXT): $(top_srcdir)/internal/static_assert.h
shape.$(OBJEXT): $(top_srcdir)/internal/string.h
shape.$(OBJEXT): $(top_srcdir)/internal/symbol.h
shape.$(OBJEXT): $(top_srcdir)/internal/variable.h
shape.$(OBJEXT): $(top_srcdir)/internal/vm.h
Expand Down
4 changes: 4 additions & 0 deletions error.c
View file
Expand Up @@ -77,6 +77,7 @@ static ID id_warn;
static ID id_category;
static ID id_deprecated;
static ID id_experimental;
static ID id_performance;
static VALUE sym_category;
static VALUE sym_highlight;
static struct {
Expand Down Expand Up @@ -3147,6 +3148,7 @@ Init_Exception(void)
id_category = rb_intern_const("category");
id_deprecated = rb_intern_const("deprecated");
id_experimental = rb_intern_const("experimental");
id_performance = rb_intern_const("performance");
id_top = rb_intern_const("top");
id_bottom = rb_intern_const("bottom");
id_iseq = rb_make_internal_id();
Expand All @@ -3158,11 +3160,13 @@ Init_Exception(void)
warning_categories.id2enum = rb_init_identtable();
st_add_direct(warning_categories.id2enum, id_deprecated, RB_WARN_CATEGORY_DEPRECATED);
st_add_direct(warning_categories.id2enum, id_experimental, RB_WARN_CATEGORY_EXPERIMENTAL);
st_add_direct(warning_categories.id2enum, id_performance, RB_WARN_CATEGORY_PERFORMANCE);

warning_categories.enum2id = rb_init_identtable();
st_add_direct(warning_categories.enum2id, RB_WARN_CATEGORY_NONE, 0);
st_add_direct(warning_categories.enum2id, RB_WARN_CATEGORY_DEPRECATED, id_deprecated);
st_add_direct(warning_categories.enum2id, RB_WARN_CATEGORY_EXPERIMENTAL, id_experimental);
st_add_direct(warning_categories.enum2id, RB_WARN_CATEGORY_PERFORMANCE, id_performance);
}

void
Expand Down
3 changes: 3 additions & 0 deletions include/ruby/internal/error.h
View file
Expand Up @@ -50,6 +50,9 @@ typedef enum {
/** Warning is for experimental features. */
RB_WARN_CATEGORY_EXPERIMENTAL,

/** Warning is for performance issues (not enabled by -w). */
RB_WARN_CATEGORY_PERFORMANCE,

RB_WARN_CATEGORY_ALL_BITS = (
(1U << RB_WARN_CATEGORY_DEPRECATED) |
(1U << RB_WARN_CATEGORY_EXPERIMENTAL) |
Expand Down
4 changes: 4 additions & 0 deletions ruby.c
View file
Expand Up @@ -331,6 +331,7 @@ usage(const char *name, int help, int highlight, int columns)
static const struct ruby_opt_message warn_categories[] = {
M("deprecated", "", "deprecated features"),
M("experimental", "", "experimental features"),
M("performance", "", "performance issues"),
};
#if USE_YJIT
static const struct ruby_opt_message yjit_options[] = {
Expand Down Expand Up @@ -1190,6 +1191,9 @@ proc_options(long argc, char **argv, ruby_cmdline_options_t *opt, int envopt)
else if (NAME_MATCH_P("experimental", s, len)) {
bits = 1U << RB_WARN_CATEGORY_EXPERIMENTAL;
}
else if (NAME_MATCH_P("performance", s, len)) {
bits = 1U << RB_WARN_CATEGORY_PERFORMANCE;
}
else {
rb_warn("unknown warning category: `%s'", s);
}
Expand Down
11 changes: 11 additions & 0 deletions shape.c
View file
Expand Up @@ -7,6 +7,7 @@
#include "internal/gc.h"
#include "internal/symbol.h"
#include "internal/variable.h"
#include "internal/error.h"
#include "variable.h"
#include <stdbool.h>

Expand Down Expand Up @@ -407,6 +408,16 @@ rb_shape_get_next(rb_shape_t* shape, VALUE obj, ID id)

if (variation_created) {
RCLASS_EXT(klass)->variation_count++;
if (rb_warning_category_enabled_p(RB_WARN_CATEGORY_PERFORMANCE)) {
if (RCLASS_EXT(klass)->variation_count >= SHAPE_MAX_VARIATIONS) {
rb_category_warning(
RB_WARN_CATEGORY_PERFORMANCE,
"Maximum shapes variations (%d) reached by %"PRIsVALUE", instance variables accesses will be slower.",
SHAPE_MAX_VARIATIONS,
rb_class_path(klass)
);
}
}
}
}

Expand Down
12 changes: 12 additions & 0 deletions test/ruby/test_object.rb
View file
Expand Up @@ -422,6 +422,18 @@ def test_check_to_integer
assert_equal(1+3+5+7+9, n)
end

def test_max_shape_variation_with_performance_warnings
assert_in_out_err([], <<-INPUT, %w(), /Maximum shapes variations \(8\) reached by Foo, instance variables accesses will be slower\.$/)
$VERBOSE = true
Warning[:performance] = true
class Foo; end
10.times do |i|
Foo.new.instance_variable_set(:"@a\#{i}", nil)
end
INPUT
end

def test_redefine_method_under_verbose
assert_in_out_err([], <<-INPUT, %w(2), /warning: method redefined; discarding old foo$/)
$VERBOSE = true
Expand Down
15 changes: 10 additions & 5 deletions test/ruby/test_rubyoptions.rb
View file
Expand Up @@ -99,19 +99,24 @@ def test_warning
assert_in_out_err(%w(-W:no-deprecated -e) + ['p Warning[:deprecated]'], "", %w(false), [])
assert_in_out_err(%w(-W:experimental -e) + ['p Warning[:experimental]'], "", %w(true), [])
assert_in_out_err(%w(-W:no-experimental -e) + ['p Warning[:experimental]'], "", %w(false), [])
assert_in_out_err(%w(-W -e) + ['p Warning[:performance]'], "", %w(false), [])
assert_in_out_err(%w(-W:performance -e) + ['p Warning[:performance]'], "", %w(true), [])
assert_in_out_err(%w(-W:qux), "", [], /unknown warning category: `qux'/)
assert_in_out_err(%w(-w -e) + ['p Warning[:deprecated]'], "", %w(true), [])
assert_in_out_err(%w(-W -e) + ['p Warning[:deprecated]'], "", %w(true), [])
assert_in_out_err(%w(-We) + ['p Warning[:deprecated]'], "", %w(true), [])
assert_in_out_err(%w(-e) + ['p Warning[:deprecated]'], "", %w(false), [])
code = 'puts "#{$VERBOSE}:#{Warning[:deprecated]}:#{Warning[:experimental]}"'
assert_in_out_err(%w(-w -e) + ['p Warning[:performance]'], "", %w(false), [])
assert_in_out_err(%w(-W -e) + ['p Warning[:performance]'], "", %w(false), [])
code = 'puts "#{$VERBOSE}:#{Warning[:deprecated]}:#{Warning[:experimental]}:#{Warning[:performance]}"'
Tempfile.create(["test_ruby_test_rubyoption", ".rb"]) do |t|
t.puts code
t.close
assert_in_out_err(["-r#{t.path}", '-e', code], "", %w(false:false:true false:false:true), [])
assert_in_out_err(["-r#{t.path}", '-w', '-e', code], "", %w(true:true:true true:true:true), [])
assert_in_out_err(["-r#{t.path}", '-W:deprecated', '-e', code], "", %w(false:true:true false:true:true), [])
assert_in_out_err(["-r#{t.path}", '-W:no-experimental', '-e', code], "", %w(false:false:false false:false:false), [])
assert_in_out_err(["-r#{t.path}", '-e', code], "", %w(false:false:true:false false:false:true:false), [])
assert_in_out_err(["-r#{t.path}", '-w', '-e', code], "", %w(true:true:true:false true:true:true:false), [])
assert_in_out_err(["-r#{t.path}", '-W:deprecated', '-e', code], "", %w(false:true:true:false false:true:true:false), [])
assert_in_out_err(["-r#{t.path}", '-W:no-experimental', '-e', code], "", %w(false:false:false:false false:false:false:false), [])
assert_in_out_err(["-r#{t.path}", '-W:performance', '-e', code], "", %w(false:false:true:true false:false:true:true), [])
end
ensure
ENV['RUBYOPT'] = save_rubyopt
Expand Down

0 comments on commit ac123f1

Please sign in to comment.