Skip to content

Implement Object Shapes #6386

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.

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

Merged
merged 1 commit into from
Sep 26, 2022
Merged

Implement Object Shapes #6386

merged 1 commit into from
Sep 26, 2022

Conversation

jemmaissroff
Copy link
Contributor

This commit implements the Object Shapes technique in CRuby. Object Shapes is used for accessing instance variables and representing the "frozenness" of objects. Object instances have a "shape" and the shape represents some attributes of the object (currently which instance variables are set and the "frozenness"). Shapes form a tree data structure, and when a new instance variable is set on an object, that object "transitions" to a new shape in the shape tree. Each shape has an ID that is used for caching. The shape structure is independent of class, so objects of different types can have the same shape.

For example:

class Foo
  def initialize
    # Starts with shape id 0
    @a = 1 # transitions to shape id 1
    @b = 1 # transitions to shape id 2
  end
end

class Bar
  def initialize
    # Starts with shape id 0
    @a = 1 # transitions to shape id 1
    @b = 1 # transitions to shape id 2
  end
end

foo = Foo.new # `foo` has shape id 2
bar = Bar.new # `bar` has shape id 2

Both foo and bar instances have the same shape because they both set instance variables of the same name in the same order.

This technique can help to improve inline cache hits as well as generate more efficient machine code in JIT compilers.

This commit also adds some methods for debugging shapes on objects. See RubyVM::Shape for more details.

For more context on Object Shapes, see [Feature: #18776]

Co-Authored-By: Aaron Patterson tenderlove@ruby-lang.org
Co-Authored-By: Eileen M. Uchitelle eileencodes@gmail.com

@matzbot matzbot requested a review from a team September 15, 2022 19:16
}
}
RUBY_SYMBOL_EXPORT_BEGIN
void rb_obj_freeze_inline(VALUE obj);
Copy link
Contributor

Choose a reason for hiding this comment

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

inline function is used for performance but is it hard to keep inline?

Copy link
Member

Choose a reason for hiding this comment

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

It seems like the idea is to avoid exporting rb_shape_transition_shape_frozen as C API.

I think it's okay to make it a non-inline function for C API users even while the _inline name becomes a lie, but as Koichi said, it's worth checking if it impacts the interpreter's performance. If it does, you could define it as an inline function in an internal header, and use it as the implementation of (non-inline) rb_obj_freeze_inline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for flagging this. Yes, we removed the inline because we didn't want to export the shape transition function.

I added a new benchmark for measuring freezing objects.

Running this benchmark, it looks like there's no performance change on freezing objects:

compare-ruby: ruby 3.2.0dev (2022-09-20T13:25:05Z master b3d8dddee7) [arm64-darwin21]
built-ruby: ruby 3.2.0dev (2022-09-21T21:33:46Z object-shapes-prot.. d3003b71e3) [arm64-darwin21]
# Iteration per second (i/s)

|               |compare-ruby|built-ruby|
|:--------------|-----------:|---------:|
|vm_freeze_obj  |     364.956|   364.985|
|               |           -|     1.00x|

Copy link
Member

Choose a reason for hiding this comment

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

👍 Thank you for confirming it.

@maximecb maximecb requested review from k0kubun and XrXr September 22, 2022 14:01
configure.ac Outdated
@@ -3724,6 +3724,25 @@ AS_IF([test x"$MJIT_SUPPORT" = "xyes"],

AC_SUBST(MJIT_SUPPORT)

AC_ARG_ENABLE(compile-commands,
Copy link
Member

Choose a reason for hiding this comment

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

It'd be nice to remove the diff of #6352 from this PR.

@@ -73,23 +73,6 @@ def compile_body(f, iseq, status)
src << "#undef GET_SELF\n"
src << "#define GET_SELF() cfp_self\n"

# Generate merged ivar guards first if needed
Copy link
Member

@k0kubun k0kubun Sep 22, 2022

Choose a reason for hiding this comment

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

I think you shouldn't have deleted the merge_ivar_guards_p optimization because checking RB_TYPE_P(obj, T_OBJECT) for self multiple times remains to be redundant. Now this place should check only RB_TYPE_P(GET_SELF(), T_OBJECT) and then the check should be removed from the following places when merge_ivar_guards_p.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, didn't mean to remove this. Should be fixed now!

@k0kubun k0kubun closed this Sep 22, 2022
@k0kubun k0kubun reopened this Sep 22, 2022
@matzbot matzbot requested a review from a team September 22, 2022 15:14
@k0kubun
Copy link
Member

k0kubun commented Sep 22, 2022

Great job. My comment about MJIT #6386 (comment) is not a blocker because I can easily fix it as long as I'm aware that it was removed. However, I do want you to check #6386 (comment) because it's performance-related. #6386 (comment) is fairly trivial, but it's obvious that it should be removed.

I have zero experience of using imemo even today, so I'll leave it for other committers to review how it's used for shapes.

P.S. I misclicked the close button while cleaning up dust on my trackpad. I'm sorry 🙇

@k0kubun
Copy link
Member

k0kubun commented Sep 22, 2022

FYI, #6418 caused a conflict and I fixed it myself at 50d113e (I just re-executed the new bindgen.rb). I wanted to avoid force-pushing your branch, so I just pushed a merge commit. We could use "Squash and merge" if you don't want to rebase it and fix the same conflict again.

Copy link
Member

@k0kubun k0kubun left a comment

Choose a reason for hiding this comment

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

👍

@jemmaissroff jemmaissroff force-pushed the object-shapes branch 2 times, most recently from 8e2fa28 to ecd2b50 Compare September 23, 2022 18:02
Object Shapes is used for accessing instance variables and representing the
"frozenness" of objects.  Object instances have a "shape" and the shape
represents some attributes of the object (currently which instance variables are
set and the "frozenness").  Shapes form a tree data structure, and when a new
instance variable is set on an object, that object "transitions" to a new shape
in the shape tree.  Each shape has an ID that is used for caching. The shape
structure is independent of class, so objects of different types can have the
same shape.

For example:

```ruby
class Foo
  def initialize
    # Starts with shape id 0
    @A = 1 # transitions to shape id 1
    @b = 1 # transitions to shape id 2
  end
end

class Bar
  def initialize
    # Starts with shape id 0
    @A = 1 # transitions to shape id 1
    @b = 1 # transitions to shape id 2
  end
end

foo = Foo.new # `foo` has shape id 2
bar = Bar.new # `bar` has shape id 2
```

Both `foo` and `bar` instances have the same shape because they both set
instance variables of the same name in the same order.

This technique can help to improve inline cache hits as well as generate more
efficient machine code in JIT compilers.

This commit also adds some methods for debugging shapes on objects.  See
`RubyVM::Shape` for more details.

For more context on Object Shapes, see [Feature: #18776]

Co-Authored-By: Aaron Patterson <tenderlove@ruby-lang.org>
Co-Authored-By: Eileen M. Uchitelle <eileencodes@gmail.com>
Co-Authored-By: John Hawthorn <john@hawthorn.email>
@tenderlove tenderlove merged commit 9ddfd2c into ruby:master Sep 26, 2022
@ivoanjo
Copy link
Contributor

ivoanjo commented Sep 27, 2022

This is amazing, congrats for getting this merged in! :)

@YurySolovyov
Copy link

I think it got reverted at 06abfa5

@jaynetics
Copy link
Contributor

The revert was reverted in ad63b66

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants