[llvm-dev] XRay feature – pid reporting

[Henry Zhu via llvm-dev]

Thanks for all the help.

I am based on the east coast of US, but I don't mind the latency.

I will you message you on LLVM IRC channel if I have any more questions.
Username is zhenry.

On Thu, Jun 28, 2018 at 7:32 PM, Dean Michael Berris <dean.berris at gmail.com>
wrote:

>
>
> > On 29 Jun 2018, at 07:18, Henry Zhu <henryzhu at seas.upenn.edu> wrote:
> >
> > I'm still somewhat unclear about what you mean by "metadata record entry
> at the beginning of the block". I understand that I can make a
> MetadataRecord that contains the pid/tid since a metadata record contains
> 16 bytes. However, I don't understand what do you mean by the "beginning of
> the block". Do you mean right after the file header?
> >
>
> Please see https://llvm.org/docs/XRayFDRFormat.html which documents the
> FDR mode log format.
>
> Note that this is out of date given the changes made recently which adds
> an explicit size record before each new buffer. The code for the FDR mode
> logging implementation shows what the first few metadata records we write
> for each buffer/block are in `writeNewBufferPreamble` (
> https://github.com/llvm-project/llvm-project-20170507/
> blob/master/compiler-rt/lib/xray/xray_fdr_logging.cc#L128) which is where
> we write down the first few metadata records for each new buffer.
>
> In particular, we write down `NewBuffer` records which contain the thread
> ID. Today, we’re using 4 bytes for the thread ID which should really be 8
> bytes to support platforms that use 64-bit thread IDs (but that’s another
> issue).
>
> What I’m suggesting is writing down a different metadata record to host
> the process ID.
>
> > My understanding is that at initialization, the generated log file
> should contain:
> > [File header with pid][metadata record containing pid and tid]
> >
> > Updating handlers to fetch the PID and TID directly can be done.
> However, XRayRecord and XRayArgRecord do not have PID fields, Do I replace
> some of the padding with the PID field, or should I make another
> XRayPayload containing the TID/PID?
> >
>
> The log format for Basic Mode is different from FDR mode. In Basic Mode,
> after the file header we’ll have a stream of records each one containing
> all the information — currently that’s thread id along with the tsc and
> other details. That’s written down in the handler implementations (
> https://github.com/llvm-project/llvm-project-20170507/
> blob/master/compiler-rt/lib/xray/xray_basic_logging.cc#L224). In Basic
> Mode, changing the `XRayRecord` type (https://github.com/llvm-
> project/llvm-project-20170507/blob/master/compiler-rt/
> include/xray/xray_records.h#L73) to include the PID should be sufficient.
>
> What we need to change here to get the accurate thread ID is to not cache
> it in `getThreadLocalData` (https://github.com/llvm-
> project/llvm-project-20170507/blob/master/compiler-rt/lib/
> xray/xray_basic_logging.cc#L114) and instead call GetTid() directly when
> we write down the XRayRecord in the handler.
>
> > For now, what would be the best way to test the new format to make sure
> the format has the correct values?
>
> The patches I’ve pointed to show that we change the trace loading code in
> LLVM first to support the new records and new file versions. I suggested
> using the git monorepo to make the change simultaneously in LLVM (
> https://github.com/llvm-project/llvm-project-20170507/
> blob/master/llvm/lib/XRay/Trace.cpp#L682) and compiler-rt. You can ensure
> that none of the XRay tests in compiler-rt (https://github.com/llvm-
> project/llvm-project-20170507/tree/master/compiler-rt/test/
> xray/TestCases/Posix) fail, and can add tests that will use `fork()` to
> generate new version Basic and FDR mode logs that have the changes
> mentioned above.
>
> >
> > Thanks
> >
>
> No worries — thank you for your patience through this process!
>
> I’m based in Sydney, Australia and am ‘dberris’ on the LLVM IRC channel (
> https://llvm.org/docs/#irc). This explains some of the latency in my
> responses because timezones are hard. :)
>
> But if you message me and I respond, that’s a good indicator that I’m
> awake and able to chat in async but closer to real-time than through email
> (which is async but latency is in the order of minutes/hours/days).
>
> Let me know if anything else is still unclear.
>
> Cheers
>
> > On Wed, Jun 27, 2018 at 11:29 PM, Dean Michael Berris <
> dean.berris at gmail.com> wrote:
> >
> >
> > > On 28 Jun 2018, at 07:28, Henry Zhu <henryzhu at seas.upenn.edu> wrote:
> > >
> > > Thanks, that cleared up my confusion about version numbers.
> > >
> > > I've implemented the handlers and done up to 6) below. I'm unsure of
> how to test what I have added.
> > >
> > > 1) Update the Header.Version = 3 (from 2)
> > > 2) Add a new XRayRecord for pid (XRayPidRecord) in xray_basic_logging.h
> > > 3) Implement InMemoryRawLogWithPid similar to how
> InMemoryRawLogWithArg is implemented
> > > 4) Implement __xray_set_handler_pid
> > > 5) call __xray_set_handler_pid, passing in basicLoggingHandlePid in
> the initialization function for basic mode
> > > 6) Add an assembly stub for the [od handler
> > >
> >
> > Huh, I’m sorry for not being clear here — I suspect you don’t need steps
> 3 to 6.
> >
> > You may just need to add a metadata record at the beginning of the
> block, and getting the PID and TID directly (instead of the cached
> versions). This way FDR mode will have the PID record and the TID records
> at the beginning of the block.
> >
> > For Basic Mode we need to get the TID directly instead of using the
> cached version, and also to get the PID directly instead of attempting to
> cache it. This would be an update in the handlers.
> >
> > In Profiling Mode this would be a little tricky, because it may need
> changes in more places. I need to think about that I little more.
> >
> > > 7) Add additional parsing for pid for llvm-xray tool to parse the
> header for pid and xray entries for pid
> > >
> > > Q1. For 7), on order to log the pid, one would need to patch the
> function to call the pid logger. Should I add an attribute to clang that
> patches the function, so that the function calls the pid? Or is there an
> easier way to test the functionality of the pid logger?
> >
> > Please see above.
> >
> > > Q2. Should the PID always be set in the file header when xray starts?
> >
> > Yes.
> >
> > > Q3. How do I run test cases?
> >
> > There’s a ‘check-all’ and ‘check-xray’ target when you build
> LLVM+Clang+compiler-rt.
> >
> > > Q4. Would it be possible to always call the pid logger when fork is
> called?
> > >
> >
> > Unfortunately no. The simpler solution would be to update the handlers
> to get the PID alongside the TID, and to always get the TID instead of
> using a cached version.
> >
> > Cheers
> >
> > -- Dean
> >
> >
>
> -- Dean
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180703/63766967/attachment.html>