[compiler-rt] 985486d - [Profile] Remove duplicate file locks when enabled continuous mode and online merging.
Zequan Wu via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 10 08:01:33 PDT 2023
Author: Zequan Wu
Date: 2023-07-10T11:01:28-04:00
New Revision: 985486dca48bffd9e991d9f5ac32e1d109ae000f
URL: https://github.com/llvm/llvm-project/commit/985486dca48bffd9e991d9f5ac32e1d109ae000f
DIFF: https://github.com/llvm/llvm-project/commit/985486dca48bffd9e991d9f5ac32e1d109ae000f.diff
LOG: [Profile] Remove duplicate file locks when enabled continuous mode and online merging.
In `initializeProfileForContinuousMode`, we have already locked the profile file when merging is enabled, so there's no need to lock the same file second time in `openFileForMerging`.
On Linux/Darwin, the locking the same file twice doesn't cause any problem. But on Windows, it causes the problem to hang forever.
With this minor fix, continuous mode seems working with online merging on Windows.
Reviewed By: phosek
Differential Revision: https://reviews.llvm.org/D154748
Added:
compiler-rt/test/profile/ContinuousSyncMode/online-merging-windows.c
Modified:
compiler-rt/lib/profile/InstrProfilingFile.c
compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c
Removed:
################################################################################
diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index f74be55bcd5ea4..5666607ff457b0 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -424,13 +424,10 @@ static void createProfileDir(const char *Filename) {
* its instrumented shared libraries dump profile data into their own data file.
*/
static FILE *openFileForMerging(const char *ProfileFileName, int *MergeDone) {
- FILE *ProfileFile = NULL;
+ FILE *ProfileFile = getProfileFile();
int rc;
- ProfileFile = getProfileFile();
- if (ProfileFile) {
- lprofLockFileHandle(ProfileFile);
- } else {
+ if (!ProfileFile) {
createProfileDir(ProfileFileName);
ProfileFile = lprofOpenFileEx(ProfileFileName);
}
@@ -481,9 +478,6 @@ static int writeFile(const char *OutputName) {
if (OutputFile == getProfileFile()) {
fflush(OutputFile);
- if (doMerging()) {
- lprofUnlockFileHandle(OutputFile);
- }
} else {
fclose(OutputFile);
}
diff --git a/compiler-rt/test/profile/ContinuousSyncMode/online-merging-windows.c b/compiler-rt/test/profile/ContinuousSyncMode/online-merging-windows.c
new file mode 100644
index 00000000000000..542663fcd20931
--- /dev/null
+++ b/compiler-rt/test/profile/ContinuousSyncMode/online-merging-windows.c
@@ -0,0 +1,156 @@
+// REQUIRES: windows
+
+// Test the online merging mode (%m) along with continuous mode (%c).
+//
+// Split files & cd into a temporary directory.
+// RUN: rm -rf %t.dir && split-file %s %t.dir && cd %t.dir
+//
+// Create two DLLs and a driver program that uses them.
+// RUN: %clang_pgogen foo.c -mllvm -instrprof-atomic-counter-update-all=1 -mllvm -runtime-counter-relocation=true -fuse-ld=lld -Wl,-dll -o %t.dir/foo.dll
+// RUN: %clang_pgogen bar.c -mllvm -instrprof-atomic-counter-update-all=1 -mllvm -runtime-counter-relocation=true -fuse-ld=lld -Wl,-dll -o %t.dir/bar.dll
+// RUN: %clang_pgogen main.c -o main.exe %t.dir/foo.lib %t.dir/bar.lib -mllvm -instrprof-atomic-counter-update-all=1 -mllvm -runtime-counter-relocation=true -fuse-ld=lld
+//
+// === Round 1 ===
+// Test merging+continuous mode without any file contention.
+//
+// RUN: env LLVM_PROFILE_FILE="%t.dir/profdir/%m%c.profraw" %run %t.dir/main.exe nospawn
+// RUN: llvm-profdata merge -o %t.profdata %t.dir/profdir
+// RUN: llvm-profdata show --counts --all-functions %t.profdata | FileCheck %s -check-prefix=ROUND1
+
+// ROUND1-LABEL: Counters:
+// ROUND1-DAG: foo:
+// ROUND1-DAG: Hash: 0x{{.*}}
+// ROUND1-DAG: Counters: 1
+// ROUND1-DAG: Block counts: [1]
+// ROUND1-DAG: bar:
+// ROUND1-DAG: Hash: 0x{{.*}}
+// ROUND1-DAG: Counters: 1
+// ROUND1-DAG: Block counts: [1]
+// ROUND1-DAG: main:
+// ROUND1-DAG: Hash: 0x{{.*}}
+// ROUND1-LABEL: Instrumentation level: IR
+//
+// === Round 2 ===
+// Test merging+continuous mode with some file contention.
+//
+// RUN: env LLVM_PROFILE_FILE="%t.dir/profdir/%m%c.profraw" %run %t.dir/main.exe spawn
+// RUN: llvm-profdata merge -o %t.profdata %t.dir/profdir
+// RUN: llvm-profdata show --counts --all-functions %t.profdata | FileCheck %s -check-prefix=ROUND2
+
+// ROUND2-LABEL: Counters:
+// ROUND2-DAG: foo:
+// ROUND2-DAG: Hash: 0x{{.*}}
+// ROUND2-DAG: Counters: 1
+// ROUND2-DAG: Block counts: [97]
+// ROUND2-DAG: bar:
+// ROUND2-DAG: Hash: 0x{{.*}}
+// ROUND2-DAG: Counters: 1
+// ROUND2-DAG: Block counts: [97]
+// ROUND2-DAG: main:
+// ROUND2-DAG: Hash: 0x{{.*}}
+// ROUND2-LABEL: Instrumentation level: IR
+
+//--- foo.c
+__declspec(dllexport) void foo(void) {}
+
+//--- bar.c
+__declspec(dllexport) void bar(void) {}
+
+//--- main.c
+#include <stdio.h>
+#include <string.h>
+#include <windows.h>
+
+
+const int num_child_procs_to_spawn = 32;
+
+extern int __llvm_profile_is_continuous_mode_enabled(void);
+extern char *__llvm_profile_get_filename(void);
+
+__declspec(dllimport) void foo(void);
+__declspec(dllimport) void bar(void);
+
+// Change to "#define" for debug output.
+#undef DEBUG_TEST
+
+#ifdef DEBUG_TEST
+# define DEBUG(...) fprintf(stderr, __VA_ARGS__);
+#else
+# define DEBUG(...)
+#endif
+
+int main(int argc, char *const argv[]) {
+ if (argc < 2) {
+ DEBUG("Requires at least one argument.\n");
+ return 1;
+ }
+ if (strcmp(argv[1], "nospawn") == 0) {
+ DEBUG(
+ "Hello from child (pid = %lu, cont-mode-enabled = %d, profile = %s).\n",
+ GetCurrentProcessId(), __llvm_profile_is_continuous_mode_enabled(),
+ __llvm_profile_get_filename());
+
+ foo();
+ bar();
+ return 0;
+ } else if (strcmp(argv[1], "spawn") == 0) {
+ // This is the start of Round 2.
+ // Expect Counts[dsoX] = 1, as this was the state at the end of Round 1.
+ int I;
+ HANDLE child_pids[num_child_procs_to_spawn];
+ for (I = 0; I < num_child_procs_to_spawn; ++I) {
+ foo(); // Counts[dsoX] += 2 * num_child_procs_to_spawn
+ bar();
+
+ DEBUG("Spawning child with argv = {%s, %s, NULL} and envp = {%s, NULL}\n",
+ child_argv[0], child_argv[1], child_envp[0]);
+
+ // Start the child process.
+ STARTUPINFO si;
+ ZeroMemory(&si, sizeof(si));
+ PROCESS_INFORMATION pi;
+ ZeroMemory(&pi, sizeof(pi));
+ if (!CreateProcess(NULL, // No module name (use command line)
+ "main.exe nospawn", // Command line
+ NULL, // Process handle not inheritable
+ NULL, // Thread handle not inheritable
+ FALSE, // Set handle inheritance to FALSE
+ 0, // No creation flags
+ NULL, // Use parent's environment block
+ NULL, // Use parent's starting directory
+ &si, // Pointer to STARTUPINFO structure
+ &pi) // Pointer to PROCESS_INFORMATION structure
+ ) {
+ fprintf(stderr, "Child %d could not be spawned: %lu\n", I,
+ GetLastError());
+ return 1;
+ }
+ child_pids[I] = pi.hProcess;
+
+ DEBUG("Spawned child %d (pid = %zu).\n", I, pi.dwProcessId);
+ }
+ for (I = 0; I < num_child_procs_to_spawn; ++I) {
+ foo(); // Counts[dsoX] += num_child_procs_to_spawn
+ bar();
+
+ DWORD exit_code;
+ WaitForSingleObject(child_pids[I], INFINITE);
+ if (!GetExitCodeProcess(child_pids[I], &exit_code)) {
+ fprintf(stderr, "Failed to get exit code of child %d.\n", I);
+ return 1;
+ }
+ if (exit_code != 0) {
+ fprintf(stderr, "Child %d did not exit with code 0.\n", I);
+ return 1;
+ }
+ }
+
+ // At the end of Round 2, we have:
+ // Counts[dsoX] = 1 + (2 * num_child_procs_to_spawn) + num_child_procs_to_spawn
+ // = 97
+
+ return 0;
+ }
+
+ return 1;
+}
diff --git a/compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c b/compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c
index 5e434410da1402..4ca8bf62455371 100644
--- a/compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c
+++ b/compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c
@@ -1,4 +1,4 @@
-// REQUIRES: linux
+// REQUIRES: linux || windows
// RUN: %clang -fprofile-instr-generate -fcoverage-mapping -mllvm -runtime-counter-relocation=true -o %t.exe %s
// RUN: echo "garbage" > %t.profraw
More information about the llvm-commits
mailing list