-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Move globals in top level exprs to the old generation of the GC. #54433
base: master
Are you sure you want to change the base?
Conversation
@vtjnash, what does the 👎 mean here? Should that be taken as an objection to merging this? |
Toplevel exprs are supposed to be rooted |
Right, basically this fixes this bug, but it seems there's another bug somewhere that's causing this. |
So there are a couple things with this code, the correct solution here would be to special case globals from top level expressions in codegen to be assumed not to always have a root so having them trigger write barriers. IMO that is a lot of complexity for a corner case (compiled top level expressions) to save rooting a couple of extra values. |
This fixes #54422, the code here assumes that top level exprs are always rooted, but I don't see that referenced anywhere else, or guaranteed, so conservatively always root objects that show up in code.