Run perf continuously #113
Conversation
This is done for 2 reasons: 1. More correct & accurate: collect samples *all* the time, without excluding any interval. 2. For performance reasons - this still has to be benched, but we believe that avoiding the start-stop-start-stop-... of "perf record" will be beneficial.
We now profile foreverrrr
We have decided to get rid of it, in favor of true continuity. |
I don't see why it's needed *here*. We can add such a check in the main file.
buildid recording & cache is enabled by default in perf, but disabled in "switch output" mode (see explanation in Linux commit 0c1d46a8796e830). We had it enabled (and specifically handled it in #17) because I had in mind that it's required for perf to properly resolve symbols across namespaces. I have proved this incorrect by testing, so there's no reason to keep these flags on - we run "script" right after record is done, it's okay to access files just by path.
|
|
||
| logger = get_logger_adapter(__name__) | ||
|
|
||
| PERF_BUILDID_DIR = os.path.join(TEMPORARY_STORAGE_PATH, "perf-buildids") | ||
|
|
||
| class PerfProcess: |
There was a problem hiding this comment.
Looks like your can derive from ProfilerBase here and get away with much of the code
There was a problem hiding this comment.
Not really. It's not the same API. For example, its result isn't ProcessToStackSampleCounters but plain perf script.
| def stop(self) -> None: | ||
| if self._process is not None: | ||
| self._process.terminate() # okay to call even if process is already dead | ||
| self._process.wait() |
There was a problem hiding this comment.
terminate sends SIGTERM, which kills perf.
But I agree timeouts are nice nonetheless
There was a problem hiding this comment.
But we'll add timeouts somewhen else (we have plenty of other missing timeouts - for example, when running py-spy, I happened to see it hang). So in another PR. There's an issue open (or I will open one)
|
Changes LGTM |
|
I'll update the README |
Description
This PR changes
SystemProfiler(which runsperf) to run in true continuous mode (like PyPerf & phpspy, and more to come, hopefully).Continuous mode produces more accurate results because we don't miss anything - we're always sampling. Also, avoiding the re-starts of
perfshould yield better performance.Supersedes #103.
Based on #89.Motivation and Context
Performance & accuracy.
How Has This Been Tested?
I will decide on more elaborate tests & preferably add them to our test suite.
Checklist:
does it really help in benchmarks & do we see lessI ranperfin the flamegraph now :)masterand this branch on the same machine, and compared the numbers of--log-cpu-usage& how much ofperfI see in the graph. Both went down.--profiling-intervaloption? Withperfnow being continuous, there's no sense in allowingduration != interval.