Skip to content

RubyVM.stat constant cache metrics #5766

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
maximecb merged 1 commit into from
Apr 5, 2022
Merged

RubyVM.stat constant cache metrics #5766

maximecb merged 1 commit into from
Apr 5, 2022

Conversation

@kddnewton
Copy link
Contributor

@kddnewton kddnewton commented Apr 5, 2022 •
edited
Loading

Before the new constant cache behavior, caches were invalidated by a
single global variable. You could inspect the value of this variable
with RubyVM.stat(:global_constant_state). This was mostly useful to
verify the behavior of the VM or to test constant loading like in Rails.

With the new constant cache behavior, we introduced
RubyVM.stat(:constant_cache) which returned a hash with symbol keys and
integer values that represented the number of live constant caches
associated with the given symbol. Additionally, we removed the old
RubyVM.stat(:global_constant_state).

This was proven to be not very useful, so it doesn't help you diagnose
constant loading issues. So, instead we added the global constant state
back into the RubyVM output. However, that number can be misleading as
now when you invalidate something like Foo::Bar::Baz you're actually
invalidating 3 different lists of inline caches.

This commit attempts to get the best of both worlds. We remove
RubyVM.stat(:global_constant_state) like we did originally, as it
doesn't have the same semantic meaning and it could be confusing going
forward. Instead we add RubyVM.stat(:constant_cache_invalidations) and
RubyVM.stat(:constant_cache_misses). These two metrics should provide
enough information to diagnose any constant loading issues, as well as
provide a replacement for the old global constant state.

cc @byroot @XrXr @tenderlove

@kddnewton
RubyVM.stat constant cache metrics
6ab6eb7 
Before the new constant cache behavior, caches were invalidated by a
single global variable. You could inspect the value of this variable
with RubyVM.stat(:global_constant_state). This was mostly useful to
verify the behavior of the VM or to test constant loading like in Rails.

With the new constant cache behavior, we introduced
RubyVM.stat(:constant_cache) which returned a hash with symbol keys and
integer values that represented the number of live constant caches
associated with the given symbol. Additionally, we removed the old
RubyVM.stat(:global_constant_state).

This was proven to be not very useful, so it doesn't help you diagnose
constant loading issues. So, instead we added the global constant state
back into the RubyVM output. However, that number can be misleading as
now when you invalidate something like `Foo::Bar::Baz` you're actually
invalidating 3 different lists of inline caches.

This commit attempts to get the best of both worlds. We remove
RubyVM.stat(:global_constant_state) like we did originally, as it
doesn't have the same semantic meaning and it could be confusing going
forward. Instead we add RubyVM.stat(:constant_cache_invalidations) and
RubyVM.stat(:constant_cache_misses). These two metrics should provide
enough information to diagnose any constant loading issues, as well as
provide a replacement for the old global constant state.
@ko1
Copy link
Contributor

ko1 commented Apr 5, 2022 •
edited
Loading

NOTE for RubyVM.stat(:constant_cache_misses): We don't have such performance counters in MRI because this kind measurements can become overhead in general. This is why debug_counters are disabled on default. I think constant_cache_misses does not hurt performance because the incrementing may be rare, so I think it is acceptable. But I don't want to increase such counters more. (I don't against for this PR, but I don't want to increase other counters because of this PR)

maximecb
maximecb approved these changes Apr 5, 2022
View reviewed changes
Copy link
Contributor

@maximecb maximecb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice simplification :)

@maximecb maximecb merged commit 8ee4a82 into ruby:master Apr 5, 2022
casperisfine pushed a commit to Shopify/rails that referenced this pull request Apr 6, 2022
@byroot
Handle renaming of global_constant_state in Ruby 3.2
94884da 
Ref: ruby/ruby#5766
maximecb pushed a commit to Shopify/ruby that referenced this pull request Apr 7, 2022
@XrXr @BurdetteLamar
@mame @hsbt @jeremyevans @eregon @znz @nobu @matzbot @kddnewton @aellispierce @soutaro
Sync with upstream (#268)
434b8ed 
* Enhanced RDoc for String#index (ruby#5759)

* ruby_gc_set_params: update malloc_limit when env is set

During VM startup, rb_objspace_alloc sets malloc_limit
(objspace->malloc_params.limit) before ruby_gc_set_params is called, thus
nullifying the effect of RUBY_GC_MALLOC_LIMIT before the initial GC run.

The call sequence is as follows:

  main.c::main()
    ruby_init
      ruby_setup
        Init_BareVM
          rb_objspace_alloc // malloc_limit = gc_params.malloc_limit_min;
    ruby_options
      ruby_process_options
        process_options
          ruby_gc_set_params // RUBY_GC_MALLOC_LIMIT => gc_params.malloc_limit_min

With ruby_gc_set_params setting malloc_limit, RUBY_GC_MALLOC_LIMIT
affects the process sooner.

[ruby-core:107170]

* [ruby/psych] Improve libyaml source downloading error messages

People trying to build CRuby by following the instructions in its
[README] have been running into [errors] due to missing `libyaml`
on their system. Let's try to present a better error message when
it happens.

[README]: https://github.com/ruby/ruby/tree/fb5aa31e2d20ea8e1425432672f4de4c8ca2c26b#how-to-compile-and-install
[errors]: ruby/psych#552

ruby/psych@20a633028e

* Apply timescale configuration for tests of Regexp.timeout

* Removed mswin patch for zlib-1.2.11

* Ignore yaml source

* Unflag a splatted flagged hash if the method doesn't use ruby2_keywords

For a method such as:

  def foo(*callee_args) end

If this method is called with a flagged hash (created by a method
flagged with ruby2_keywords), this previously passed the hash
through without modification.  With this change, it acts as if the
last hash was passed as keywords, so a call to:

  foo(*caller_args)

where the last element of caller_args is a flagged hash, will be
treated as:

  foo(*caller_args[0...-1], **caller_args[-1])

As a result, inside foo, callee_args[-1] is an unflagged duplicate
of caller_args[-1] (all other elements of callee_args match
caller_args).

Fixes [Bug #18625]

* Use latest RSpec to get rspec-mocks ruby2_keywords fix

* Add NEWS entry for Bug #18625 to help adding ruby2_keywords in the missing places

* Give some tips on how to find the missing ruby2_keywords

* Try to fix NoMethodError on slow environments

```
  1) Failure:
TestParallel::TestParallel#test_hungup [/home/user/ruby/tool/test/testunit/test_parallel.rb:215]:
Expected /^Retrying hung up testcases\.+$/ to match "Run options: \n" +
"  --seed=43403\n" +
"  --ruby\n" +
"  \"./miniruby -I../lib -I. -I.ext/common ../tool/runruby.rb --extout=.ext -- --disable-gems\"\n" +
"  -j\n" +
"  t1\n" +
"  --worker-timeout=1\n" +
"\n" +
"# Running tests:\n" +
"\n" +
"/home/user/ruby/tool/lib/test/unit.rb:687:in `block in _run_parallel': undefined method `<' for nil:NilClass (NoMethodError)\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:538:in `block in quit_workers'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:537:in `reject!'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:537:in `quit_workers'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:687:in `_run_parallel'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:810:in `_run_suites'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:849:in `_run_suites'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:1479:in `_run_anything'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:1263:in `_run_anything'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:1654:in `run_tests'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:1641:in `block in _run'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:1640:in `each'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:1640:in `_run'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:1682:in `run'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:1034:in `run'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:882:in `run'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:154:in `run'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:1761:in `run'\n" +
"\tfrom /home/user/ruby/tool/lib/test/unit.rb:1765:in `run'\n" +
"\tfrom /home/user/ruby/tool/test/testunit/tests_for_parallel/runner.rb:14:in `<main>'\n".
```

* Load fake.rb at `BTESTRUBY`

So that `mkmf` checks work from `make run`, and also remove
duplicate `$(MINIRUBYOPT)` which is used in `$(MINIRUBY)`.

* Fix using anonymous block in method accepting explicit keywords

Record block ID before vtable_pop, so the incorrect one doesn't
override it.

Fixes [Bug #18673]

* Document MakeMakefile#append_cflags

This method is at least 7 years old and is widely used in the wild.
Since we need to support it, let's document it to make it discoverable.
Add docs and move it out of the `# :stopdoc:` zone.

* * 2022-04-06 [ci skip]

* RubyVM.stat constant cache metrics (ruby#5766)

Before the new constant cache behavior, caches were invalidated by a
single global variable. You could inspect the value of this variable
with RubyVM.stat(:global_constant_state). This was mostly useful to
verify the behavior of the VM or to test constant loading like in Rails.

With the new constant cache behavior, we introduced
RubyVM.stat(:constant_cache) which returned a hash with symbol keys and
integer values that represented the number of live constant caches
associated with the given symbol. Additionally, we removed the old
RubyVM.stat(:global_constant_state).

This was proven to be not very useful, so it doesn't help you diagnose
constant loading issues. So, instead we added the global constant state
back into the RubyVM output. However, that number can be misleading as
now when you invalidate something like `Foo::Bar::Baz` you're actually
invalidating 3 different lists of inline caches.

This commit attempts to get the best of both worlds. We remove
RubyVM.stat(:global_constant_state) like we did originally, as it
doesn't have the same semantic meaning and it could be confusing going
forward. Instead we add RubyVM.stat(:constant_cache_invalidations) and
RubyVM.stat(:constant_cache_misses). These two metrics should provide
enough information to diagnose any constant loading issues, as well as
provide a replacement for the old global constant state.

* [rubygems/rubygems] Enable mfa on specific keys during gem signin

ruby/rubygems@e787f7f655

* [rubygems/rubygems] Correct mfa level name

ruby/rubygems@a002e351ae

* [rubygems/rubygems] Make mfa the default

ruby/rubygems@0b636f6902

* [rubygems/rubygems] Fix lint

ruby/rubygems@a68bfde18e

* [rubygems/rubygems] Make changes <2.6 compatible

Multiple params to merge was not introduced until Ruby 2.6, so this
merges the two additional params together first and then merges that
with the request body

ruby/rubygems@870f7e9a1c

* [rubygems/rubygems] Remove whitespace

ruby/rubygems@08c2d88137

* [rubygems/rubygems] Update endpoint

ruby/rubygems@a5a7b3ec96

* [rubygems/rubygems] Accomodate gem hosts without profile/me endpoint

ruby/rubygems@31b6dcf5d3

* [rubygems/rubygems] Use YAML

ruby/rubygems@6122e8cac5

* [rubygems/rubygems] Extract default_host method

ruby/rubygems@6e10e75574

* [rubygems/rubygems] Use `ask_yes_no`

ruby/rubygems@1d38e167fa

* Fix a typo [ci skip]

* Bundle RBS 2.3.2 (ruby#5762)

* Update bundled gems list at 8197ae3 [ci skip]

* [DOC] Enhanced RDoc for string slices (ruby#5769)

Creates file doc/string/slices.rdoc that the string slicing methods can link to.

* * 2022-04-07 [ci skip]

Co-authored-by: Burdette Lamar <BurdetteLamar@Yahoo.com>
Co-authored-by: Eric Wong <e@80x24.org>
Co-authored-by: Yusuke Endoh <mame@ruby-lang.org>
Co-authored-by: Hiroshi SHIBATA <hsbt@ruby-lang.org>
Co-authored-by: Jeremy Evans <code@jeremyevans.net>
Co-authored-by: Benoit Daloze <eregontp@gmail.com>
Co-authored-by: Kazuhiro NISHIYAMA <zn@mbf.nifty.com>
Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org>
Co-authored-by: git <svn-admin@ruby-lang.org>
Co-authored-by: Kevin Newton <kddnewton@gmail.com>
Co-authored-by: Ashley Ellis Pierce <anellis12@gmail.com>
Co-authored-by: Soutaro Matsumoto <matsumoto@soutaro.com>
ivoanjo added a commit to DataDog/dd-trace-rb that referenced this pull request Apr 8, 2022
@ivoanjo
Fix broken CI by disabling testing with Ruby head/nightly on macOS
4783902 
For quite a while Ruby-head was stable enough that it did not affect
our testing, but recently a few tests started failing consistently, e.g.
<https://github.com/DataDog/dd-trace-rb/runs/5885670306?check_suite_focus=true>

At least one failure is related to a change to the `RubyVM.stat`
output, which seems to be undergoing some development
(see <ruby/ruby#5766>).

At least one of the other failures (regarding initialization of
`Datadog::Profiling::Recorder` seems to be a regression/new bug
on upstream Ruby.

I don't think it's worth necessarily tracking Ruby-head breaking
changes (for instance the `RubyVM` change had a bit of back and
forth and we would've had to do more work if we were trying to
keep up with it), so let's remove this for now.

As an alternative, Ruby 3.2.0-preview1 is out, so I suggest we look
into adding it to our usual CircleCI setup, and then keep up with
the other preview/rc releases so when December rolls around
we're in good shape to support 3.2.0.
@ivoanjo ivoanjo mentioned this pull request Apr 8, 2022
Merged
@XrXr XrXr mentioned this pull request Apr 8, 2022
Merged
@ivoanjo ivoanjo mentioned this pull request Apr 12, 2022
Merged
eugeneius pushed a commit to rails/rails that referenced this pull request May 19, 2022
@byroot @eugeneius
Handle renaming of global_constant_state in Ruby 3.2
d8e5d1c 
Ref: ruby/ruby#5766
@kddnewton kddnewton deleted the rubyvm-constant-cache branch July 18, 2022 13:24
@fallwith fallwith mentioned this pull request Sep 15, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maximecb maximecb maximecb approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kddnewton @ko1 @maximecb