[PATCH] D68351: [profile] Add a mode to continuously sync counter updates to a file

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In D68351#1691883 <https://reviews.llvm.org/D68351#1691883>, @sajjadm wrote:

> Not sure it belongs in this CL specifically, but would it be possible to later add a mode that defers the initialization of the mmap'd file until specifically requested by the process?


This sounds tricky to me, as the initialization requires some work to be done in each copy of the profile runtime linked into the process.

Could you clarify whether you're referring to processes which use the profiling runtime's static initializer, or not? My understanding is that for processes which //do// use the runtime's static initializer, it's always desirable to set up the mmap'd file in that initializer. If that's not true, I'd appreciate a counterexample. For processes that //don't// use the runtime's static initializer, the situation (at Apple, at least) is typically that it's a kernel extension or bare-metal piece of firmware, and the continuous mode isn't really applicable anyway.



================
Comment at: compiler-rt/lib/profile/InstrProfiling.h:179
+ * program initialization time. If the filename is changed in one image, there
+ * is no way to remap the counter-to-file mappings in every other image.
  */
----------------
sajjadm wrote:
> I'm afraid I don't follow, could you explain in more detail why changing the filename is a problem?
The problem is that there are N copies of `__llvm_profile_set_filename` in a process, one for each instrumented image (1 executable + (N-1) DSOs). When one copy of `__llvm_profile_set_filename` is called, it can change the filename visible to all images because of weak symbol coalescing. However, in continuous mode, that is not sufficient. What's needed is to remap the `__llvm_prf_cnts -> FILE` mappings in each image. And a call to `__llvm_profile_set_filename` cannot achieve that. At least, not unless each image publishes its section mappings -- imho that level of complexity would be excessive.


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

https://reviews.llvm.org/D68351