Skip to content

Commit 985486d

Browse files
ZequanWuZequanWu
committed
[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
1 parent c9c32fe commit 985486d

File tree

3 files changed

+159
-9
lines changed

3 files changed

+159
-9
lines changed

‎compiler-rt/lib/profile/InstrProfilingFile.c

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -424,13 +424,10 @@ static void createProfileDir(const char *Filename) {
424424
* its instrumented shared libraries dump profile data into their own data file.
425425
*/
426426
static FILE *openFileForMerging(const char *ProfileFileName, int *MergeDone) {
427-
FILE *ProfileFile = NULL;
427+
FILE *ProfileFile = getProfileFile();
428428
int rc;
429429

430-
ProfileFile = getProfileFile();
431-
if (ProfileFile) {
432-
lprofLockFileHandle(ProfileFile);
433-
} else {
430+
if (!ProfileFile) {
434431
createProfileDir(ProfileFileName);
435432
ProfileFile = lprofOpenFileEx(ProfileFileName);
436433
}
@@ -481,9 +478,6 @@ static int writeFile(const char *OutputName) {
481478

482479
if (OutputFile == getProfileFile()) {
483480
fflush(OutputFile);
484-
if (doMerging()) {
485-
lprofUnlockFileHandle(OutputFile);
486-
}
487481
} else {
488482
fclose(OutputFile);
489483
}

‎compiler-rt/test/profile/ContinuousSyncMode/online-merging-windows.c

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
// REQUIRES: windows
2+
3+
// Test the online merging mode (%m) along with continuous mode (%c).
4+
//
5+
// Split files & cd into a temporary directory.
6+
// RUN: rm -rf %t.dir && split-file %s %t.dir && cd %t.dir
7+
//
8+
// Create two DLLs and a driver program that uses them.
9+
// 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
10+
// 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
11+
// 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
12+
//
13+
// === Round 1 ===
14+
// Test merging+continuous mode without any file contention.
15+
//
16+
// RUN: env LLVM_PROFILE_FILE="%t.dir/profdir/%m%c.profraw" %run %t.dir/main.exe nospawn
17+
// RUN: llvm-profdata merge -o %t.profdata %t.dir/profdir
18+
// RUN: llvm-profdata show --counts --all-functions %t.profdata | FileCheck %s -check-prefix=ROUND1
19+
20+
// ROUND1-LABEL: Counters:
21+
// ROUND1-DAG: foo:
22+
// ROUND1-DAG: Hash: 0x{{.*}}
23+
// ROUND1-DAG: Counters: 1
24+
// ROUND1-DAG: Block counts: [1]
25+
// ROUND1-DAG: bar:
26+
// ROUND1-DAG: Hash: 0x{{.*}}
27+
// ROUND1-DAG: Counters: 1
28+
// ROUND1-DAG: Block counts: [1]
29+
// ROUND1-DAG: main:
30+
// ROUND1-DAG: Hash: 0x{{.*}}
31+
// ROUND1-LABEL: Instrumentation level: IR
32+
//
33+
// === Round 2 ===
34+
// Test merging+continuous mode with some file contention.
35+
//
36+
// RUN: env LLVM_PROFILE_FILE="%t.dir/profdir/%m%c.profraw" %run %t.dir/main.exe spawn
37+
// RUN: llvm-profdata merge -o %t.profdata %t.dir/profdir
38+
// RUN: llvm-profdata show --counts --all-functions %t.profdata | FileCheck %s -check-prefix=ROUND2
39+
40+
// ROUND2-LABEL: Counters:
41+
// ROUND2-DAG: foo:
42+
// ROUND2-DAG: Hash: 0x{{.*}}
43+
// ROUND2-DAG: Counters: 1
44+
// ROUND2-DAG: Block counts: [97]
45+
// ROUND2-DAG: bar:
46+
// ROUND2-DAG: Hash: 0x{{.*}}
47+
// ROUND2-DAG: Counters: 1
48+
// ROUND2-DAG: Block counts: [97]
49+
// ROUND2-DAG: main:
50+
// ROUND2-DAG: Hash: 0x{{.*}}
51+
// ROUND2-LABEL: Instrumentation level: IR
52+
53+
//--- foo.c
54+
__declspec(dllexport) void foo(void) {}
55+
56+
//--- bar.c
57+
__declspec(dllexport) void bar(void) {}
58+
59+
//--- main.c
60+
#include <stdio.h>
61+
#include <string.h>
62+
#include <windows.h>
63+
64+
65+
const int num_child_procs_to_spawn = 32;
66+
67+
extern int __llvm_profile_is_continuous_mode_enabled(void);
68+
extern char *__llvm_profile_get_filename(void);
69+
70+
__declspec(dllimport) void foo(void);
71+
__declspec(dllimport) void bar(void);
72+
73+
// Change to "#define" for debug output.
74+
#undef DEBUG_TEST
75+
76+
#ifdef DEBUG_TEST
77+
# define DEBUG(...) fprintf(stderr, __VA_ARGS__);
78+
#else
79+
# define DEBUG(...)
80+
#endif
81+
82+
int main(int argc, char *const argv[]) {
83+
if (argc < 2) {
84+
DEBUG("Requires at least one argument.\n");
85+
return 1;
86+
}
87+
if (strcmp(argv[1], "nospawn") == 0) {
88+
DEBUG(
89+
"Hello from child (pid = %lu, cont-mode-enabled = %d, profile = %s).\n",
90+
GetCurrentProcessId(), __llvm_profile_is_continuous_mode_enabled(),
91+
__llvm_profile_get_filename());
92+
93+
foo();
94+
bar();
95+
return 0;
96+
} else if (strcmp(argv[1], "spawn") == 0) {
97+
// This is the start of Round 2.
98+
// Expect Counts[dsoX] = 1, as this was the state at the end of Round 1.
99+
int I;
100+
HANDLE child_pids[num_child_procs_to_spawn];
101+
for (I = 0; I < num_child_procs_to_spawn; ++I) {
102+
foo(); // Counts[dsoX] += 2 * num_child_procs_to_spawn
103+
bar();
104+
105+
DEBUG("Spawning child with argv = {%s, %s, NULL} and envp = {%s, NULL}\n",
106+
child_argv[0], child_argv[1], child_envp[0]);
107+
108+
// Start the child process.
109+
STARTUPINFO si;
110+
ZeroMemory(&si, sizeof(si));
111+
PROCESS_INFORMATION pi;
112+
ZeroMemory(&pi, sizeof(pi));
113+
if (!CreateProcess(NULL, // No module name (use command line)
114+
"main.exe nospawn", // Command line
115+
NULL, // Process handle not inheritable
116+
NULL, // Thread handle not inheritable
117+
FALSE, // Set handle inheritance to FALSE
118+
0, // No creation flags
119+
NULL, // Use parent's environment block
120+
NULL, // Use parent's starting directory
121+
&si, // Pointer to STARTUPINFO structure
122+
&pi) // Pointer to PROCESS_INFORMATION structure
123+
) {
124+
fprintf(stderr, "Child %d could not be spawned: %lu\n", I,
125+
GetLastError());
126+
return 1;
127+
}
128+
child_pids[I] = pi.hProcess;
129+
130+
DEBUG("Spawned child %d (pid = %zu).\n", I, pi.dwProcessId);
131+
}
132+
for (I = 0; I < num_child_procs_to_spawn; ++I) {
133+
foo(); // Counts[dsoX] += num_child_procs_to_spawn
134+
bar();
135+
136+
DWORD exit_code;
137+
WaitForSingleObject(child_pids[I], INFINITE);
138+
if (!GetExitCodeProcess(child_pids[I], &exit_code)) {
139+
fprintf(stderr, "Failed to get exit code of child %d.\n", I);
140+
return 1;
141+
}
142+
if (exit_code != 0) {
143+
fprintf(stderr, "Child %d did not exit with code 0.\n", I);
144+
return 1;
145+
}
146+
}
147+
148+
// At the end of Round 2, we have:
149+
// Counts[dsoX] = 1 + (2 * num_child_procs_to_spawn) + num_child_procs_to_spawn
150+
// = 97
151+
152+
return 0;
153+
}
154+
155+
return 1;
156+
}

‎compiler-rt/test/profile/ContinuousSyncMode/runtime-counter-relocation.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// REQUIRES: linux
1+
// REQUIRES: linux || windows
22

33
// RUN: %clang -fprofile-instr-generate -fcoverage-mapping -mllvm -runtime-counter-relocation=true -o %t.exe %s
44
// RUN: echo "garbage" > %t.profraw

0 commit comments

Comments
 (0)