Skip to content

Commit efb1adb

Browse files
author
Pete Stevenson
authored
Do not adjust pprof sample time by dividing by CPU count. (#1348)
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 <jps@pixielabs.ai>
1 parent 9711f0f commit efb1adb

3 files changed

Lines changed: 5 additions & 7 deletions

File tree

src/shared/pprof/pprof.cc

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@
2424
namespace px {
2525
namespace shared {
2626

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

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

7069
// That sample will record its count and time in nanos.
71-
const uint64_t nanos = count * period_ns / num_cpus;
70+
const uint64_t nanos = count * period_ns;
7271
sample->add_value(count);
7372
sample->add_value(nanos);
7473

src/shared/pprof/pprof.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ namespace px {
2828
namespace shared {
2929

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

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

3636
} // namespace shared
3737
} // namespace px

src/stirling/binaries/stirling_profiler.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,8 @@ class Profiler : public UnitConnector<PerfProfileConnector> {
4444
PX_RETURN_IF_ERROR(BuildHistogram());
4545

4646
// Create the pprof profile.
47-
const uint32_t num_cpus = get_nprocs_conf();
4847
const uint32_t period_ms = FLAGS_stirling_profiler_stack_trace_sample_period_ms;
49-
const auto pprof_pb = px::shared::CreatePProfProfile(num_cpus, period_ms, histo_);
48+
const auto pprof_pb = px::shared::CreatePProfProfile(period_ms, histo_);
5049

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

0 commit comments

Comments
 (0)