#68241 been blocked on review from @pnkfelix for a few weeks now. That PR is in turn blocking significant perf improvements in #68528. Would someone be willing to review all or part of #68241? I say "part" because that PR includes several small, independent changes involving and debugging that are closely related to the borrowck dataflow changes but could be split out. cc @T-compiler
Yeah I’m sorry, a bunch of my review requests have gone mishandled for far too long
It didn’t help that I didn’t get as much free time at the Mozilla all hands as I was expected
I won’t get to it until Monday at the earliest
No worries. Just hoping to find someone who could work steal.
@ecstatic-morse If you split out the first 5 commits into a separate PR, I'd be happy to r+ that. I may be able to review additional parts later as well.
Thanks @Wesley Wiser! I split the graphviz changes into #69005.
Reviewed & approved
@ecstatic-morse just so we're on the same page regarding expectations for this PR : when I followed the perf.rlo link, the the task-clock and wall-times show a fair amount of red (i.e. regressions). They might be acceptable regressions, but reading the description of the PR itself, I wasn't expecting to see much regression at all.
@ecstatic-morse am I misunderstanding the perf results? Or looking at an outdated link:?
I expect this PR to be roughly performance neutral: some regressions due to additional boilerplate and a more fine-grained cursor offset by skipping block-transfer function caching.
cc @simulacrum I am assuming that the changes in wall-time measurements were due to variance. For example, here's the task-clock results for an initial version of the PR. This is without the MIR visitor or the block transfer function skipping. I don't think these results represent a true improvement, even though there is more green.
Btw, lemme rebase away the graphviz changes
If you like I can split out the block transfer function caching optimization and/or do another perf run
Otherwise, we can always revert this if it actually causes a persistent wall-time regression. We'll be able to see it in the graphs.
I mostly wanted clarity about your expectations