[llvm-profgen] Add process filter for perf reader

[wenlei]

Summary

For profile generation, we need to filter raw perf samples for binary of interest. Sometimes binary name along isn't enough as we can have binary of the same name running in the system. This change adds a process id filter to allow users to further disambiguiate the input raw samples.

[hoy]

llvm/tools/llvm-profgen/PerfReader.h

549

Can we just check against the switch ProcessId in convertPerfDataToTrace? Feel like PID is perf data specific and changing the general interface here may not be necessary. We have other filters that are for a particular reader, as an example ignore-stack-samples.

[wenlei]

llvm/tools/llvm-profgen/PerfReader.h

549

Can we just check against the switch ProcessId in convertPerfDataToTrace?

We also need to check PID in updateBinaryAddress. I think PID is in the same category as Binary, both are filters for perf events.

Feel like PID is perf data specific and changing the general interface here may not be necessary.

Isn't any perf reader type under PerfReaderBase supposed to be perf data specific? The only thing that is not perf data related is UnsymbolizedProfileReader, but then I think the real problem is really why we have UnsymbolizedProfileReader inheriting from perf reader while it's clearly not reading perf data/script.

[hoy]

llvm/tools/llvm-profgen/PerfReader.h

549

We also need to check PID in updateBinaryAddress.

The PID filter isn't that meaningful in perf script file. Though we can still use pid to filter out the mmap events there, but the main LBR/stack samples mixed from multiple processes are not separable. So PID filter should be applied when converting perf.data to perf script, which makes me think it is perf data specific.

Isn't any perf reader type under PerfReaderBase supposed to be perf data specific?

Not necessarily. Perf script and our internal lbr profile do not see
everything that perf.data has. The two formats assume all data in them are for one process. Do you see with pid filter, convertPerfDataToTrace still gives mixed mmap evens from all processes?

[wenlei]

llvm/tools/llvm-profgen/PerfReader.h

549

We also need to check PID in updateBinaryAddress.

The PID filter isn't that meaningful in perf script file. Though we can still use pid to filter out the mmap events there, but the main LBR/stack samples mixed from multiple processes are not separable. So PID filter should be applied when converting perf.data to perf script, which makes me think it is perf data specific.

We need to use pid to filter mmap event, otherwise we don't know which mmap to use to set base address and the result would be wrong, so pid is meaningful for perf script.

Isn't any perf reader type under PerfReaderBase supposed to be perf data specific?

Not necessarily. Perf script and our internal lbr profile do not see
everything that perf.data has. The two formats assume all data in them are for one process.

Maybe I didn't understand what you're trying to say. I don't see a problem because: 1) there's no fundamental difference between perf data and perf script; 2) By the name PerfReaderBase, the entire hierarchy is perf specific; 3) then adding pid to general interface isn't a problem.

Also I don't see pid is being very different from binary name, it's another filter, similar to binary name.

Do you see with pid filter, convertPerfDataToTrace still gives mixed mmap evens from all processes?

Yes, if you are asking about output from perf script .. with pid filter. mmap from processes are still there.

[hoy]

llvm/tools/llvm-profgen/PerfReader.h

549

I see, looks like mmap events are special from other sample events in that they cannot be filtered by pid. I was hoping that perf script output could be made dedicate to a process since that's what perf script --pid is shooting for? Once filtered, the processing of perf script output should no longer need to deal with pid again. This sounds like a bug of perf script.

The PerfReaderBase hierarchy seems improper in naming. Both perf.data and perf script output are processed by PerfScriptReader and the base class PerfReaderBase is there only to provide an interface to create a specific reader instance, including UnsymbolizedProfileReader. I think the intention of including UnsymbolizedProfileReader in the hierarchy was to avoid a special path in the driver, but then PerfReaderBase should be named ReaderBase or something.

It then makes sense to pass pid to the perf reader hierarchy and separating UnsymbolizedProfileReader out. The separation can be done in a different patch.

Use optional<uint32_t> for PIDFilter in case zero is a valid kernel process?

[wenlei]

llvm/tools/llvm-profgen/PerfReader.h

549

I was hoping that perf script output could be made dedicate to a process since that's what perf script --pid is shooting for?

No, as I said that's what you get with perf script. I added the change in updateBinaryAddress after I realized that.

This sounds like a bug of perf script.

I'm not sure, unless there's a clear spec this is basically implementation dependent. As you mentioned, mmap can be considered special.

I think the intention of including UnsymbolizedProfileReader in the hierarchy was to avoid a special path in the driver, but then PerfReaderBase should be named ReaderBase or something.

I don't think make non-perf reader a subclass of perf reader is a good idea, and avoiding special path in driver doesn't justify that.

The PerfReaderBase hierarchy seems improper in naming.

The hierarchy is improper, but I'm not sure if it's just a naming thing. One can also argue perhaps it's better to have special path in driver like llvm-sample-profile. It was a similar situation and we settled on having special path there. Regardless this is unrelated to this patch.

Use optional<uint32_t> for PIDFilter in case zero is a valid kernel process?

I'd prefer simplicity unless proven necessary. I don't think zero process id is something we'd worry about. But I don't have strong opinion on this.

[hoy]

llvm/tools/llvm-profgen/PerfReader.h

549

I'd prefer simplicity unless proven necessary. I don't think zero process id is something we'd worry about. But I don't have strong opinion on this.

In case we do with Kernel with autofdo where zero can be a valid kernel process id. Also using optional sounds a trend as I see more refactoring changes for that in the code base. The way I see it, it helps achieve simplicity, otherwise you would need an explicit comment somewhere about using zero as an invalid pid.

[wenlei]

llvm/tools/llvm-profgen/PerfReader.h

549

I'd prefer simplicity unless proven necessary. I don't think zero process id is something we'd worry about. But I don't have strong opinion on this.

In case we do with Kernel with autofdo where zero can be a valid kernel process id. Also using optional sounds a trend as I see more refactoring changes for that in the code base. The way I see it, it helps achieve simplicity, otherwise you would need an explicit comment somewhere about using zero as an invalid pid.

Sounds good. I just realized that kernel scheduler actually uses 0 process id.

[wenlei]

Use Optional<> for PIDFilter

[hoy]

lgtm, thanks.

[Meinersbur]

llvm/tools/llvm-profgen/PerfReader.h

16

This broke the Windows build. bits/stdint-uintn.h is platform-specific.

[wenlei]

llvm/tools/llvm-profgen/PerfReader.h

16

Ops, sorry this change wasn't intentional, just pushed in a fix to remove it. https://reviews.llvm.org/rG526af13ebaa9

llvm/tools/llvm-profgen/PerfReader.h

 //===-- PerfReader.h - perfscript reader -----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TOOLS_LLVM_PROFGEN_PERFREADER_H
 #define LLVM_TOOLS_LLVM_PROFGEN_PERFREADER_H
 #include "ErrorHandling.h"
 #include "ProfiledBinary.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Regex.h"
+#include <bits/stdint-uintn.h>
 #include <cstdint>
 #include <fstream>
 #include <list>
 #include <map>
 #include <vector>
 
 using namespace llvm;
 using namespace sampleprof;

 public:
   PerfReaderBase(ProfiledBinary *B, StringRef PerfTrace)
       : Binary(B), PerfTraceFile(PerfTrace) {
     // Initialize the base address to preferred address.
     Binary->setBaseAddress(Binary->getPreferredBaseAddress());
   };
   virtual ~PerfReaderBase() = default;
   static std::unique_ptr<PerfReaderBase> create(ProfiledBinary *Binary,
-                                                PerfInputFile &PerfInput);
+                                                PerfInputFile &PerfInput,
+                                                Optional<uint32_t> PIDFilter);
 
   // Entry of the reader to parse multiple perf traces
   virtual void parsePerfTraces() = 0;
   const ContextSampleCounterMap &getSampleCounters() const {
     return SampleCounters;
   }
   bool profileIsCSFlat() { return ProfileIsCSFlat; }
 
 protected:
   ProfiledBinary *Binary = nullptr;
   StringRef PerfTraceFile;
 
   ContextSampleCounterMap SampleCounters;
   bool ProfileIsCSFlat = false;
 
   uint64_t NumTotalSample = 0;
   uint64_t NumLeafExternalFrame = 0;
   uint64_t NumLeadingOutgoingLBR = 0;
 };
 
 // Read perf script to parse the events and samples.
 class PerfScriptReader : public PerfReaderBase {
 public:
-  PerfScriptReader(ProfiledBinary *B, StringRef PerfTrace)
-      : PerfReaderBase(B, PerfTrace){};
+  PerfScriptReader(ProfiledBinary *B, StringRef PerfTrace,
+                   Optional<uint32_t> PID)
+      : PerfReaderBase(B, PerfTrace), PIDFilter(PID){};
 
   // Entry of the reader to parse multiple perf traces
   virtual void parsePerfTraces() override;
   // Generate perf script from perf data
   static PerfInputFile convertPerfDataToTrace(ProfiledBinary *Binary,
-                                              PerfInputFile &File);
+                                              PerfInputFile &File,
+                                              Optional<uint32_t> PIDFilter);
   // Extract perf script type by peaking at the input
   static PerfContent checkPerfScriptType(StringRef FileName);
 
 protected:
   // The parsed MMap event
   struct MMapEvent {
     uint64_t PID = 0;
     uint64_t Address = 0;
protected:
   virtual void generateUnsymbolizedProfile();
   void writeUnsymbolizedProfile(StringRef Filename);
   void writeUnsymbolizedProfile(raw_fd_ostream &OS);
 
   // Samples with the repeating time generated by the perf reader
   AggregatedCounter AggregatedSamples;
   // Keep track of all invalid return addresses
   std::set<uint64_t> InvalidReturnAddresses;
+  // PID for the process of interest
+  Optional<uint32_t> PIDFilter;
 };
 
 /*
   The reader of LBR only perf script.
   A typical LBR sample is like:
     40062f 0x4005c8/0x4005dc/P/-/-/0   0x40062f/0x4005b0/P/-/-/0 ...
           ... 0x4005c8/0x4005dc/P/-/-/0
 */
 class LBRPerfReader : public PerfScriptReader {
 public:
-  LBRPerfReader(ProfiledBinary *Binary, StringRef PerfTrace)
-      : PerfScriptReader(Binary, PerfTrace){};
+  LBRPerfReader(ProfiledBinary *Binary, StringRef PerfTrace,
+                Optional<uint32_t> PID)
+      : PerfScriptReader(Binary, PerfTrace, PID){};
   // Parse the LBR only sample.
   virtual void parseSample(TraceStream &TraceIt, uint64_t Count) override;
 };
 
 /*
   Hybrid perf script includes a group of hybrid samples(LBRs + call stack),
   which is used to generate CS profile. An example of hybrid sample:
     4005dc    # call stack leaf
     400634
     400684    # call stack root
     0x4005c8/0x4005dc/P/-/-/0   0x40062f/0x4005b0/P/-/-/0 ...
           ... 0x4005c8/0x4005dc/P/-/-/0    # LBR Entries
 */
 class HybridPerfReader : public PerfScriptReader {
 public:
-  HybridPerfReader(ProfiledBinary *Binary, StringRef PerfTrace)
-      : PerfScriptReader(Binary, PerfTrace){};
+  HybridPerfReader(ProfiledBinary *Binary, StringRef PerfTrace,
+                   Optional<uint32_t> PID)
+      : PerfScriptReader(Binary, PerfTrace, PID){};
   // Parse the hybrid sample including the call and LBR line
   void parseSample(TraceStream &TraceIt, uint64_t Count) override;
   void generateUnsymbolizedProfile() override;
 
 private:
   // Unwind the hybrid samples after aggregration
   void unwindSamples();
 };

llvm/tools/llvm-profgen/PerfReader.cpp

bool VirtualUnwinder::unwind(const PerfSample *Sample, uint64_t Repeat) {
   }
   // As samples are aggregated on trie, record them into counter map
   collectSamplesFromFrameTrie(State.getDummyRootPtr());
 
   return true;
 }
 
 std::unique_ptr<PerfReaderBase>
-PerfReaderBase::create(ProfiledBinary *Binary, PerfInputFile &PerfInput) {
+PerfReaderBase::create(ProfiledBinary *Binary, PerfInputFile &PerfInput,
+                       Optional<uint32_t> PIDFilter) {
   std::unique_ptr<PerfReaderBase> PerfReader;
 
   if (PerfInput.Format == PerfFormat::UnsymbolizedProfile) {
     PerfReader.reset(
         new UnsymbolizedProfileReader(Binary, PerfInput.InputFile));
     return PerfReader;
   }
 
   // For perf data input, we need to convert them into perf script first.
   if (PerfInput.Format == PerfFormat::PerfData)
-    PerfInput = PerfScriptReader::convertPerfDataToTrace(Binary, PerfInput);
+    PerfInput =
+        PerfScriptReader::convertPerfDataToTrace(Binary, PerfInput, PIDFilter);
 
   assert((PerfInput.Format == PerfFormat::PerfScript) &&
          "Should be a perfscript!");
 
   PerfInput.Content =
       PerfScriptReader::checkPerfScriptType(PerfInput.InputFile);
   if (PerfInput.Content == PerfContent::LBRStack) {
-    PerfReader.reset(new HybridPerfReader(Binary, PerfInput.InputFile));
+    PerfReader.reset(
+        new HybridPerfReader(Binary, PerfInput.InputFile, PIDFilter));
   } else if (PerfInput.Content == PerfContent::LBR) {
-    PerfReader.reset(new LBRPerfReader(Binary, PerfInput.InputFile));
+    PerfReader.reset(new LBRPerfReader(Binary, PerfInput.InputFile, PIDFilter));
   } else {
     exitWithError("Unsupported perfscript!");
   }
 
   return PerfReader;
 }
 
-PerfInputFile PerfScriptReader::convertPerfDataToTrace(ProfiledBinary *Binary,
-                                                       PerfInputFile &File) {
+PerfInputFile PerfScriptReader::convertPerfDataToTrace(
+    ProfiledBinary *Binary, PerfInputFile &File, Optional<uint32_t> PIDFilter) {
   StringRef PerfData = File.InputFile;
   // Run perf script to retrieve PIDs matching binary we're interested in.
   auto PerfExecutable = sys::Process::FindInEnvPath("PATH", "perf");
   if (!PerfExecutable) {
     exitWithError("Perf not found.");
   }
   std::string PerfPath = *PerfExecutable;
   std::string PerfTraceFile = PerfData.str() + ".script.tmp";
PerfInputFile PerfScriptReader::convertPerfDataToTrace(
   TraceStream TraceIt(PerfTraceFile);
   std::string PIDs;
   std::unordered_set<uint32_t> PIDSet;
   while (!TraceIt.isAtEoF()) {
     MMapEvent MMap;
     if (isMMap2Event(TraceIt.getCurrentLine()) &&
         extractMMap2EventForBinary(Binary, TraceIt.getCurrentLine(), MMap)) {
       auto It = PIDSet.emplace(MMap.PID);
-      if (It.second) {
+      if (It.second && (!PIDFilter || MMap.PID == *PIDFilter)) {
         if (!PIDs.empty()) {
           PIDs.append(",");
         }
         PIDs.append(utostr(MMap.PID));
       }
     }
     TraceIt.advance();
   }

 }
 
 void PerfScriptReader::updateBinaryAddress(const MMapEvent &Event) {
   // Drop the event which doesn't belong to user-provided binary
   StringRef BinaryName = llvm::sys::path::filename(Event.BinaryPath);
   if (Binary->getName() != BinaryName)
     return;
 
+  // Drop the event if process does not match pid filter
+  if (PIDFilter && Event.PID != *PIDFilter)
+    return;
+
   // Drop the event if its image is loaded at the same address
   if (Event.Address == Binary->getBaseAddress()) {
     Binary->setIsLoadedByMMap(true);
     return;
   }
 
   if (Event.Offset == Binary->getTextSegmentOffset()) {
     // A binary image could be unloaded and then reloaded at different

llvm/tools/llvm-profgen/llvm-profgen.cpp

SampleProfFilename("llvm-sample-profile",
                        cl::desc("Path of the LLVM sample profile"),
                        cl::cat(ProfGenCategory));
 
 static cl::opt<std::string>
     BinaryPath("binary", cl::value_desc("binary"), cl::Required,
                cl::desc("Path of profiled executable binary."),
                cl::cat(ProfGenCategory));
 
+static cl::opt<uint32_t>
+    ProcessId("pid", cl::value_desc("process Id"), cl::ZeroOrMore, cl::init(0),
+              cl::desc("Process Id for the profiled executable binary."),
+              cl::cat(ProfGenCategory));
+
 static cl::opt<std::string> DebugBinPath(
     "debug-binary", cl::value_desc("debug-binary"), cl::ZeroOrMore,
     cl::desc("Path of debug info binary, llvm-profgen will load the DWARF info "
              "from it instead of the executable binary."),
     cl::cat(ProfGenCategory));
 
 extern cl::opt<bool> ShowDisassemblyOnly;
 extern cl::opt<bool> ShowSourceLocations;
if (SampleProfFilename.getNumOccurrences()) {
     Reader->read();
     std::unique_ptr<ProfileGeneratorBase> Generator =
         ProfileGeneratorBase::create(Binary.get(),
                                      std::move(Reader->getProfiles()),
                                      Reader->profileIsCSFlat());
     Generator->generateProfile();
     Generator->write();
   } else {
+    Optional<uint32_t> PIDFilter;
+    if (ProcessId.getNumOccurrences())
+      PIDFilter = ProcessId;
     PerfInputFile PerfFile = getPerfInputFile();
     std::unique_ptr<PerfReaderBase> Reader =
-        PerfReaderBase::create(Binary.get(), PerfFile);
+        PerfReaderBase::create(Binary.get(), PerfFile, PIDFilter);
     // Parse perf events and samples
     Reader->parsePerfTraces();
 
     if (SkipSymbolization)
       return EXIT_SUCCESS;
 
     std::unique_ptr<ProfileGeneratorBase> Generator =
         ProfileGeneratorBase::create(Binary.get(), &Reader->getSampleCounters(),
                                      Reader->profileIsCSFlat());
     Generator->generateProfile();
     Generator->write();
   }
 
   return EXIT_SUCCESS;
 }