-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
cmd/compile: greedy basic block layout #66420
Comments
Implement Pettis&Hanse's greedy algorithm, i.e. bottom-up variant Fixes golang#66420
Change https://go.dev/cl/572975 mentions this issue: |
What does the graph represent? What tests were run? What is a "testcase ID"? What's the difference between the orange and blue lines? How do you interpret the graph to conclude the CL is net positive? Thanks. |
https://go.dev/cl/c/go/+/571535 is another CL related to basic block reordering. |
@mknyszek Thanks for your reminder, I'll take a closer look at it. @mdempsky Hi,
A Chain Graph is composed of chains and edges. It aggregates a series of blocks into chains, which are then interconnected by edges. The purpose of this is to minimize jmp instructions and maximize fallthrough occurrences as much as possible. A vivid and real-life example can be referenced in the image below.
id means testcase1 Each round runs all go tests with count=10 at given CPU set
Changes within 2% are considered reasonable and tolerable fluctuations, which allows us to disregard the majority of cases. However, it has notably improved several cases, specifically those with sharp fluctuations exceeding 6%. On the other hand, this is a simple and time-tested algorithm; variations of it are used worldwide for block layout implementation, and I believe Go is also an excellent candidate for its application. Furthermore, the current graph does not contain PGO information; each edge is either 100% taken or 0% taken. Yet, it still manages to achieve such results. We can optimistically and reasonably expect that a graph augmented with PGO information will yield very favorable outcomes. |
Sorry, I was asking what the graph in your "experimental results" indicated. Your argument seems to be that the graph demonstrates an experimental improvement on the benchmarks, but it looks like noise to me. Typically we look at the results from running
Okay. What are "round1" and "round2"? Can you please instead use "old" and "new"? Or "baseline" and "experiment"? These are unambiguous terms. Thanks. |
In this CL https://go.dev/cl/c/go/+/ the basic block counters are implemented and the likely information is corrected. You can use it in your algorithm. |
The raw benchstat results are as follow They are too big for inspection, that's why I draw the line char. Each point on x-xis represents geomean of bench result for each package. "Round" inicates completing a full run of all go tests. Please let me know if there is nything else that is unclear to you. Thanks!
Thanks, this could be a follow-up enhancement for PH block lyout IMHO. I'll take a closer look on Monday |
Hi @mdempsky , do you have any plan on reviewing this patch? Thanks |
For performance testing best practice is currently to run the "sweet" and "bent" benchmarks, e.g.
then capture the output of the "bench.exe" run. Once you have two runs worth (one with your change and one without), then create a final report based on
This will give you a better picture overall than running the std library package benchmarks (which is what it sounds like you are doing?). |
Hi @mdempsky @thanm , sorry for the delay.
Yes, the above benchmark results are all std library package benchmarks. The
I think current performance results of both |
PING: Anyone? |
When we last left the CL (572975, right?) it had trybot failures. Those need to be fixed at some point. (Generally it is probably better to ping on the CL than the issue.) More generally, it is unclear to me at least that this is actually a performance improvement, as opposed to just noise. I think we're still waiting for an answer for how this compares to CL 571535. That CL uses pgo information, but otherwise is it the same algorithm? A different one? If different, how do we choose which one to use? |
Thank you for the reply. I hope for a positive start to the review process, and I am happy to fix the errors that the trybot points out.
In the first chart of performance results I provided, I ran all std benchmarks (archive, bufio, bytes, crypto, ...) twice, and the results (blue line and orange line) show a high degree of similarity. This means that the same packages consistently receive similar proportions of performance improvements, which is no mere coincidence. I believe it sufficiently demonstrates that the performance results are not noise.
Nearly all basic block algorithms (such as exttsp, cache) are variations of the PH algorithm, which can be derived by altering the PH algorithm's weight computation formula. Another rationale for proposing the current CL is its modest size and clarity, which could simplify the review process. This mitigates one of the concerns I have perceived—why many large patches from external contributors do not get merged/move forward. If we clear the first hurdle, that is, if PH gets integrated, then we can proceed to incorporate PGO information into the PH algorithm. |
Unfortunately just running twice is not enough to avoid noise. Noise comes from many sources, some of which are repeatable given the same binary. See, for example, https://go-review.googlesource.com/c/go/+/562157 I really need to see a few examples of where this makes a difference. A particular change this CL makes in this function makes this benchmark faster. And here's the difference in assembly, note the layout changes make this jump instruction go away. |
@randall77 Sure~ one of examples is bench source
benchstat diff
perf diff
ssa diffvictim of branch-load-miss is The base handles b37 'likely' and 'unlikely' without distinction, while 'opt' shows a preference for 'likely'. This is precisely the issue of concern for 'ph'. See more traces in comment#2 |
Hmm, I don't see that improvement, but my work machine is x86:
I will try again on my arm (M2 ultra) when I get home. |
Looking at the generated arm64 assembly for
The tip compiler does
Whereas with your CL it does
The latter is larger (by 1 unconditional jump) but is probably better (forward branches are default predicted not taken?) when c is seldom true. 3+% is a lot of binary size increase. On microbenchmarks binary size doesn't really matter, but on real applications that extra size can lead to more general slowdowns. It's not clear if that would end up countering the observed speedup or not. |
Yes
Yes
Based on test performance, the cost of increasing binary size is that a few cases see significant performance improvements(10%+), while most cases do not show obvious changes. The performance with PGO may lead to noticeable improvements for the majority of cases, which is exactly what I want to do next. Perhaps in the future, we could consider a configurable block layout algorithm, like GOEXPERIMENT=ph/basic. Additionally, the fact that a large number of compilers apply this algorithm also strengthens our confidence to some extent. |
On my M2 ultra, the performance of your CL is quite good.
|
So, considering all the information given, is it enough to start our review process? Or is there any additional information required from me? Thanks. |
I will take a look at your CL today. I think we still have to figure out the right way to decide when to turn it on. |
I've found a heuristic spot where we could consider sorting adjacent chains together(9fd9a54). For the given chains: before:
Even if [b1 b19] and [b2 b18 b5] are very close to each other, the final generated block order will still arrange them far apart. If we consider arranging adjacent chains together, we could generate better block order: after:
--bench
7.1 update:
|
Hello. I was experimenting with my pgobb and integrated your patch with it. My original patch just adds counters to basic blocks and edits the likely branch information. Integration with your patch was the following:
Later I will share the results of my performance evaluations, but maybe you will find this patch useful for your purposes. PS. Please, see my evaluations here. |
Proposal Details
Fine-grained performance metrics (such as L1-iTLB, IPC, branch-load, branch-load-miss, etc.) and optimization experiments(#63670) have repeatedly shown that block layout can noticably impact code execution efficiency.
In contrast to the current case-by-case tuning approach, I propose adopting the traditional and time-tested Pettis-Hansen (PH) basic block layout algorithm. Its core concept is to place hot block as closely together as possible, allowing basic blocks to fall through whenever feasible, thereby reducing jump instructions. This principle is considered the golden rule in the field of block layout, and state-of-the-art algorithms like extTSP and almost all variants are based on this idea, incorporating advanced heuristic techniques.
The PH algorithm relies on a weighted chain graph, where the weights represent the frequency of edges. In the absence of PGO information, we can only resort to branch prediction results from
likelyadjust
pass. In the future, we can incorporate PGO data as weights to make the algorithm even more effective.Experiment Results
The x-axis represents testcase id, the y-axis indicates the performance change in percentage points, and negative values denote performance improvement.
The text was updated successfully, but these errors were encountered: