-
-
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
simplify @allocated macro #33717
simplify @allocated macro #33717
Conversation
while false; end # compiler heuristic: compile this block (alter this if the heuristic changes) | ||
local b0 = Ref{Int64}(0) | ||
local b1 = Ref{Int64}(0) | ||
gc_bytes(b0) |
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.
If this code is compiled, shouldn't it be ok for gc_bytes()
to return a value?
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's some code paths that'll compile but not infer this code. But anyways, it seemed nicer to be at least robust against ever including @allocated
in the overhead.
Changes testset to avoid compiler heuristics (copyast) that disables inference. And changes the allocated macro to rely less on inference to elid allocations for the machinary itself.
Just to make sure this doesn't affect nanosoldier: @nanosoldier |
Your benchmark job has completed - no performance regressions were detected. A full report can be found here. cc @ararslan |
There were a long list of gotchas in the
@allocated
macro to deal with problems that it couldn't actually solve. The simplified macro doesn't have those issues—and even solves some of the issues it was trying to address. This needed an adjustment to@testset
to avoid it triggering the compiler heuristic that disables inference.