[PATCH] D155290: [PGO] Use Unique Profile Files when New Processes are Forked

Petr Hosek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 15 00:16:28 PDT 2023


phosek added a comment.

Can we avoid the use of `pthread_atfork` and the dependency on pthreads? Using pthreads inside the profile runtime implementation is problematic in the case where you want to instrument the libc itself which is used on some platforms and it's one of the reasons why we would like to eliminate the dependency on libc altogether and use `sanitizer_common` instead, see https://discourse.llvm.org/t/using-c-and-sanitizer-common-in-profile-runtime/58629.

In D155290#4585607 <https://reviews.llvm.org/D155290#4585607>, @qiongsiwu1 wrote:

> In D155290#4582825 <https://reviews.llvm.org/D155290#4582825>, @MaskRay wrote:
>
>> Using `%p` should not magically change the behavior and I am unsure the result is better for PGO. 
>> If we decide to provide such a mode, it needs to be opt-in by adding a new format specifier or API.
>> Can you elaborate separate profile files are going to be useful?
>
> Thanks again for taking a look at this patch!
>
> Sure! We are generating separate profiles through `%p` for two reasons. First, we think that `%p` implies a profile per process, even if `exec` is not called after `fork`. It is unexpected that with `%p`, a program that forks child processes only create one profile file from the parent. This is why we did not create a new API or create a new pattern. Second, we are trying to catch non-gracefully terminated processes during profile generate. We observed a scenario where a hot function having all 0 counters. The reasons was the child process was created by a fork (but no `exec` followed), and then was terminated probably by a signal, instead of going through `exit()`. Such a process did not have a chance to write back the profile for merging. The parent process took a different call path and never called the hot function. We would like to detect such a situation for a particular process. Hence this patch creates an empty profile file if a process is not terminated gracefully. Using `%p` is probably the most natural choice when debugging such issues.
>
> One can argue that we may look into the continuous mode to manage abnormal terminations during profile generate. I have not looked too closely at that, but we think it is still useful for a user to tell that some processes did not write any profile back, for debugging, or for diagnostics.

Handling cases like these is exactly why the continous mode was created. Your solution only covers a single case, but there's many scenarios where process can terminate abnormally and fail to invoke `atexit` hooks which would results in profile not being written out.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155290/new/

https://reviews.llvm.org/D155290



More information about the cfe-commits mailing list