[compiler-rt] 98dec28 - Reland [Profile] Remove duplicate file locks when enabled continuous mode and online merging.
Zequan Wu via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 4 11:11:43 PDT 2023
Author: Zequan Wu
Date: 2023-08-04T14:11:22-04:00
New Revision: 98dec28458172091893e46040d7b5ab745a44b82
URL: https://github.com/llvm/llvm-project/commit/98dec28458172091893e46040d7b5ab745a44b82
DIFF: https://github.com/llvm/llvm-project/commit/98dec28458172091893e46040d7b5ab745a44b82.diff
LOG: Reland [Profile] Remove duplicate file locks when enabled continuous mode and online merging.
This reverts commit 4b08be77c98a4c15b8b3d634fab4ffc24bf4ef38.
Add file lock when doing online merging only and associated test.
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
compiler-rt/test/profile/ContinuousSyncMode/set-file-object.c
Removed:
################################################################################
diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index 2bd6a49ce06544..fa71c3fc8f717a 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -424,13 +424,15 @@ 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) {
+ // initializeProfileForContinuousMode will lock the profile, but if
+ // ProfileFile is set by user via __llvm_profile_set_file_object, it's assumed
+ // unlocked at this point.
+ if (ProfileFile && !__llvm_profile_is_continuous_mode_enabled()) {
lprofLockFileHandle(ProfileFile);
- } else {
+ }
+ if (!ProfileFile) {
createProfileDir(ProfileFileName);
ProfileFile = lprofOpenFileEx(ProfileFileName);
}
@@ -481,10 +483,10 @@ static int writeFile(const char *OutputName) {
if (OutputFile == getProfileFile()) {
fflush(OutputFile);
- if (doMerging()) {
+ } else {
+ if (doMerging() && !__llvm_profile_is_continuous_mode_enabled()) {
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..0f2d484787706d
--- /dev/null
+++ b/compiler-rt/test/profile/ContinuousSyncMode/online-merging-windows.c
@@ -0,0 +1,156 @@
+// REQUIRES: target={{.*windows-msvc.*}}
+
+// 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;
+}
\ No newline at end of file
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
diff --git a/compiler-rt/test/profile/ContinuousSyncMode/set-file-object.c b/compiler-rt/test/profile/ContinuousSyncMode/set-file-object.c
index 1f8ea474e976c1..b52324d7091eb2 100644
--- a/compiler-rt/test/profile/ContinuousSyncMode/set-file-object.c
+++ b/compiler-rt/test/profile/ContinuousSyncMode/set-file-object.c
@@ -18,6 +18,13 @@
// RUN: llvm-profdata show --counts --all-functions %t.dir/profdir/profdata | FileCheck %s -check-prefix=MERGE
// RUN: llvm-profdata show --counts --all-functions %t.dir/profdir/*profraw.old | FileCheck %s -check-prefix=ZERO
+// Test __llvm_profile_set_file_object with mergin enabled and continuous mode disabled.
+// RUN: rm -rf %t.dir/profdir/
+// RUN: env LLVM_PROFILE_FILE="%t.dir/profdir/%mprofraw.old" %run %t.dir/main.exe merge %t.dir/profdir/profraw.new 'LLVM_PROFILE_FILE=%t.dir/profdir/%m.profraw'
+// RUN: llvm-profdata merge -o %t.dir/profdir/profdata %t.dir/profdir/profraw.new
+// RUN: llvm-profdata show --counts --all-functions %t.dir/profdir/profdata | FileCheck %s -check-prefix=MERGE
+// RUN: llvm-profdata show --counts --all-functions %t.dir/profdir/*profraw.old | FileCheck %s -check-prefix=ZERO
+
// MERGE: Counters:
// MERGE: coverage_test:
// MERGE: Hash: {{.*}}
@@ -52,15 +59,19 @@ int coverage_test() {
int main(int argc, char **argv) {
char *file_name = argv[2];
- FILE *file = fopen(file_name, "a+b");
- if (strcmp(argv[1], "nomerge") == 0)
+ FILE *file = NULL;
+ if (strcmp(argv[1], "nomerge") == 0) {
+ file = fopen(file_name, "a+b");
__llvm_profile_set_file_object(file, 0);
+ }
else if (strcmp(argv[1], "merge") == 0) {
// Parent process.
int I;
pid_t child_pids[num_child_procs_to_spawn];
char *const child_argv[] = {argv[0], "set", file_name, NULL};
char *const child_envp[] = {argv[3], NULL};
+ FILE *file = fopen(file_name, "w+");
+ fclose(file);
for (I = 0; I < num_child_procs_to_spawn; ++I) {
int ret =
posix_spawn(&child_pids[I], argv[0], NULL, NULL, child_argv, child_envp);
@@ -90,10 +101,7 @@ int main(int argc, char **argv) {
}
} else if (strcmp(argv[1], "set") == 0) {
// Child processes.
- if (!__llvm_profile_is_continuous_mode_enabled()) {
- fprintf(stderr, "Continuous mode disabled\n");
- return 1;
- }
+ file = fopen(file_name, "r+b");
if (__llvm_profile_set_file_object(file, 1)) {
fprintf(stderr, "Call to __llvm_profile_set_file_object failed\n");
return 1;
More information about the llvm-commits
mailing list