-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
GH-118093: Don't lose confidence when tracing through 100% biased branches #124813
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
Conversation
The original purpose of only assuming ~90% confidence was to avoid an exponential explosion of traces. for i in range(1<<32):
if i & 32:
a
if i & 64:
b
if i & 128:
c
if i & 256:
d
if i & 512:
e
if i & 1024:
f
if i & 2048:
g
if i & 4096:
h Which goes the same way on each branch for at least 16 iterations, but is unbiased and could produce 256 traces covering If we do merge this, we need some alternative mechanism to avoid producing too many traces for a single piece of code. |
But the old code and the new code handle this highly contrived example the exact same. In fact, the current code accepts up to ten ( I think the answer to pathological cases like this is probably tail fusion during trace compaction... not ending traces early, warming them up 64 hits later, and continuing to project. I think this PR probably handles "real world" code a tiny bit better though, as evidenced by the stats. |
And, to note, we already sort of fuse the tails. After four side exits in a chain, all traces that hit the next instruction will fuse anyways, since we stop projecting when we hit an |
I just ran your example locally. On both |
So this is no worse than what we currently have in terms of generating excessive numbers of traces.
It sounds like you have ideas about how to limit the trace overlap. Could you make an issue for that? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe remove the comment. Otherwise, lgtm.
A 16/16 bias in one direction of a branch is a very strong indicator that we'll stay on trace in that direction. I don't think our current 10% confidence drop is helping us very much.
I originally tried a value closer to 6% (precisely, 1/18th), which follows from applying Laplace's rule of succession. But that still underperformed relative to this PR, which just takes the naive approach of not adjusting our confidence when encountering a highly-biased branch.
~1% faster, with a ~2% increase in uops executed and a ~3% decrease in traces executed (stats).