-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
fix missing gc root on store to iparams #49820
Conversation
Try to optimize the order of this code a bit more, given that these checks are somewhat infrequently to be needed. Fix #49762
Any test that can be added? |
Not really. You have to get the timing of GC just right for it to fail. But as long as CI passes, you know it is fixed. |
@nanosoldier |
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. |
This simplifies the types, which may help subtyping other other similar lookup code any time this is later used as a parameter, so it is probably worthwhile to do. This is a followup to #49820, where we reorganized the code to make this more straightforward.
The |
I think you can hard-code it to |
This simplifies the types, which may help subtyping other other similar lookup code any time this is later used as a parameter, so it is probably worthwhile to do. This is a followup to #49820, where we reorganized the code to make this more straightforward.
Backporting this seems to cause the following: #50090 (comment). I removed it for now. |
Try to optimize the order of this code a bit more, given that these checks are somewhat infrequently to be needed.
Fix #49762