Conversation
✅ All Tests passed!✖️no tests failed ✔️61961 tests passed(4 flakes) |
7194d9b to
01b3831
Compare
|
Try on Playground: https://ruby.github.io/play-ruby?run=14672208990 |
0885cf8 to
ce8b928
Compare
|
The YJIT implementation looks good. As to the interpreter, I tried to come up with a simpler instruction sequence, but the current one seemed reasonable as is. |
As I mentioned on the redmine ticket, I think we can remove one of the swap instructions, but it will take some work. I was able to remove it, but Coverage tests fail and I'm not sure how to fix them yet. |
4862a4c to
5fe5aff
Compare
This commit inlines instructions for Class#new. To make this work, we
added a new YARV instructions, `opt_new`. `opt_new` checks whether or
not the `new` method is the default allocator method. If it is, it
allocates the object, and pushes the instance on the stack. If not, the
instruction jumps to the "slow path" method call instructions.
Old instructions:
```
> ruby --dump=insns -e'Object.new'
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,10)>
0000 opt_getconstant_path <ic:0 Object> ( 1)[Li]
0002 opt_send_without_block <calldata!mid:new, argc:0, ARGS_SIMPLE>
0004 leave
```
New instructions:
```
> ./miniruby --dump=insns -e'Object.new'
== disasm: #<ISeq:<main>@-e:1 (1,0)-(1,10)>
0000 opt_getconstant_path <ic:0 Object> ( 1)[Li]
0002 putnil
0003 swap
0004 opt_new <calldata!mid:new, argc:0, ARGS_SIMPLE>, 11
0007 opt_send_without_block <calldata!mid:initialize, argc:0, FCALL|ARGS_SIMPLE>
0009 jump 14
0011 opt_send_without_block <calldata!mid:new, argc:0, ARGS_SIMPLE>
0013 swap
0014 pop
0015 leave
```
This commit speeds up basic object allocation (`Foo.new`) by 60%, but
classes that take keyword parameters see an even bigger benefit because
no hash is allocated when instantiating the object (3x to 6x faster).
Here is an example that uses `Hash.new(capacity: 0)`:
```
> hyperfine "ruby --disable-gems -e'i = 0; while i < 10_000_000; Hash.new(capacity: 0); i += 1; end'" "./ruby --disable-gems -e'i = 0; while i < 10_000_000; Hash.new(capacity: 0); i += 1; end'"
Benchmark 1: ruby --disable-gems -e'i = 0; while i < 10_000_000; Hash.new(capacity: 0); i += 1; end'
Time (mean ± σ): 1.082 s ± 0.004 s [User: 1.074 s, System: 0.008 s]
Range (min … max): 1.076 s … 1.088 s 10 runs
Benchmark 2: ./ruby --disable-gems -e'i = 0; while i < 10_000_000; Hash.new(capacity: 0); i += 1; end'
Time (mean ± σ): 627.9 ms ± 3.5 ms [User: 622.7 ms, System: 4.8 ms]
Range (min … max): 622.7 ms … 633.2 ms 10 runs
Summary
./ruby --disable-gems -e'i = 0; while i < 10_000_000; Hash.new(capacity: 0); i += 1; end' ran
1.72 ± 0.01 times faster than ruby --disable-gems -e'i = 0; while i < 10_000_000; Hash.new(capacity: 0); i += 1; end'
```
This commit changes the backtrace for `initialize`:
```
aaron@tc ~/g/ruby (inline-new)> cat test.rb
class Foo
def initialize
puts caller
end
end
def hello
Foo.new
end
hello
aaron@tc ~/g/ruby (inline-new)> ruby -v test.rb
ruby 3.4.2 (2025-02-15 revision d2930f8) +PRISM [arm64-darwin24]
test.rb:8:in 'Class#new'
test.rb:8:in 'Object#hello'
test.rb:11:in '<main>'
aaron@tc ~/g/ruby (inline-new)> ./miniruby -v test.rb
ruby 3.5.0dev (2025-03-28T23:59:40Z inline-new c4157884e4) +PRISM [arm64-darwin24]
test.rb:8:in 'Object#hello'
test.rb:11:in '<main>'
```
It also increases memory usage for calls to `new` by 122 bytes:
```
aaron@tc ~/g/ruby (inline-new)> cat test.rb
require "objspace"
class Foo
def initialize
puts caller
end
end
def hello
Foo.new
end
puts ObjectSpace.memsize_of(RubyVM::InstructionSequence.of(method(:hello)))
aaron@tc ~/g/ruby (inline-new)> make runruby
RUBY_ON_BUG='gdb -x ./.gdbinit -p' ./miniruby -I./lib -I. -I.ext/common ./tool/runruby.rb --extout=.ext -- --disable-gems ./test.rb
656
aaron@tc ~/g/ruby (inline-new)> ruby -v test.rb
ruby 3.4.2 (2025-02-15 revision d2930f8) +PRISM [arm64-darwin24]
544
```
Thanks to @ko1 for coming up with this idea!
Co-Authored-By: John Hawthorn <john@hawthorn.email>
|
You need to run zjit-bindgen because opt_new shifted some op numbers around. |
| end | ||
|
|
||
| def obj_class_path | ||
| has_class_frame? ? "Class" : nil |
There was a problem hiding this comment.
The nil seems wrong because it looks like it basically breaks the ObjectSpace.trace_object_allocations functionality, at least the allocation_class_path and allocation_method_id methods.
Though to be fair if those basically always return "Class" and :new they probably have little value.
Notably the spec no longer works on TruffleRuby because of the changed expectations due to this commit.
TruffleRuby also inlines Class#new, and has logic inside the implementation of Class#new that if allocation tracing is enabled then it records the relevant information directly on the object, while this information is available.
Maybe CRuby could do the same? Or maybe these methods should be deprecated?
There was a problem hiding this comment.
What if CRuby would return "Class" for allocation_class_path and :new for allocation_method_id if the information is not there? Then we could revert this commit and avoid changing behavior.
Would it be incorrect? In which case?
There was a problem hiding this comment.
@eregon We could probably do that, but to be honest I think we should delete these tests (or update them not to run inside anonymous blocks). As you noted, returning Class and :new is not very helpful information. I think the current behavior is actually more helpful. AFAICT, the reason it's returning nil in this case has something to do with how the test runner is set up (I think because everything is being executed in blocks?)
For example:
require "objspace"
class Foo
def test
ObjectSpace.trace_object_allocations do
o = Object.new
p ObjectSpace.allocation_class_path(o)
p ObjectSpace.allocation_method_id(o)
end
end
end
Foo.new.test
1.times do
ObjectSpace.trace_object_allocations do
o = Object.new
p ObjectSpace.allocation_class_path(o)
p ObjectSpace.allocation_method_id(o)
end
endOutput is this:
make runruby
RUBY_ON_BUG='gdb -x ./.gdbinit -p' ./miniruby -I./lib -I. -I.ext/common ./tool/runruby.rb --extout=.ext -- --disable-gems ./test.rb
"Foo"
:test
nil
nil
The nil is odd, but there is no class or method name in that case. Also the first example is way more useful than just returning Class and :new.
There was a problem hiding this comment.
Good point, we'll refactor those specs to move the trace_object_allocations usages inside named classes and methods and test that as it's more representative of a real usage of the feature.
**What does this PR do?** This PR fixes three issues in the profiler when used with latest ruby-head: 1. It's no longer possible to ask the object_id from a T_IMEMO object. This showed up as a Ruby VM crash with an error message "T_IMEMO can't have an object_id". (See ruby/ruby#13347 for the upstream change) 2. Creating new instances of a class is now inlined into the caller, and there is no longer a frame to represent the new. This broke some of our tests that expected the stack from allocating an object to have the `new` at the top. (See ruby/ruby#13080 for the upstream change) 3. Object ids now count towards the size of objects. This broke some of our tests that asserted on size of profiled objects. (See ruby/ruby#13159 for the upstream change) **Motivation:** Fix support for Ruby 3.5. **Additional Notes:** N/A **How to test the change?** I've updated our specs to cover these changes. Unfortunately, we don't yet test with Ruby 3.5 in CI, so you'll have to test manually if you want to see the fixes working with 3.5. (Note that these changes showed up after 3.5.0-preview1, so testing on -preview1 is not enough)
This commit inlines instructions for Class#new. To make this work, we added a new YARV instructions,
opt_new.opt_newchecks whether or not thenewmethod is the default allocator method. If it is, it allocates the object, and pushes the instance on the stack. If not, the instruction jumps to the "slow path" method call instructions.Old instructions:
New instructions:
This commit speeds up basic object allocation (
Foo.new) by 60%, but classes that take keyword parameters see an even bigger benefit because no hash is allocated when instantiating the object (3x to 6x faster).Here is an example that uses
Hash.new(capacity: 0):This commit changes the backtrace for
initialize:It also increases memory usage for calls to
newby 122 bytes:Feature #21254