perf: Use /tmp/gprofiler/perf-buildids as the buildid cache for perf #17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Avoid cluttering other parts of the filesystem (by default, it uses /root/.debug). When gprofiler runs as a container, it doesn't really matter (both /root/ and /tmp are just part of the container filesystem). But we plan to allow running gprofiler as an executable, and in that case, we should keep our files in /tmp/gprofiler.
DanielShaulov
approved these changes
Apr 8, 2021
Jongy
added a commit
that referenced
this pull request
Jul 15, 2021
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.
Jongy
added a commit
that referenced
this pull request
Jul 18, 2021
* perf: Run perf in continuous mode, utilizing --switch-output. This is done for 2 reasons: 1. More correct & accurate: collect samples *all* the time, without excluding any interval. 2. For performance reasons - no overhead of starting/stopping perf. * Remove profiling interval, it makes no sense to maintain it with perf running in continuous mode. * perf: Remove disk check I don't see why it's needed *here*. We can add such a check in the main file. But I don't think it's needed now. * perf: Don't use buildid recording & cache 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Avoid cluttering other parts of the filesystem (by default, it uses /root/.debug).
When gprofiler runs as a container, it doesn't really matter (both /root/ and /tmp
are just part of the container filesystem).
But we plan to allow running gprofiler as an executable, and in that case, we
should keep our files in /tmp/gprofiler.
Motivation and Context
Keep our files in
/tmp/gprofiler.How Has This Been Tested?
I ran the gprofiler container with this fix and verified that
/root/.debugis not used anymore, and that all cached DSOs go to/tmp/gprofiler/....Types of changes
Checklist: