Reduce overhead of sampling profiler by having only one thread do it by abadams · Pull Request #6433 · halide/Halide · GitHub
Skip to content

Reduce overhead of sampling profiler by having only one thread do it#6433

Merged
abadams merged 4 commits into
masterfrom
abadams/sampling_profiler_overhead_v2
Dec 2, 2021
Merged

Reduce overhead of sampling profiler by having only one thread do it#6433
abadams merged 4 commits into
masterfrom
abadams/sampling_profiler_overhead_v2

Conversation

@abadams

@abadams abadams commented Nov 19, 2021

Copy link
Copy Markdown
Member

The current built-in profiler has a lot of overhead for fine-grained compute_at schedules. E.g. for the bgu app it inflates runtime by about 50% to turn on the profiler. This is happening because all threads are writing their current state to the same cache line, causing a lot of cross-cache traffic. Each one of these writes is effectively a cache miss.

This PR changes it so that whenever we have lots of threads all doing the same thing (so in a leaf parallel loop body and not inside a fork node), one of them gets elected to write to that status field. The election is done by racing to grab a pipeline-scope token using an atomic op. The winner does the reporting. This speeds things up in two ways: First, the threads that don't write don't incur the cache misses. Second, the thread that does write can keep that line in cache, with the sampler thread just snooping on the bus traffic when it wants to read instead of invalidating the cache line (assuming I remember how MESI works properly).

@steven-johnson steven-johnson self-requested a review November 23, 2021 17:26

@steven-johnson steven-johnson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but the explanatory description in this PR should really be added in code comments somewhere.

Comment thread src/Profiling.cpp Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ubernit: this is an unusual formatting for our code; we almost always put member var declarations one-per-line.

@abadams

abadams commented Nov 24, 2021

Copy link
Copy Markdown
Member Author

I'm a bit nervous about how hard it is to test this. I'd appreciate it if a someone could run an important production pipeline at Google before and after and tell me if the profile looks reasonable.

@abadams

abadams commented Dec 2, 2021

Copy link
Copy Markdown
Member Author

Actually a systematic test of the app suite is probably enough. I'll do that and report back.

@abadams

abadams commented Dec 2, 2021

Copy link
Copy Markdown
Member Author

@abadams abadams merged commit 5cf9ae5 into master Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants