[llvm-dev] XRay feature – pid reporting

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[llvm-dev] XRay feature – pid reporting

Zhizhou Yang via llvm-dev

Hello,

 

I’ve noticed that XRay does not report the PID in the trace log. Could this be added for multi-process support? (Maybe as a command line argument).

 

Thanks,

Henry



_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] XRay feature – pid reporting

Zhizhou Yang via llvm-dev


> On 12 Jun 2018, at 07:49, Henry Zhu via llvm-dev <[hidden email]> wrote:
>
> Hello,
>
>  
> I’ve noticed that XRay does not report the PID in the trace log. Could this be added for multi-process support? (Maybe as a command line argument).
>

We can certainly add the PID in the trace header. This will be a format change which will require a version increment, but we’ve done that before so it should be doable.

Do you have time to implement this feature? It will span LLVM and compiler-rt, but it should be straight-forward to do so.

Let me know and I’ll get you an example patch that did this for some of the changes we’ve done recently in this regard. Keith can also help as he’s worked on updating the FDR format in the past as well.

Cheers

-- Dean

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] XRay feature – pid reporting

Zhizhou Yang via llvm-dev
I would be happy to help. Could you send me the example patch? Where would I submit my patch to be reviewed?

Thanks,
Henry

On Mon, Jun 11, 2018 at 8:31 PM, Dean Michael Berris <[hidden email]> wrote:


> On 12 Jun 2018, at 07:49, Henry Zhu via llvm-dev <[hidden email]> wrote:
>
> Hello,
>

> I’ve noticed that XRay does not report the PID in the trace log. Could this be added for multi-process support? (Maybe as a command line argument).
>

We can certainly add the PID in the trace header. This will be a format change which will require a version increment, but we’ve done that before so it should be doable.

Do you have time to implement this feature? It will span LLVM and compiler-rt, but it should be straight-forward to do so.

Let me know and I’ll get you an example patch that did this for some of the changes we’ve done recently in this regard. Keith can also help as he’s worked on updating the FDR format in the past as well.

Cheers

-- Dean



_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] XRay feature – pid reporting

Zhizhou Yang via llvm-dev
Hi Henry,

> On 26 Jun 2018, at 07:07, Henry Zhu <[hidden email]> wrote:
>
> I would be happy to help. Could you send me the example patch? Where would I submit my patch to be reviewed?
>

Thanks!

First, please see https://llvm.org/docs/Contributing.html#how-to-submit-a-patch which details the process of contributing to the LLVM project.

There are two patches that show how we changed the version number for Basic mode (formerly known as “naive” mode):

- https://reviews.llvm.org/D38550 (change in the tooling to support newer versions of the files)
- https://reviews.llvm.org/D38551 (change in the runtime to write out new kinds of records; in this case we started writing out tail exits)

We have the chance now to change this in a single patch through the git monorepo process detailed in http://llvm.org/docs/GettingStarted.html#for-developers-to-work-with-a-git-monorepo — but that’s only if you’d like to do that in a single patch.

Cheers
_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] XRay feature – pid reporting

Zhizhou Yang via llvm-dev


> On 27 Jun 2018, at 01:46, Henry Zhu <[hidden email]> wrote:
>
> I'm not too clear on what you mean by changing the version number. Does that mean that I should change the Header.Version = 3? Therefore, I should add XRayPidPayload to xray_records.h and install a handler for fetching the pid like what was done in those commits?
>

This is the preferred way of doing it (updating the log version number). We have a set of metadata records in FDR mode to support writing down the thread ID at the start of the FDR mode block as a trade-off to reduce the log size. We’ll need a similar change in FDR mode to support writing the process ID at the start of the block as well.

Basic mode doesn’t make this trade-off which means you need to update the basic mode file header and function records to always include the process ID.

> Originally, I thought of modifying XRayRecord to contain pid field since there is an unused buffer at the end that could hold the pid value.
>

The XRayRecord type in LLVM is only used by the tooling, so this definitely could work. In compiler-rt though we need to be able to write them down in all modes that we support — this means writing it down in FDR, Basic, and Profiling mode.

For Basic mode, adding the PID in every record should be OK but we still need to update the version number and add the PID in the file header as well.

Cheers

-- Dean

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] XRay feature – pid reporting

Zhizhou Yang via llvm-dev


> On 28 Jun 2018, at 07:28, Henry Zhu <[hidden email]> 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

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] XRay feature – pid reporting

Zhizhou Yang via llvm-dev
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?

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?

For now, what would be the best way to test the new format to make sure the format has the correct values?

Thanks

On Wed, Jun 27, 2018 at 11:29 PM, Dean Michael Berris <[hidden email]> wrote:


> On 28 Jun 2018, at 07:28, Henry Zhu <[hidden email]> 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



_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] XRay feature – pid reporting

Zhizhou Yang via llvm-dev


> On 29 Jun 2018, at 07:18, Henry Zhu <[hidden email]> 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 <[hidden email]> wrote:
>
>
> > On 28 Jun 2018, at 07:28, Henry Zhu <[hidden email]> 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

_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Reply | Threaded
Open this post in threaded view
|

Re: [llvm-dev] XRay feature – pid reporting

Zhizhou Yang 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 <[hidden email]> wrote:


> On 29 Jun 2018, at 07:18, Henry Zhu <[hidden email]> 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 <[hidden email]> wrote:
>
>
> > On 28 Jun 2018, at 07:28, Henry Zhu <[hidden email]> 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



_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev