Feature #19538: Performance warnings

• Status: Closed
• Assignee: -
• Target version: -

[byroot (Jean Boussier)]

Suggested by @Eregon (Benoit Daloze).

There are program behaviors that are supported, but that we know aren't good for performance, however it's hard for users to know about them.

Now that we have warning categories, we could add a :performance category to allow the VM to emit warning in some situations.

The category would be disabled by default, and users interested in optimizing their program could turn it on in development.

Warning[:performance] = true

[mame (Yusuke Endoh)]

When does the option print a warning, for example?

[byroot (Jean Boussier)]

So the example that sparked the idea was SHAPE_TOO_COMPLEX. It would be useful to emit a warning such as:

warning: SomeClass has too many shapes.

Other potential ideas could be to warn when constant caches are invalidated, or thing like that.

However there is probably a complicated balance to find between relevance and verbosity.

[mame (Yusuke Endoh)]

• Related to Bug #19535: Instance variables order is unpredictable on objects with `OBJ_TOO_COMPLEX_SHAPE_ID` added

[Eregon (Benoit Daloze)]

Yes, I think this would be great.
TruffleRuby for instance already uses performance warnings for:

• unstable interpolated regexps, i.e., an interpolated regexp for which we have seen more than N=8 different sources, so then we cannot really inline cache anymore and have to compile the regexp on the fly which is quite expensive (especially for what looks like a "literal" in source code).
• some methods need caller state, when we can't find the caller node for those it means we have to walk the stack on every call to get that info, so then we warn for performance (pretty rare).

In TruffleRuby those performance warnings are shown at most once per source location, that seems good to avoid too much verbosity and perf impact (of printing lots of warnings).

There wasn't Warning[] when we added them, but I think that's a great fit.

[Eregon (Benoit Daloze)]

From investigating some benchmark today, I think it would be valuable to (opt-in of course) performance warn for:

• uncached method lookup (i.e., the inline cache gave up, it saw too many different obj->klass at a call site)
• singleton class creation on non-module objects (i.e., on instances). They basically cause the above, but being able to know where the singleton class is created is quite valuable and not easy to find from an uncached call site.

Some amount of singleton classes on instances is fine though, the problem is if it's done repeatedly. If it's on a few global objects it's no problem, just like it's no problem on class/modules (as long as one doesn't create many classes/modules dynamically and use them at the same call site).
Given the relation, I think warning for singleton class creation probably deserves a different opt-in mechanism, it's more like debugging aid for uncached method lookup (which are pretty much always bad for performance).

One concern though is IIRC CRuby uses monomophic inline caches, so it would warn even for just 2 different classes seen at a call site. That seems too aggressive. TruffleRuby uses a limit of 8 which is probably too high but at least if we see more than 8 classes at a call site that's fair to call it megamorphic and not polymorphic anymore.

[Eregon (Benoit Daloze)]

• Related to Feature #19573: Add Class#singleton_inherited added

[Eregon (Benoit Daloze)]

#19573 is an alternative way to track singleton class creation, and @jeremyevans0 (Jeremy Evans) posted numbers for the cost of singleton classes in https://bugs.ruby-lang.org/issues/19573#note-3.
I think a performance warning is more convenient to track singleton class creation, rather than Class#singleton_inherited, but I'm not against it.
Performance warnings are easier to optimize for the "not enabled" case than avoiding the extra calls to Class#singleton_inherited, because those still need a method lookup to figure out it's the default empty singleton_inherited and skip the call.

[matz (Yukihiro Matsumoto)]

Adding Warning[:performance] = true is acceptable. Warning for OBJ_TOO_COMPLEX_SHAPE_ID is also acceptable.
But warnings for singleton class creation is not acceptable (because it's unavoidable for specific use cases).

Matz.

[mame (Yusuke Endoh)]

@matz (Yukihiro Matsumoto) (and @ko1 (Koichi Sasada), @shyouhei (Shyouhei Urabe), and @nobu (Nobuyoshi Nakada)) also said that ruby -w should not enable this warning mode. ruby -W:performance should do.

[nobu (Nobuyoshi Nakada)]

https://github.com/ruby/ruby/pull/7707

[byroot (Jean Boussier)]

I was working on one too, to add the max variation warnings: https://github.com/ruby/ruby/pull/7708

[byroot (Jean Boussier)]

• Status changed from Open to Closed

Applied in changeset git|ac123f167a364c3d7a43eca78d564e41f6dbb91e.

---

Emit a performance warning when a class reached max variations

[Feature #19538]

This new peformance warning category is disabled by default.
It needs to be specifically enabled via -W:performance or Warning[:performance] = true

[Eregon (Benoit Daloze)]

Thank you, makes sense.
Of course these performance warnings might differ per Ruby implementation.
I thought -w should enable them but it seems multiple people disagree, OK.

What about megamorphic method lookup?
I guess the problem there is CRuby has no way to detect that due to monomophic caches?

CRuby can detect singleton class creation though.
Maybe we could warn for singleton classes but only if it happens more than some threshold (e.g. 10) per superclass of that singleton class (and ignore it if superclass is Class).
WDYT?