[compiler-rt] 32476b9 - Revert " [profile] Implement a non-mmap path when reading profile files from a non-local filesystem (#131177)"
Arthur Eubanks via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 19 13:31:49 PDT 2025
Author: Arthur Eubanks
Date: 2025-03-19T20:31:32Z
New Revision: 32476b99344fb0a3b85acbdba666e165181ec638
URL: https://github.com/llvm/llvm-project/commit/32476b99344fb0a3b85acbdba666e165181ec638
DIFF: https://github.com/llvm/llvm-project/commit/32476b99344fb0a3b85acbdba666e165181ec638.diff
LOG: Revert " [profile] Implement a non-mmap path when reading profile files from a non-local filesystem (#131177)"
This reverts commit 14c95e0c8b25f6deba47cd279c5dcdeef3870159.
Test fails on mac, e.g. https://green.lab.llvm.org/job/llvm.org/job/clang-stage1-RA/3899/testReport/junit/Profile-x86_64/Profile-x86_64/instrprof_no_mmap_during_merging_c/
Added:
Modified:
compiler-rt/lib/profile/InstrProfilingFile.c
compiler-rt/lib/profile/InstrProfilingPort.h
compiler-rt/lib/profile/InstrProfilingUtil.c
compiler-rt/lib/profile/InstrProfilingUtil.h
compiler-rt/test/profile/Posix/instrprof-fork.c
Removed:
compiler-rt/test/profile/instrprof-no-mmap-during-merging.c
################################################################################
diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index 4667c02892505..ce5aa9a8fd328 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -419,17 +419,17 @@ static int getProfileFileSizeForMerging(FILE *ProfileFile,
* \p ProfileBuffer. Returns -1 on failure. On success, the caller is
* responsible for unmapping the mmap'd buffer in \p ProfileBuffer. */
static int mmapProfileForMerging(FILE *ProfileFile, uint64_t ProfileFileSize,
- ManagedMemory *ProfileBuffer) {
- lprofGetFileContentBuffer(ProfileFile, ProfileFileSize, ProfileBuffer);
-
- if (ProfileBuffer->Status == MS_INVALID) {
- PROF_ERR("Unable to merge profile data: %s\n", "reading file failed");
+ char **ProfileBuffer) {
+ *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->Addr,
- ProfileFileSize)) {
- (void)lprofReleaseBuffer(ProfileBuffer, ProfileFileSize);
+ 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 -1;
@@ -444,7 +444,7 @@ static int mmapProfileForMerging(FILE *ProfileFile, uint64_t ProfileFileSize,
*/
static int doProfileMerging(FILE *ProfileFile, int *MergeDone) {
uint64_t ProfileFileSize;
- ManagedMemory ProfileBuffer;
+ char *ProfileBuffer;
/* Get the size of the profile on disk. */
if (getProfileFileSizeForMerging(ProfileFile, &ProfileFileSize) == -1)
@@ -460,9 +460,9 @@ static int doProfileMerging(FILE *ProfileFile, int *MergeDone) {
return -1;
/* Now start merging */
- if (__llvm_profile_merge_from_buffer(ProfileBuffer.Addr, ProfileFileSize)) {
+ if (__llvm_profile_merge_from_buffer(ProfileBuffer, ProfileFileSize)) {
PROF_ERR("%s\n", "Invalid profile data to merge");
- (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
+ (void)munmap(ProfileBuffer, ProfileFileSize);
return -1;
}
@@ -471,7 +471,7 @@ static int doProfileMerging(FILE *ProfileFile, int *MergeDone) {
(void)COMPILER_RT_FTRUNCATE(ProfileFile,
__llvm_profile_get_size_for_buffer());
- (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
+ (void)munmap(ProfileBuffer, ProfileFileSize);
*MergeDone = 1;
return 0;
@@ -672,13 +672,13 @@ static void initializeProfileForContinuousMode(void) {
} else {
/* The merged profile has a non-zero length. Check that it is compatible
* with the data in this process. */
- ManagedMemory ProfileBuffer;
+ char *ProfileBuffer;
if (mmapProfileForMerging(File, ProfileFileSize, &ProfileBuffer) == -1) {
lprofUnlockFileHandle(File);
fclose(File);
return;
}
- (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
+ (void)munmap(ProfileBuffer, ProfileFileSize);
}
} else {
File = fopen(Filename, FileOpenMode);
@@ -1257,12 +1257,12 @@ COMPILER_RT_VISIBILITY int __llvm_profile_set_file_object(FILE *File,
} else {
/* The merged profile has a non-zero length. Check that it is compatible
* with the data in this process. */
- ManagedMemory ProfileBuffer;
+ char *ProfileBuffer;
if (mmapProfileForMerging(File, ProfileFileSize, &ProfileBuffer) == -1) {
lprofUnlockFileHandle(File);
return 1;
}
- (void)lprofReleaseBuffer(&ProfileBuffer, ProfileFileSize);
+ (void)munmap(ProfileBuffer, ProfileFileSize);
}
mmapForContinuousMode(0, File);
lprofUnlockFileHandle(File);
diff --git a/compiler-rt/lib/profile/InstrProfilingPort.h b/compiler-rt/lib/profile/InstrProfilingPort.h
index a77fa30731670..66d697885eaee 100644
--- a/compiler-rt/lib/profile/InstrProfilingPort.h
+++ b/compiler-rt/lib/profile/InstrProfilingPort.h
@@ -23,7 +23,6 @@
#define COMPILER_RT_FTRUNCATE(f,l) _chsize(_fileno(f),l)
#define COMPILER_RT_ALWAYS_INLINE __forceinline
#define COMPILER_RT_CLEANUP(x)
-#define COMPILER_RT_UNUSED
#define COMPILER_RT_USED
#elif __GNUC__
#ifdef _WIN32
@@ -39,7 +38,6 @@
#define COMPILER_RT_ALLOCA __builtin_alloca
#define COMPILER_RT_ALWAYS_INLINE inline __attribute((always_inline))
#define COMPILER_RT_CLEANUP(x) __attribute__((cleanup(x)))
-#define COMPILER_RT_UNUSED __attribute__((unused))
#define COMPILER_RT_USED __attribute__((used))
#endif
diff --git a/compiler-rt/lib/profile/InstrProfilingUtil.c b/compiler-rt/lib/profile/InstrProfilingUtil.c
index 0fae91cfb8950..c637b9d0b893c 100644
--- a/compiler-rt/lib/profile/InstrProfilingUtil.c
+++ b/compiler-rt/lib/profile/InstrProfilingUtil.c
@@ -21,15 +21,6 @@
#include <unistd.h>
#endif
-#ifdef _AIX
-#include <sys/statfs.h>
-// <sys/vmount.h> depends on `uint` to be a typedef from <sys/types.h> to
-// `uint_t`; however, <sys/types.h> does not always declare `uint`. We provide
-// the typedef prior to including <sys/vmount.h> to work around this issue.
-typedef uint_t uint;
-#include <sys/vmount.h>
-#endif
-
#ifdef COMPILER_RT_HAS_UNAME
#include <sys/utsname.h>
#endif
@@ -267,121 +258,6 @@ COMPILER_RT_VISIBILITY FILE *lprofOpenFileEx(const char *ProfileName) {
return f;
}
-#if defined(_AIX)
-// Return 1 (true) if the file descriptor Fd represents a file that is on a
-// local filesystem, otherwise return 0.
-static int isLocalFilesystem(int Fd) {
- struct statfs Vfs;
- if (fstatfs(Fd, &Vfs) != 0) {
- PROF_ERR("%s: fstatfs(%d) failed: %s\n", __func__, Fd, strerror(errno));
- return 0;
- }
-
- int Ret;
- size_t BufSize = 2048u;
- char *Buf;
- int Tries = 3;
- while (Tries--) {
- Buf = malloc(BufSize);
- // mntctl returns -1 if `Buf` is `NULL`.
- Ret = mntctl(MCTL_QUERY, BufSize, Buf);
- if (Ret != 0)
- break;
- BufSize = *(unsigned int *)Buf;
- free(Buf);
- }
-
- if (Ret != -1) {
- // Look for the correct vmount entry.
- char *CurObjPtr = Buf;
- while (Ret--) {
- struct vmount *Vp = (struct vmount *)CurObjPtr;
- _Static_assert(sizeof(Vfs.f_fsid) == sizeof(Vp->vmt_fsid),
- "fsid length mismatch");
- if (memcmp(&Vfs.f_fsid, &Vp->vmt_fsid, sizeof Vfs.f_fsid) == 0) {
- int Answer = (Vp->vmt_flags & MNT_REMOTE) == 0;
- free(Buf);
- return Answer;
- }
- CurObjPtr += Vp->vmt_length;
- }
- }
-
- free(Buf);
- // There was an error in mntctl or vmount entry not found; "remote" is the
- // conservative answer.
- return 0;
-}
-#endif
-
-static int isMmapSafe(int Fd) {
- if (getenv("LLVM_PROFILE_NO_MMAP")) // For testing purposes.
- return 0;
-#ifdef _AIX
- return isLocalFilesystem(Fd);
-#else
- return 1;
-#endif
-}
-
-COMPILER_RT_VISIBILITY void lprofGetFileContentBuffer(FILE *F, uint64_t Length,
- ManagedMemory *Buf) {
- Buf->Status = MS_INVALID;
- if (isMmapSafe(fileno(F))) {
- Buf->Addr =
- mmap(NULL, Length, PROT_READ, MAP_SHARED | MAP_FILE, fileno(F), 0);
- if (Buf->Addr == MAP_FAILED)
- PROF_ERR("%s: mmap failed: %s\n", __func__, strerror(errno))
- else
- Buf->Status = MS_MMAP;
- return;
- }
-
- if (getenv("LLVM_PROFILE_VERBOSE"))
- PROF_NOTE("%s\n", "could not use mmap; using fread instead");
-
- void *Buffer = malloc(Length);
- if (!Buffer) {
- PROF_ERR("%s: malloc failed: %s\n", __func__, strerror(errno));
- return;
- }
- if (ftell(F) != 0) {
- PROF_ERR("%s: expecting ftell to return zero\n", __func__);
- free(Buffer);
- return;
- }
-
- // Read the entire file into memory.
- size_t BytesRead = fread(Buffer, 1, Length, F);
- if (BytesRead != (size_t)Length) {
- PROF_ERR("%s: fread failed%s\n", __func__,
- feof(F) ? ": end of file reached" : "");
- free(Buffer);
- return;
- }
-
- // Reading was successful, record the result in the Buf parameter.
- Buf->Addr = Buffer;
- Buf->Status = MS_MALLOC;
-}
-
-COMPILER_RT_VISIBILITY
-void lprofReleaseBuffer(ManagedMemory *Buf, size_t Length) {
- switch (Buf->Status) {
- case MS_MALLOC:
- free(Buf->Addr);
- break;
- case MS_MMAP:
- (void)munmap(Buf->Addr, Length);
- break;
- default:
- PROF_ERR("%s: Buffer has invalid state: %d\n", __func__, Buf->Status);
- break;
- }
- Buf->Addr = NULL;
- Buf->Status = MS_INVALID;
-}
-
COMPILER_RT_VISIBILITY const char *lprofGetPathPrefix(int *PrefixStrip,
size_t *PrefixLen) {
const char *Prefix = getenv("GCOV_PREFIX");
diff --git a/compiler-rt/lib/profile/InstrProfilingUtil.h b/compiler-rt/lib/profile/InstrProfilingUtil.h
index 234cf26cc0af7..227c2aa0a7cae 100644
--- a/compiler-rt/lib/profile/InstrProfilingUtil.h
+++ b/compiler-rt/lib/profile/InstrProfilingUtil.h
@@ -31,25 +31,6 @@ int lprofUnlockFileHandle(FILE *F);
* lock for exclusive access. The caller will block
* if the lock is already held by another process. */
FILE *lprofOpenFileEx(const char *Filename);
-
-enum MemoryStatus {
- MS_INVALID, // Addr is not a valid address
- MS_MMAP, // Addr was mmap'ed
- MS_MALLOC // Addr was malloc'ed
-};
-typedef struct {
- void *Addr;
- enum MemoryStatus Status;
-} ManagedMemory;
-
-/* Read the content of a file using mmap or fread into a buffer.
- * Certain files (e.g. NFS mounted) cannot be opened reliably with mmap,
- * so we use fread in those cases. The corresponding lprofReleaseBuffer
- * will free/munmap the buffer.
- */
-void lprofGetFileContentBuffer(FILE *F, uint64_t FileSize, ManagedMemory *Buf);
-void lprofReleaseBuffer(ManagedMemory *FileBuffer, size_t Length);
-
/* PS4 doesn't have setenv/getenv/fork. Define a shim. */
#if __ORBIS__
#include <sys/types.h>
diff --git a/compiler-rt/test/profile/Posix/instrprof-fork.c b/compiler-rt/test/profile/Posix/instrprof-fork.c
index a1b99b492aa4f..8df0abd73c4c3 100644
--- a/compiler-rt/test/profile/Posix/instrprof-fork.c
+++ b/compiler-rt/test/profile/Posix/instrprof-fork.c
@@ -3,15 +3,11 @@
// RUN: %clang_pgogen=%t.profdir -o %t -O2 %s
// RUN: %run %t
// RUN: llvm-profdata show --all-functions --counts %t.profdir/default_*.profraw | FileCheck %s
-// RUN: rm -fr %t.profdir
-// RUN: env LLVM_PROFILE_NO_MMAP=1 %run %t
-// RUN: llvm-profdata show --all-functions --counts %t.profdir/default_*.profraw | FileCheck %s
-
//
// CHECK: func1:
-// CHECK: Block counts: [21]
+// CHECK: Block counts: [4]
// CHECK: func2:
-// CHECK: Block counts: [10]
+// CHECK: Block counts: [1]
#include <sys/wait.h>
#include <unistd.h>
@@ -20,23 +16,17 @@ __attribute__((noinline)) void func1() {}
__attribute__((noinline)) void func2() {}
int main(void) {
- // child | parent
- // func1 func2 | func1 func2
- func1(); // +10 | +1 (*)
- int i = 10; // |
- while (i-- > 0) { // |
- pid_t pid = fork(); // |
- if (pid == -1) // |
- return 1; // |
- if (pid == 0) { // |
- func2(); // +10 |
- func1(); // +10 |
- return 0; // |
- } // |
- } // ------------+------------
- int status; // 20 10 | 1 0
- i = 10; // (*) the child inherits counter values prior to fork
- while (i-- > 0) // from the parent in non-continuous mode.
- wait(&status);
- return 0;
+ // child | parent
+ int status; // func1 func2 | func1 func2
+ func1(); // +1 | +1 (*)
+ pid_t pid = fork(); // |
+ if (pid == -1) // |
+ return 1; // |
+ if (pid == 0) // |
+ func2(); // +1 |
+ func1(); // +1 | +1
+ if (pid) // ------------+------------
+ wait(&status); // 2 1 | 2 0
+ return 0; // (*) the child inherits counter values prior to fork
+ // from the parent in non-continuous mode.
}
diff --git a/compiler-rt/test/profile/instrprof-no-mmap-during-merging.c b/compiler-rt/test/profile/instrprof-no-mmap-during-merging.c
deleted file mode 100644
index c3f0c786e9fa5..0000000000000
--- a/compiler-rt/test/profile/instrprof-no-mmap-during-merging.c
+++ /dev/null
@@ -1,26 +0,0 @@
-// RUN: mkdir -p %t.d && cd %t.d
-// RUN: rm -f *.profraw
-// RUN: %clang_pgogen %s -o a.out
-
-// Need to run a.out twice. On the second time, a merge will occur, which will
-// trigger an mmap.
-// RUN: ./a.out
-// RUN: llvm-profdata show default_*.profraw --all-functions --counts --memop-sizes 2>&1 | FileCheck %s -check-prefix=PROFDATA
-// RUN: env LLVM_PROFILE_NO_MMAP=1 LLVM_PROFILE_VERBOSE=1 ./a.out 2>&1 | FileCheck %s
-// RUN: llvm-profdata show default_*.profraw --all-functions --counts --memop-sizes 2>&1 | FileCheck %s -check-prefix=PROFDATA2
-
-// CHECK: could not use mmap; using fread instead
-// PROFDATA: Block counts: [1]
-// PROFDATA: [ 0, 0, 1 ]
-// PROFDATA: Maximum function count: 1
-// PROFDATA2: Block counts: [2]
-// PROFDATA2: [ 0, 0, 2 ]
-// PROFDATA2: Maximum function count: 2
-
-#include <string.h>
-int ar[8];
-int main() {
- memcpy(ar, ar + 2, ar[0]);
- memcpy(ar, ar + 2, ar[2]);
- return ar[2];
-}
More information about the llvm-commits
mailing list