Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.Sign up
Speed up registerizeHarder for huge numbers of locals #3368
Stage 2 of being fed up with long compile times. There's some cleanup (as promised in #3364), but the important commit for performance is "Pre-compute possible links and conflicts when considering a junction". The motivation for changes around here was profiling and observing that the highlighted two lines
were really hot.
I have not:
Let me know if you have any more comments.
I'm annoyed github has swallowed a couple of helpful comments for future people looking back in history, so pasting here (I guess this is a lesson to comment on the PR diff/in comments, rather than on commits):
I wouldn't bother looking at the diff unless you're putting them side-by-side and considering them in isolation - this change actually alters the algorithm a bit.
To explain, let's consider 3 bocks (A, B and C) with their exit junctions (aJ, bJ, cJ). B and C are outblocks of aJ, i.e.:
To run through the two versions:
Note that we've got three nested loops - this is why the innermost stuff was so hot, because it gets executed a lot. Even the optimisation is touch and go - turn it into a slightly more expensive check (like checking if it's currently conflicting, rather than is just live in the current junction) and it slows down!
We've effectively flipped the loop inside out, which has the advantage of being clearer - "for variable X, does Y conflict? let's check the blocks Y is present in".
There are a few other tweaks I thought of while writing this, but they can wait.
The break is a 'free' optimisation as there's effectively no overhead in any situation.
I did consider adding a 'check if conflict exists' condition, but it had no noticeable effect I could detect and may cause slower running in some situations. We don't need it because we're inserting into a set - duplicates aren't an issue.
We must always enter this[do...while] loop in order to do a first pass consideration of any blocks added to bWorkSet above. The reason the condition moved to the end was exactly because of the exit junction case - even when we have an empty junction work set (i.e. it was just the exit junction), we need to look at the blocks going into the exit junction.
Ah, yes, good point. I think github is smart about pull diff comments, but I commented on a commit - which is since the rebase not in this pull, I guess! You should get a separate email about it, but it doesn't show up here. Anyhow, all I said there was "makes sense, got it, thanks" or similar.