[profile] in-process profile merging support Part-3

[davidxl]

Summary

With this patch, the merging feature will be fully functional.

There are two way to turn on profile merging: command line and environment variable. They both share the same mechanism %[0-9]m specifiler in the name pattern. The numeric modifier in the %m specifier specifies the size of the shared profile file pool size. By default %m is equivalent to %1m which means all processes from the same binary share one single profile file.

Example. Do an instrumented clang build with option -fprofile-instr-generate=/tmp/clang_raw_profile_%m and run clang test or do a clang bootstrap, all instrumented binaries such as clang, opt, llc, llvm-profdata etc will dump profile data into its own profile data file.

Running instrumented binary with LLVM_PROFILE_FILE=..._%m will have the same effect.

[vsk]

Awesome! Some comments/questions inline --

lib/profile/InstrProfilingFile.c

99

Possible dropped error here

102

Ditto

104

Could we skip these two checks by moving the call to __llvm_profile_check_compatibility here?

110

0 -> NULL

112

MAP_FAILED instead of -1?

149

I think any successful call to doProfileMerging ought to be followed by COMPILER_RT_FTRUNCATE. Sink it into the function?

155

Ditto, fseek error

262

Should this check be: if (MergingEnabled && (FilenamePat[I] == 'm' || FilenamePat[I + 1] == 'm'))? Why check whether the character after the 'm' is nil?

296

Could you add a comment explaining that '24' comes from lprofGetLoadModuleSignature? In fact, it might be better to hide this detail in a helper function in InstrProfilingMerge.c.

343

Can this predicate be shared with parseFilenamePattern? I'd prefer not to have it duplicated.

test/profile/instrprof-basic.c

18

Is the lprofGetLoadModuleSignature code path covered?

[davidxl]

lib/profile/InstrProfilingFile.c

104

yes

149

doProfileMerge just merges profile into memory, and the wrapper OpenFileForMerge does the rest to prepare for writing (including truncating). Another reason it is here is that doProfileMerge has multiple 'return 0' so extracting truncation code out is cleaner.

262

The second error condition is that %m specifier is not at the end of the pat. This restriction is for convenience. I don't see a reason to support %[0-9]m in arbitrary locations.

test/profile/instrprof-basic.c

18

The plan is to have basic test coverage for now. More tests will be added as follow up as things stablizes.

[davidxl]

Addressed vsk's comments.

[vsk]

lib/profile/InstrProfilingFile.c

149

Ok

238

I think "binary-%m.profraw" could be a common pattern; moreover I don't see a reason to not support this. Would removing this restriction require adding more code to getCurFilenameLength()/getCurFilename()?

test/profile/instrprof-basic.c

18

Ok

[davidxl]

Relax the constraint that %m can only be at the end of the pattern. It can now appear anywhere.

[vsk]

LGTM with one nit.

lib/profile/InstrProfilingFile.c

34

I don't think this is used anywhere, and it might be a bit cumbersome. Delete?

[silvas]

Looks like Vedant covered most of the things I wanted to mention. LGTM with a couple nits.

lib/profile/InstrProfilingFile.c

226

Naively, this seems like it may read off the end. A comment explaining why not would probably be helpful.

296

typo: ty should be by

lib/profile/InstrProfilingInternal.h

160

I think you meant "executable" instead of "executation".

lib/profile/InstrProfilingMerge.c

24

I think it is more accurate to say "a module signature" instead of "the module signature". Ideally we would e.g. hash all the names or something else a bit more robust (but I think that just the sizes will be fine for now).

[davidxl]

lib/profile/InstrProfilingInternal.h

160

Fixed.

lib/profile/InstrProfilingFile.c

 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #ifdef _MSC_VER
 /* For _alloca. */
 #include <malloc.h>
 #endif
+#if defined(_WIN32)
+#include "WindowsMMap.h"
+/* For _chsize_s */
+#include <io.h>
+#else
+#include <sys/file.h>
+#include <sys/mman.h>
+#include <unistd.h>
+#if defined(__linux__)
+#include <sys/types.h>
+#endif
+#endif
 
+#define PROFILE_POOL_MAX_SIZE 9
 #define MAX_PID_SIZE 16
 /* Data structure holding the result of parsed filename pattern.  */
 typedef struct lprofFilename {
   /* File name string possibly with %p or %h specifiers. */
   const char *FilenamePat;
   char PidChars[MAX_PID_SIZE];
   char Hostname[COMPILER_RT_MAX_HOSTLEN];
   unsigned NumPids;
   unsigned NumHosts;
+  /* When in-process merging is enabled, this parameter specifies
+   * the total number of profile data files shared by all the processes
+   * spawned from the same binary. By default the value is 1. If merging
+   * is not enabled, its value should be 0. This parameter is specified
+   * by the %[0-9]m specifier. For instance %2m enables merging using
+   * 2 profile data files. %1m is equivalent to %m. Also %m specifier
+   * can only appear once at the end of the name pattern. */
+  unsigned MergePoolSize;
 } lprofFilename;
 
-lprofFilename lprofCurFilename = {0, {0}, {0}, 0, 0};
+lprofFilename lprofCurFilename = {0, {0}, {0}, 0, 0, 0};
 
 int getpid(void);
 static int getCurFilenameLength();
 static const char *getCurFilename(char *FilenameBuf);
+static unsigned doMerging() { return lprofCurFilename.MergePoolSize; }
 
 /* Return 1 if there is an error, otherwise return  0.  */
 static uint32_t fileWriter(ProfDataIOVec *IOVecs, uint32_t NumIOVecs,
                            void **WriterCtx) {
   uint32_t I;
   FILE *File = (FILE *)*WriterCtx;
   for (I = 0; I < NumIOVecs; I++) {
     if (fwrite(IOVecs[I].Data, IOVecs[I].ElmSize, IOVecs[I].NumElm, File) !=
static void setupIOBuffer() {
   const char *BufferSzStr = 0;
   BufferSzStr = getenv("LLVM_VP_BUFFER_SIZE");
   if (BufferSzStr && BufferSzStr[0]) {
     VPBufferSize = atoi(BufferSzStr);
     DynamicBufferIOBuffer = (uint8_t *)calloc(VPBufferSize, 1);
   }
 }
 
+/* Read profile data in \c ProfileFile and merge with in-memory
+   profile counters. Returns -1 if there is fatal error, otheriwse
+   0 is returned.
+*/
+static int doProfileMerging(FILE *ProfileFile) {
+  uint64_t ProfileFileSize;
+  char *ProfileBuffer;
+
+  if (fseek(ProfileFile, 0L, SEEK_END) == -1) {
+    PROF_ERR("Unable to merge profile data, unable to get size: %s\n",
+             strerror(errno));
+    return -1;
+  }
+  ProfileFileSize = ftell(ProfileFile);
+
+  /* Restore file offset.  */
+  if (fseek(ProfileFile, 0L, SEEK_SET) == -1) {
+    PROF_ERR("Unable to merge profile data, unable to rewind: %s\n",
+             strerror(errno));
+    return -1;
+  }
+
+  /* Nothing to merge.  */
+  if (ProfileFileSize < sizeof(__llvm_profile_header)) {
+    if (ProfileFileSize)
+      PROF_WARN("Unable to merge profile data: %s\n",
+                "source profile file is too small.");
+    return 0;
+  }
+
+  ProfileBuffer = mmap(NULL, ProfileFileSize, PROT_READ, MAP_SHARED | MAP_FILE,
+                       fileno(ProfileFile), 0);
+  if (ProfileBuffer == MAP_FAILED) {
+    PROF_ERR("Unable to merge profile data, mmap failed: %s\n",
+             strerror(errno));
+    return -1;
+  }
+
+  if (__llvm_profile_check_compatibility(ProfileBuffer, ProfileFileSize)) {
+    (void)munmap(ProfileBuffer, ProfileFileSize);
+    PROF_WARN("Unable to merge profile data: %s\n",
+              "source profile file is not compatible.");
+    return 0;
+  }
+
+  /* Now start merging */
+  __llvm_profile_merge_from_buffer(ProfileBuffer, ProfileFileSize);
+  (void)munmap(ProfileBuffer, ProfileFileSize);
+
+  return 0;
+}
+
+/* Open the profile data for merging. It opens the file in r+b mode with
+ * file locking.  If the file has content which is compatible with the
+ * current process, it also reads in the profile data in the file and merge
+ * it with in-memory counters. After the profile data is merged in memory,
+ * the original profile data is truncated and gets ready for the profile
+ * dumper. With profile merging enabled, each executable as well as any of
+ * its instrumented shared libraries dump profile data into their own data file.
+*/
+static FILE *openFileForMerging(const char *ProfileFileName) {
+  FILE *ProfileFile;
+  int rc;
+
+  ProfileFile = lprofOpenFileEx(ProfileFileName);
+  if (!ProfileFile)
+    return NULL;
+
+  rc = doProfileMerging(ProfileFile);
+  if (rc || COMPILER_RT_FTRUNCATE(ProfileFile, 0L) ||
+      fseek(ProfileFile, 0L, SEEK_SET) == -1) {
+    PROF_ERR("Profile Merging of file %s failed: %s\n", ProfileFileName,
+             strerror(errno));
+    fclose(ProfileFile);
+    return NULL;
+  }
+  fseek(ProfileFile, 0L, SEEK_SET);
+  return ProfileFile;
+}
+
 /* Write profile data to file \c OutputName.  */
 static int writeFile(const char *OutputName) {
   int RetVal;
   FILE *OutputFile;
 
-  /* Append to the file to support profiling multiple shared objects. */
+  if (!doMerging())
     OutputFile = fopen(OutputName, "ab");
+  else
+    OutputFile = openFileForMerging(OutputName);
+
   if (!OutputFile)
     return -1;
 
   FreeHook = &free;
   setupIOBuffer();
   RetVal = lprofWriteData(fileWriter, OutputFile, lprofGetVPDataReader());
 
   fclose(OutputFile);
static void truncateCurrentFile(void) {
   fclose(File);
 }
 
 static void resetFilenameToDefault(void) {
   memset(&lprofCurFilename, 0, sizeof(lprofCurFilename));
   lprofCurFilename.FilenamePat = "default.profraw";
 }
 
-/* Parses the pattern string \p FilenamePat and store the result to
- * lprofcurFilename structure. */
+static int containsMergeSpecifier(const char *FilenamePat, int I) {
+  return (FilenamePat[I] == 'm' ||
+          (FilenamePat[I] >= '1' && FilenamePat[I] <= '9' &&
+           FilenamePat[I + 1] == 'm'));
+}
 
+/* Parses the pattern string \p FilenamePat and stores the result to
+ * lprofcurFilename structure. */
 static int parseFilenamePattern(const char *FilenamePat) {
   int NumPids = 0, NumHosts = 0, I;
   char *PidChars = &lprofCurFilename.PidChars[0];
   char *Hostname = &lprofCurFilename.Hostname[0];
+  int MergingEnabled = 0;
 
   lprofCurFilename.FilenamePat = FilenamePat;
   /* Check the filename for "%p", which indicates a pid-substitution. */
   for (I = 0; FilenamePat[I]; ++I)
     if (FilenamePat[I] == '%') {
       if (FilenamePat[++I] == 'p') {
         if (!NumPids++) {
           if (snprintf(PidChars, MAX_PID_SIZE, "%d", getpid()) <= 0) {
             PROF_WARN(
                 "Unable to parse filename pattern %s. Using the default name.",
                 FilenamePat);
             return -1;
           }
         }
       } else if (FilenamePat[I] == 'h') {
         if (!NumHosts++)
           if (COMPILER_RT_GETHOSTNAME(Hostname, COMPILER_RT_MAX_HOSTLEN)) {
             PROF_WARN(
                 "Unable to parse filename pattern %s. Using the default name.",
                 FilenamePat);
             return -1;
           }
+      } else if (containsMergeSpecifier(FilenamePat, I)) {
+        if (MergingEnabled) {
+          PROF_WARN(
+              "%%m specifier can only be specified once at the end of %s.\n",
+              FilenamePat);
+          return -1;
+        }
+        MergingEnabled = 1;
+        if (FilenamePat[I] == 'm')
+          lprofCurFilename.MergePoolSize = 1;
+        else {
+          lprofCurFilename.MergePoolSize = FilenamePat[I] - '0';
+          I++; /* advance to 'm' */
+        }
       }
     }
 
   lprofCurFilename.NumPids = NumPids;
   lprofCurFilename.NumHosts = NumHosts;
   return 0;
 }
 
 static void parseAndSetFilename(const char *FilenamePat) {
   int NewFile;
   const char *OldFilenamePat = lprofCurFilename.FilenamePat;
 
   if (!FilenamePat || parseFilenamePattern(FilenamePat))
     resetFilenameToDefault();
 
   NewFile =
       !OldFilenamePat || (strcmp(OldFilenamePat, lprofCurFilename.FilenamePat));
 
-  if (NewFile)
+  if (NewFile && !lprofCurFilename.MergePoolSize)
     truncateCurrentFile();
 }
 
 /* Return buffer length that is required to store the current profile
  * filename with PID and hostname substitutions. */
+/* The length to hold uint64_t followed ty 2 digit pool id including '_' */
+#define SIGLEN 24
 static int getCurFilenameLength() {
+  int Len;
   if (!lprofCurFilename.FilenamePat || !lprofCurFilename.FilenamePat[0])
     return 0;
 
-  if (!(lprofCurFilename.NumPids || lprofCurFilename.NumHosts))
+  if (!(lprofCurFilename.NumPids || lprofCurFilename.NumHosts ||
+        lprofCurFilename.MergePoolSize))
     return strlen(lprofCurFilename.FilenamePat);
 
-  return strlen(lprofCurFilename.FilenamePat) +
-         lprofCurFilename.NumPids * (strlen(lprofCurFilename.PidChars) - 2) +
-         lprofCurFilename.NumHosts * (strlen(lprofCurFilename.Hostname) - 2);
+  Len = strlen(lprofCurFilename.FilenamePat) +
+  if (lprofCurFilename.MergePoolSize)
+    Len += SIGLEN;
+  return Len;
 }
 
 /* Return the pointer to the current profile file name (after substituting
  * PIDs and Hostnames in filename pattern. \p FilenameBuf is the buffer
  * to store the resulting filename. If no substitution is needed, the
  * current filename pattern string is directly returned. */
 static const char *getCurFilename(char *FilenameBuf) {
   int I, J, PidLength, HostNameLength;
   const char *FilenamePat = lprofCurFilename.FilenamePat;
 
   if (!lprofCurFilename.FilenamePat || !lprofCurFilename.FilenamePat[0])
     return 0;
 
-  if (!(lprofCurFilename.NumPids || lprofCurFilename.NumHosts))
+  if (!(lprofCurFilename.NumPids || lprofCurFilename.NumHosts ||
+        lprofCurFilename.MergePoolSize))
     return lprofCurFilename.FilenamePat;
 
   PidLength = strlen(lprofCurFilename.PidChars);
   HostNameLength = strlen(lprofCurFilename.Hostname);
   /* Construct the new filename. */
   for (I = 0, J = 0; FilenamePat[I]; ++I)
     if (FilenamePat[I] == '%') {
       if (FilenamePat[++I] == 'p') {
         memcpy(FilenameBuf + J, lprofCurFilename.PidChars, PidLength);
         J += PidLength;
       } else if (FilenamePat[I] == 'h') {
         memcpy(FilenameBuf + J, lprofCurFilename.Hostname, HostNameLength);
         J += HostNameLength;
+      } else if (containsMergeSpecifier(FilenamePat, I)) {
+        char LoadModuleSignature[SIGLEN];
+        int S;
+        int ProfilePoolId = getpid() % lprofCurFilename.MergePoolSize;
+        S = snprintf(LoadModuleSignature, SIGLEN, "%" PRIu64 "_%d",
+                     lprofGetLoadModuleSignature(), ProfilePoolId);
+        if (S == -1 || S > SIGLEN)
+          S = SIGLEN;
+        memcpy(FilenameBuf + J, LoadModuleSignature, S);
+        J += S;
+        if (FilenamePat[I] != 'm')
+          I++;
       }
       /* Drop any unknown substitutions. */
     } else
       FilenameBuf[J++] = FilenamePat[I];
   FilenameBuf[J] = 0;
 
   return FilenameBuf;
 }

lib/profile/InstrProfilingInternal.h

 VPDataReaderType *lprofGetVPDataReader();
 
 /* Internal interface used by test to reset the max number of 
  * tracked values per value site to be \p MaxVals.
  */
 void lprofSetMaxValsPerSite(uint32_t MaxVals);
 void lprofSetupValueProfiler();
 
+/* Return the profile header 'signature' value associated with the current
+ * executation or shared library. The signature value can be used to for
+ * a profile name that is unique to this load module so that it does not
+ * collide with profiles from other binaries. It also allows shared libraries
+ * to dump merged profile data into its own profile file. */
+uint64_t lprofGetLoadModuleSignature();
+
 COMPILER_RT_VISIBILITY extern char *(*GetEnvHook)(const char *);
 COMPILER_RT_VISIBILITY extern void (*FreeHook)(void *);
 COMPILER_RT_VISIBILITY extern uint8_t *DynamicBufferIOBuffer;
 COMPILER_RT_VISIBILITY extern uint32_t VPBufferSize;
 COMPILER_RT_VISIBILITY extern uint32_t VPMaxNumValsPerSite;
 /* Pointer to the start of static value counters to be allocted. */
 COMPILER_RT_VISIBILITY extern ValueProfNode *CurrentVNode;
 COMPILER_RT_VISIBILITY extern ValueProfNode *EndVNode;
 extern void (*VPMergeHook)(struct ValueProfData *, __llvm_profile_data *);
 
 #endif

lib/profile/InstrProfilingMerge.c

 #include "InstrProfilingInternal.h"
 #include "InstrProfilingUtil.h"
 
 #define INSTR_PROF_VALUE_PROF_DATA
 #include "InstrProfData.inc"
 
 COMPILER_RT_WEAK void (*VPMergeHook)(ValueProfData *,
                                      __llvm_profile_data *) = NULL;
+COMPILER_RT_VISIBILITY
+uint64_t lprofGetLoadModuleSignature() {
+  /* A very fast way to compute the module signature.  */
+  uint64_t CounterSize = (uint64_t)(__llvm_profile_end_counters() -
+                                    __llvm_profile_begin_counters());
+  uint64_t DataSize = __llvm_profile_get_data_size(__llvm_profile_begin_data(),
+                                                   __llvm_profile_end_data());
+  uint64_t NamesSize =
+      (uint64_t)(__llvm_profile_end_names() - __llvm_profile_begin_names());
+  uint64_t NumVnodes =
+      (uint64_t)(__llvm_profile_end_vnodes() - __llvm_profile_begin_vnodes());
+  const __llvm_profile_data *FirstD = __llvm_profile_begin_data();
+
+  return (NamesSize << 40) + (CounterSize << 30) + (DataSize << 20) +
+         (NumVnodes << 10) + (DataSize > 0 ? FirstD->NameRef : 0);
+}
 
 /* Returns 1 if profile is not structurally compatible.  */
 COMPILER_RT_VISIBILITY
 int __llvm_profile_check_compatibility(const char *ProfileData,
                                        uint64_t ProfileSize) {
   /* Check profile header only for now  */
   __llvm_profile_header *Header = (__llvm_profile_header *)ProfileData;
   __llvm_profile_data *SrcDataStart, *SrcDataEnd, *SrcData, *DstData;
   SrcDataStart =
       (__llvm_profile_data *)(ProfileData + sizeof(__llvm_profile_header));
   SrcDataEnd = SrcDataStart + Header->DataSize;
 
+  if (ProfileSize < sizeof(__llvm_profile_header))
+    return 1;
+
   /* Check the header first.  */
   if (Header->Magic != __llvm_profile_get_magic() ||
       Header->Version != __llvm_profile_get_version() ||
       Header->DataSize !=
           __llvm_profile_get_data_size(__llvm_profile_begin_data(),
                                        __llvm_profile_end_data()) ||
       Header->CountersSize != (uint64_t)(__llvm_profile_end_counters() -
                                          __llvm_profile_begin_counters()) ||

lib/profile/InstrProfilingPort.h

 /*===- InstrProfilingPort.h- Support library for PGO instrumentation ------===*\
 |*
 |*                     The LLVM Compiler Infrastructure
 |*
 |* This file is distributed under the University of Illinois Open Source
 |* License. See LICENSE.TXT for details.
 |*
 \*===----------------------------------------------------------------------===*/
 
 #ifndef PROFILE_INSTRPROFILING_PORT_H_
 #define PROFILE_INSTRPROFILING_PORT_H_
 
 #ifdef _MSC_VER
 #define COMPILER_RT_ALIGNAS(x) __declspec(align(x))
 #define COMPILER_RT_VISIBILITY
 #define COMPILER_RT_WEAK __declspec(selectany)
+/* Need to include <windows.h> */
 #define COMPILER_RT_ALLOCA _alloca
+/* Need to include <stdio.h> and <io.h> */
+#define COMPILER_RT_FTRUNCATE(f,l) _chsize(_fileno(f),l)
 #elif __GNUC__
 #define COMPILER_RT_ALIGNAS(x) __attribute__((aligned(x)))
 #define COMPILER_RT_VISIBILITY __attribute__((visibility("hidden")))
 #define COMPILER_RT_WEAK __attribute__((weak))
 #define COMPILER_RT_ALLOCA __builtin_alloca
+#define COMPILER_RT_FTRUNCATE(f,l) ftruncate(fileno(f),l)
 #endif
 
 #if defined(__APPLE__)
 #define COMPILER_RT_SEG "__DATA,"
 #else
 #define COMPILER_RT_SEG ""
 #endif
 

test/profile/instrprof-basic.c

 // RUN: %clang_profgen -o %t -O3 %s
 // RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
 // RUN: llvm-profdata merge -o %t.profdata %t.profraw
-// RUN: %clang_profuse=%t.profdata -o - -S -emit-llvm %s | FileCheck %s
+// RUN: %clang_profuse=%t.profdata -o - -S -emit-llvm %s | FileCheck %s --check-prefix=COMMON --check-prefix=ORIG
+//
+// RUN: rm -f %t.profraw_e_*
+// RUN: env LLVM_PROFILE_FILE=%t.profraw_e_%1m %run %t
+// RUN: env LLVM_PROFILE_FILE=%t.profraw_e_%1m %run %t
+// RUN: llvm-profdata merge -o %t.em.profdata %t.profraw_e_*
+// RUN: %clang_profuse=%t.em.profdata -o - -S -emit-llvm %s | FileCheck %s --check-prefix=COMMON --check-prefix=MERGE
+//
+// RUN: %clang -o %t.merge -fprofile-instr-generate=%t.%m.profraw -O3 %s
+// RUN: rm -f %t.*.profraw*
+// RUN: %run %t.merge
+// RUN: %run %t.merge
+// RUN: llvm-profdata merge -o %t.m.profdata %t.*.profraw
+// RUN: %clang_profuse=%t.m.profdata -o - -S -emit-llvm %s | FileCheck %s --check-prefix=COMMON --check-prefix=MERGE
 
 int begin(int i) {
-  // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD1:[0-9]+]]
+  // COMMON: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD1:[0-9]+]]
   if (i)
     return 0;
   return 1;
 }
 
 int end(int i) {
-  // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD2:[0-9]+]]
+  // COMMON: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD2:[0-9]+]]
   if (i)
     return 0;
   return 1;
 }
 
 int main(int argc, const char *argv[]) {
   begin(0);
   end(1);
 
-  // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD2:[0-9]+]]
+  // COMMON: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD2:[0-9]+]]
   if (argc)
     return 0;
   return 1;
 }
 
-// CHECK: ![[PD1]] = !{!"branch_weights", i32 1, i32 2}
-// CHECK: ![[PD2]] = !{!"branch_weights", i32 2, i32 1}
+// ORIG: ![[PD1]] = !{!"branch_weights", i32 1, i32 2}
+// ORIG: ![[PD2]] = !{!"branch_weights", i32 2, i32 1}
+// MERGE: ![[PD1]] = !{!"branch_weights", i32 1, i32 3}
+// MERGE: ![[PD2]] = !{!"branch_weights", i32 3, i32 1}