Skip to content

Get rid of frozen shapes#13289

Merged
byroot merged 3 commits intoruby:masterfrom
Shopify:shape-id-flags
Jun 4, 2025
Merged

Get rid of frozen shapes#13289
byroot merged 3 commits intoruby:masterfrom
Shopify:shape-id-flags

Conversation

@casperisfine
Copy link
Contributor

@casperisfine casperisfine commented May 9, 2025

Instead shape_id_t higher bits contain flags, and the first one tells whether the shape is frozen.

This has multiple benefits:

  • Can check if a shape is frozen with a single bit check instead of
    dereferencing a pointer.
  • Guarantees it is always possible to transition to frozen.
  • This allow reclaiming FL_FREEZE (TODO).
  • Allow to efficiently normalize the shape_id on getivar so that frozen status doesn't invalidate inline caches.

@launchable-app
Copy link

launchable-app bot commented May 9, 2025

Tests Failed

✖️no tests failed ✔️61985 tests passed(2 flakes)

@casperisfine casperisfine force-pushed the shape-id-flags branch 7 times, most recently from f6a90fb to 6e3eb3c Compare May 13, 2025 08:56
@casperisfine

This comment was marked as resolved.

@casperisfine casperisfine force-pushed the shape-id-flags branch 16 times, most recently from fa04a3e to e076cdd Compare May 28, 2025 13:39
@byroot byroot requested a review from tenderlove May 28, 2025 13:41
@byroot byroot requested a review from jhawthorn May 28, 2025 13:41
shape.h Outdated
Comment on lines +19 to +20
#define SHAPE_ID_NUM_BITS 16
#define SHAPE_ID_OFFSET_NUM_BITS 15
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately we still can't make shape_id_t 32bit long on 32bit platforms, because inlines caches (either ivar or CC for attr_reader methods) need to pack both an attr_index_t and a shape_id_t in a single uintptr_t.

So we'd need to either make these caches larger, or find some creative solution.

For the time being, I think we cal still use 1bit for frozen, as it still leaves 32k shapes on 32bits.

But if we want to eliminate other shape types like T_OBJECT, we'll have to find a solution for these caches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#13500 would unlock all 32 bits.

Instead `shape_id_t` higher bits contain flags, and the first one
tells whether the shape is frozen.

This has multiple benefits:
  - Can check if a shape is frozen with a single bit check instead of
    dereferencing a pointer.
  - Guarantees it is always possible to transition to frozen.
  - This allow reclaiming `FL_FREEZE` (not done yet).

The downside is you have to be careful to preserve these flags
when transitioning.
byroot added 2 commits June 3, 2025 22:10
Freezing an object changes its `shape_id` This is necessary
so that `setivar` routines can use the `shape_id` as a cache key
and save on checking the frozen status every time.

However for `getivar` routines, this causes needless cache misses.
By clearing that bit we increase hit rate in codepaths that see
both frozen and mutable objects.
@byroot byroot merged commit 6b8dcb7 into ruby:master Jun 4, 2025
82 checks passed
@casperisfine casperisfine deleted the shape-id-flags branch June 4, 2025 06:40
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.

2 participants