Skip to content
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

Initialize context to nil on the Drop class #1673

Merged
merged 1 commit into from Jan 12, 2023
Merged

Conversation

jemmaissroff
Copy link
Contributor

We believe this will reduce megamorphic exits in YJIT. There are currently 241 separate shapes generated with edge_name "@context" in SFR. Initializing it to nil will significantly reduce this number.

We believe this will reduce megamorphic exits in YJIT. There are
currently 241 separate shapes generated with edge_name "@context" in
SFR. Initializing it to nil will significantly reduce this number.
@casperisfine casperisfine requested a review from a team January 12, 2023 21:18
@casperisfine
Copy link
Contributor

cc @Shopify/liquid @Shopify/liquid-perf this is a super small change to play better with Ruby 3.2 and YJIT. I'm taking the liberty to merge this.

@casperisfine casperisfine merged commit bc15315 into master Jan 12, 2023
@casperisfine casperisfine deleted the initialize-context branch January 12, 2023 21:19
@isaacbowen
Copy link

241 separate shapes generated with edge_name "@context" in SFR

@jemmaissroff What's SFR?

@dylanahsmith
Copy link
Contributor

It is a reference to Shopify's Storefront Renderer application, which was blogged about here: https://shopify.engineering/how-shopify-reduced-storefront-response-times-rewrite

Thanks for the reminder for us to avoid these internal acronyms in our open source repos.

@isaacbowen
Copy link

:D On the flipside, it's a ton of fun getting the occasional glimpse into the dynamics y'all are working with. Thanks for being willing to share. :)

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.

None yet

4 participants