[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#1692003 <https://reviews.llvm.org/D68351#1692003>, @sajjadm wrote:

> In D68351#1691969 <https://reviews.llvm.org/D68351#1691969>, @vsk wrote:
>
> > In D68351#1691934 <https://reviews.llvm.org/D68351#1691934>, @sajjadm wrote:
> >
> > > In D68351#1691915 <https://reviews.llvm.org/D68351#1691915>, @vsk wrote:
> > >
> > > > 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.
> > >
> > >
> > > We do use the static initializer, but in our use case the process doesn't necessarily know where to write coverage data until some other application-specific code has run.
> >
> >
> > I see, thanks for explaining.
> >
> > > Ideally, I would like to be able to have the instrumented process receive a shared memory buffer from another process, do continuous profile sync into that memory region, and not care whether it is file-backed or anonymous mapping. So it would be great if application code could give the runtime some sort of handle to that object [1] and tell the runtime to use that space for profiling.
> >
> > I see. Is the lack of support for value profiling a concern, or would this just be for code coverage?
>
>
> This would just be for code coverage.
>
> > Regardless, I think it's possible to achieve this. If `__llvm_profile_set_file_object` is called in every copy of the profile runtime in a process, only minor changes to this patch would be required [1]. Judging by how `setProfileFile` is implemented, I suspect this is the case. If not, a much more invasive change would be needed.
>
> How does one get multiple copies of the runtime in the same process? Does that happen when you load a shared object file that was built with instrumentation?


Yes, exactly.

> I think in our use case all our instrumented code is statically linked together, so that might not be a problem for me.

Oh, that makes sense then. There would just need to be a comment near `__llvm_profile_set_file_object` to explain that it only works on a per-image basis (i.e. that clients with instrumented DSOs should exercise caution with it).

>> [1] You could do this by recognizing a "%dc" filename pattern for "delayed continuous sync" mode (or some equivalent). The static initializer would detect this and defer the mmap() initialization. Later, when `__llvm_profile_set_file_object` is called, continuous mode can be initialized.
>> 
>>> [1] This could be just a pointer to a buffer that the application has already mmap'd, or it could be a file descriptor that the runtime is responsible for calling mmap on. Not sure which one would make more sense.
>> 
>> I'm not sure it's possible to mmap() the contents of a section using MAP_FIXED over another non file-backed VM region. You may need to experiment with this.
> 
> Good point, I'll look into it.
> 
> it seems like this approach can be extended down the road without much effort, so that's great. Thanks for humoring my questions. :)

I think so, no worries!


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

https://reviews.llvm.org/D68351