performance improvements for frontend #19346
Conversation
|
Hooray! I've been hitting #18361 recently, so this is very welcome! Can this be backported? |
|
The level of detail provided in your commit messages is truly inspiring. 💯 |
|
oops, haha, apparently I forgot to push the actual patch before opening the PR. |
|
Great! It still seems strange that there should be anything superlinear for just running N tests, though? |
|
I'm fairly certain that optimization is superlinear. For example, inference needs to know the value of every variable at every statement which is pretty directly |
vtjnash
force-pushed
the
jn/faster-lowering
branch
from
3071821
to
7a25ae0
Compare
|
Can we get a review on this? Is it ready to merge? |
|
Who needs to review this? @JeffBezanson? |
|
bump. |
|
Another thing to consider here is whether this can/should be backported. |
| (new-env (append all-vars glob env)) ;; all variables declared in or outside blok | ||
| (new-iglo-table ;; initial list of implicit globals from outside blok | ||
| (let ((tab (table))) | ||
| (map (lambda (v) (put! tab v #t)) iglo) |
when merging two frames (such as when handling exception frames), most of the time new is old, but the computation here is exponential in the number of variables so we want the inner computation here to be ultra-fast in the common case fix #15346
only the variable changes need to be propagated on each step to the exception frame (as the whole frame was already synced when handling `Expr(:enter)`) this eliminates an unnecessary exponential pass for handling exception frames along the way, this replaces `(e::Slot).id` with a manually expanded version `slot_id(e)`. Since Slot is a abstract variable, Inference has no way to directly track that the set of subtypes won't change, so it had to be pessimistic and assume that the value was Any. Adding a function barrier removes that difficulty.
vtjnash
force-pushed
the
jn/faster-lowering
branch
from
7a25ae0
to
046abcd
Compare
|
Will merge after CI. |
|
Are the OS X builds all failing right now anyway? Should I cancel that build? |
|
Looks like it was a Travis outage for all OS X builds but it's been fixed and they're working through the job backlog. See https://www.traviscistatus.com/incidents/nq0sf0srvx8s |
|
@vtjnash: any chance of backporting this? |
only the variable changes need to be propagated on each step to the exception frame (as the whole frame was already synced when handling `Expr(:enter)`) this eliminates an unnecessary exponential pass for handling exception frames along the way, this replaces `(e::Slot).id` with a manually expanded version `slot_id(e)`. Since Slot is a abstract variable, Inference has no way to directly track that the set of subtypes won't change, so it had to be pessimistic and assume that the value was Any. Adding a function barrier removes that difficulty. (cherry-picked from d25e94b, PR #19346)
|
backport is at #19501, which needs a rebase against tk/backports-0.5.1 (and I still suspect this broke a few packages when it went into the nightlies) |
only the variable changes need to be propagated on each step to the exception frame (as the whole frame was already synced when handling `Expr(:enter)`) this eliminates an unnecessary exponential pass for handling exception frames along the way, this replaces `(e::Slot).id` with a manually expanded version `slot_id(e)`. Since Slot is a abstract variable, Inference has no way to directly track that the set of subtypes won't change, so it had to be pessimistic and assume that the value was Any. Adding a function barrier removes that difficulty. (cherry-picked from d25e94b, PR #19346)
In extreme cases, such as #15346, the reduction in the scaling factor appears to be about 10x (also, from this data, both curves appear to be pretty close to n^2, where n is the number of
Base.@teststatements in the function):before:
after: