Skip to content

Commit

Permalink
Do not adjust pprof sample time by dividing by CPU count. (#1348)
Browse files Browse the repository at this point in the history
Summary: Previously we divided the sample time duration by CPU count,
i.e. such that total time would match wall clock time. Here we stop that
practice because it is more correct to report the time spent on core
(even if the total time reported is longer than wall clock time).

Type of change: /kind bug fix

Test Plan: Tests will be merged with #1336.

---------

Signed-off-by: Pete Stevenson <[email protected]>
  • Loading branch information
Pete Stevenson authored May 17, 2023
1 parent 9711f0f commit efb1adb
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 7 deletions.
5 changes: 2 additions & 3 deletions src/shared/pprof/pprof.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@
namespace px {
namespace shared {

PProfProfile CreatePProfProfile(const uint32_t num_cpus, const uint32_t period_ms,
const absl::flat_hash_map<std::string, uint64_t>& histo) {
PProfProfile CreatePProfProfile(const uint32_t period_ms, const PProfHisto& histo) {
// Info on the pprof proto format:
// https://github.com/google/pprof/blob/main/proto/profile.proto

Expand Down Expand Up @@ -68,7 +67,7 @@ PProfProfile CreatePProfProfile(const uint32_t num_cpus, const uint32_t period_m
auto sample = profile.add_sample();

// That sample will record its count and time in nanos.
const uint64_t nanos = count * period_ns / num_cpus;
const uint64_t nanos = count * period_ns;
sample->add_value(count);
sample->add_value(nanos);

Expand Down
4 changes: 2 additions & 2 deletions src/shared/pprof/pprof.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ namespace px {
namespace shared {

using PProfProfile = ::perftools::profiles::Profile;
using PProfHisto = absl::flat_hash_map<std::string, uint64_t>;

// https://github.com/google/pprof/blob/main/proto/profile.proto
PProfProfile CreatePProfProfile(const uint32_t num_cpus, const uint32_t period_ms,
const absl::flat_hash_map<std::string, uint64_t>& histo);
PProfProfile CreatePProfProfile(const uint32_t period_ms, const PProfHisto& histo);

} // namespace shared
} // namespace px
3 changes: 1 addition & 2 deletions src/stirling/binaries/stirling_profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,8 @@ class Profiler : public UnitConnector<PerfProfileConnector> {
PX_RETURN_IF_ERROR(BuildHistogram());

// Create the pprof profile.
const uint32_t num_cpus = get_nprocs_conf();
const uint32_t period_ms = FLAGS_stirling_profiler_stack_trace_sample_period_ms;
const auto pprof_pb = px::shared::CreatePProfProfile(num_cpus, period_ms, histo_);
const auto pprof_pb = px::shared::CreatePProfProfile(period_ms, histo_);

// Write the pprof profile to disk.
std::fstream outfile(FLAGS_pprof_pb_file, std::ios::out | std::ios::trunc | std::ios::binary);
Expand Down

0 comments on commit efb1adb

Please sign in to comment.