[compiler-rt] c5b94ea - [profile] Support merge pool size >= 10

[Fangrui Song via llvm-commits]

Author: Fangrui Song
Date: 2020-01-12T00:27:18-08:00
New Revision: c5b94ea265133a4a28006929643155fc8fbeafe6

URL: https://github.com/llvm/llvm-project/commit/c5b94ea265133a4a28006929643155fc8fbeafe6
DIFF: https://github.com/llvm/llvm-project/commit/c5b94ea265133a4a28006929643155fc8fbeafe6.diff

LOG: [profile] Support merge pool size >= 10

The executable acquires an advisory record lock (`fcntl(fd, F_SETLKW, *)`) on a profile file.
Merge pool size >= 10 may be beneficial when the concurrency is large.

Also fix a small problem about snprintf. It can cause the filename to be truncated after %m.

Reviewed By: davidxl

Differential Revision: https://reviews.llvm.org/D71970

Added: 
    

Modified: 
    compiler-rt/lib/profile/InstrProfilingFile.c
    compiler-rt/test/profile/instrprof-basic.c

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index 3abf7a796196..7f3727eed92d 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -588,12 +588,22 @@ static void resetFilenameToDefault(void) {
   lprofCurFilename.PNS = PNS_default;
 }
 
-static int containsMergeSpecifier(const char *FilenamePat, int I) {
-  return (FilenamePat[I] == 'm' ||
-          (FilenamePat[I] >= '1' && FilenamePat[I] <= '9' &&
-           /* If FilenamePat[I] is not '\0', the next byte is guaranteed
-            * to be in-bound as the string is null terminated. */
-           FilenamePat[I + 1] == 'm'));
+static unsigned getMergePoolSize(const char *FilenamePat, int *I) {
+  unsigned J = 0, Num = 0;
+  for (;; ++J) {
+    char C = FilenamePat[*I + J];
+    if (C == 'm') {
+      *I += J;
+      return Num ? Num : 1;
+    }
+    if (C < '0' || C > '9')
+      break;
+    Num = Num * 10 + C - '0';
+
+    /* If FilenamePat[*I+J] is between '0' and '9', the next byte is guaranteed
+     * to be in-bound as the string is null terminated. */
+  }
+  return 0;
 }
 
 /* Parses the pattern string \p FilenamePat and stores the result to
@@ -650,19 +660,17 @@ static int parseFilenamePattern(const char *FilenamePat,
 
         __llvm_profile_enable_continuous_mode();
         I++; /* advance to 'c' */
-      } else if (containsMergeSpecifier(FilenamePat, I)) {
+      } else {
+        unsigned MergePoolSize = getMergePoolSize(FilenamePat, &I);
+        if (!MergePoolSize)
+          continue;
         if (MergingEnabled) {
           PROF_WARN("%%m specifier can only be specified once in %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.MergePoolSize = MergePoolSize;
       }
     }
 
@@ -712,7 +720,7 @@ static void parseAndSetFilename(const char *FilenamePat,
 
 /* Return buffer length that is required to store the current profile
  * filename with PID and hostname substitutions. */
-/* The length to hold uint64_t followed by 2 digit pool id including '_' */
+/* The length to hold uint64_t followed by 3 digits pool id including '_' */
 #define SIGLEN 24
 static int getCurFilenameLength() {
   int Len;
@@ -766,18 +774,18 @@ static const char *getCurFilename(char *FilenameBuf, int ForceUseBuf) {
       } else if (FilenamePat[I] == 'h') {
         memcpy(FilenameBuf + J, lprofCurFilename.Hostname, HostNameLength);
         J += HostNameLength;
-      } else if (containsMergeSpecifier(FilenamePat, I)) {
-        char LoadModuleSignature[SIGLEN];
+      } else {
+        if (!getMergePoolSize(FilenamePat, &I))
+          continue;
+        char LoadModuleSignature[SIGLEN + 1];
         int S;
         int ProfilePoolId = getpid() % lprofCurFilename.MergePoolSize;
-        S = snprintf(LoadModuleSignature, SIGLEN, "%" PRIu64 "_%d",
+        S = snprintf(LoadModuleSignature, SIGLEN + 1, "%" 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

diff  --git a/compiler-rt/test/profile/instrprof-basic.c b/compiler-rt/test/profile/instrprof-basic.c
index dd8f3fca3f9c..17631d1b9a98 100644
--- a/compiler-rt/test/profile/instrprof-basic.c
+++ b/compiler-rt/test/profile/instrprof-basic.c
@@ -42,6 +42,13 @@
 // RUN: llvm-profdata merge -o %t.m4.profdata ./
 // RUN: %clang_profuse=%t.m4.profdata -O0 -o - -S -emit-llvm %s | FileCheck %s --check-prefix=COMMON  --check-prefix=PGOMERGE
 
+/// Test that the merge pool size can be larger than 10.
+// RUN: rm -fr %t.dir5
+// RUN: mkdir -p %t.dir5
+// RUN: env LLVM_PROFILE_FILE=%t.dir5/e_%20m.profraw %run %t
+// RUN: not ls %t.dir5/e_%20m.profraw
+// RUN: ls %t.dir5/e_*.profraw | count 1
+
 int begin(int i) {
   // COMMON: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}, !prof ![[PD1:[0-9]+]]
   if (i)