-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
remove precompile mutation step from staticdata #48309
Conversation
Make sure things are properly ordered here, so that when serializing, nothing is mutating the system at the same time. Fix #48047
return NULL; | ||
// We must avoid attempting to re-enter inference here | ||
assert(0 && "attempted to enter inference while writing out image"); | ||
abort(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aborting here in non-debug mode seems a bit aggressive. Generally the image may still be ok even if this fails, and it seems unfriendly to the user to not try and continue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is not really any points in this code that would be calling back into julia now, so I had considered deleting this special flag, but I did want to make it hard for someone to introduce the same mistake again. In most later places, the image might also not be complete, which also seemed more aggravating to encounter for the user than a clear abort and backtrace from the source of the problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I also favor the assert.
Is the external methodtable the main fix, and most of the rest (nice) cleanup? I like both the conditional- Oh nvm, I now see you're calling edges
allocation and the typemap visitor pattern.jl_prepare_serialization_data
twice as in #48054 (comment)
Make sure things are properly ordered here, so that when serializing, nothing is mutating the system at the same time.
Fix #48047