Skip to content
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

Remove the PREDICT macros. #104610

Closed
markshannon opened this issue May 18, 2023 · 7 comments
Closed

Remove the PREDICT macros. #104610

markshannon opened this issue May 18, 2023 · 7 comments
Labels
performance Performance or resource usage

Comments

@markshannon
Copy link
Member

markshannon commented May 18, 2023

The PREDICT() macros are supposed to speed dispatch on platforms without computed gotos (i.e. Windows).
However the "predictions" are nothing like the actual behavior observed, especially with PEP 659.

We might want to insert something like the PREDICT macros automatically, but the manually inserted ones are useless, or worse, and should be removed.

Linked PRs

@markshannon markshannon added the performance Performance or resource usage label May 18, 2023
@markshannon
Copy link
Member Author

markshannon commented May 18, 2023

Small speedup but might be noise

@brandtbucher
Copy link
Member

brandtbucher commented May 18, 2023

Previous discussion at faster-cpython/ideas#496.

So we should probably look at comparing Windows benchmarking results before doing this (which is easy, since we have the machine now).

@markshannon
Copy link
Member Author

Those are the Windows benchmarking results.

@brandtbucher
Copy link
Member

Ah, sorry, missed that.

@rhettinger
Copy link
Contributor

The PREDICT() macros are supposed to speed dispatch on platforms with computed gotos

The macros sped dispatch on platforms and builds where the big case-statement made unpredictable branches. At the time the macros were introduced, the speed-up was significant because the PREDICT macros made branches that were highly predictable while the other indirect branches where unpredictable.

Subsequently, some processors implemented more advanced branch prediction logic that included indirect branches. To exploit that capability, the computed-goto option was added. Eventually that option became the default as other processors got smarter. We left the macros in because people were still occasionally doing 32-bit builds and sometimes targeting non-intel processors.

In short, the benefits of the PREDICT macro are processor and build specific. We've long known that with computed gotos enabled on a good processor that benchmarks would show no benefit (i.e. you haven't found anything new, that is why the default build was changed). That macros are there help in the remaining situations where the case-statement is still making unpredictable branches, not because of a build option, but because the processor itself has weak branch prediction.

@markshannon
Copy link
Member Author

There is no benefit to the PREDICT macros at the moment. The predictions are wrong. They do not help; they hinder.

Hypothetically, they could be useful if the predictions were accurate, but we would need to generate them from profiling data for that to happen.

@markshannon
Copy link
Member Author

I realize that I originally wrote "with computed gotos" rather than "without computed gotos".
I can see how that could be confusing. Sorry about that.

On platforms without computed gotos, prediction macros have the potential to help dispatch, but in practice, they don't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage
Projects
None yet
Development

No branches or pull requests

4 participants