Stream: t-compiler/wg-self-profile

Topic: process-id in events


mw (May 16 2019 at 11:39, on Zulip):

It just occurred to me that Event does not have a process_id field. We should probably fix that as soon as possible.

Amanjeev Sethi (May 16 2019 at 11:40, on Zulip):

Is this a beginner friendly task?

mw (May 16 2019 at 11:42, on Zulip):

It might be once we decide what the type of that field should be

mw (May 16 2019 at 11:43, on Zulip):

@Wesley Wiser what do you think about process_id: Cow<'a, str>,? Is there a reason not to make this a string?

Wesley Wiser (May 16 2019 at 11:45, on Zulip):

I'm concerned about increasing the size of RawEvent

Wesley Wiser (May 16 2019 at 11:45, on Zulip):

I think this is the way we should store process id: https://github.com/rust-lang/measureme/issues/15

mw (May 16 2019 at 11:52, on Zulip):

I'm thinking only about Event here

mw (May 16 2019 at 11:53, on Zulip):

e.g. if you have a directory containing event streams from an entire cargo invocation and you want to iterate over all of them at once

mw (May 16 2019 at 11:53, on Zulip):

post-processing tools will be interested in which process an event came from

mw (May 16 2019 at 11:54, on Zulip):

unless you think that post-processing should handle this differently

mw (May 16 2019 at 11:55, on Zulip):

e.g. handle events from one process at a time

mw (May 16 2019 at 11:58, on Zulip):

my vision was (roughly):

Wesley Wiser (May 16 2019 at 12:00, on Zulip):

Nope, that's exactly what I was thinking as well :thumbs_up:

mw (May 16 2019 at 12:01, on Zulip):

OK, then @Amanjeev Sethi could take a stab at implementing this, if they are interested, right?

Wesley Wiser (May 16 2019 at 12:01, on Zulip):

Absolutely!

Amanjeev Sethi (May 16 2019 at 12:03, on Zulip):

sure! should i be creating a new gh ?issue

mw (May 16 2019 at 12:03, on Zulip):

I'll do that and I'll add some mentoring instructions

Amanjeev Sethi (May 16 2019 at 12:04, on Zulip):

appreciate that!

mw (May 16 2019 at 12:04, on Zulip):

The first step will be to implement https://github.com/rust-lang/measureme/issues/15

Wesley Wiser (May 16 2019 at 12:07, on Zulip):

I added some notes to #15

Wesley Wiser (May 16 2019 at 12:07, on Zulip):

Oh, @mw if you want to create a new issue, that's fine too. I can remove my comment.

mw (May 16 2019 at 12:07, on Zulip):

@Wesley Wiser I imagine the metadata to be a JSON object with some mandatory fields (at least process-id) and then an arbitrary number of other (string -> string) pairs

mw (May 16 2019 at 12:08, on Zulip):

do we want any other mandatory fields?

Wesley Wiser (May 16 2019 at 12:08, on Zulip):

I think start_time should be mandatory as well

mw (May 16 2019 at 12:08, on Zulip):

OK, in what format?

simulacrum (May 16 2019 at 12:09, on Zulip):

if we could stick the command run in there, that'd be good (but maybe not a required field)

mw (May 16 2019 at 12:09, on Zulip):

@simulacrum yeah, that sounds like a good idea

mw (May 16 2019 at 12:10, on Zulip):

what format do we store other timestamps in btw?? :)

Wesley Wiser (May 16 2019 at 12:10, on Zulip):

We lean on what serde does

Wesley Wiser (May 16 2019 at 12:11, on Zulip):

Which is { "seconds": u64, "nanos": u32 } I believe

simulacrum (May 16 2019 at 12:11, on Zulip):

hm, but this would probably be more useful as a absolute timestamp?

simulacrum (May 16 2019 at 12:12, on Zulip):

maybe that's also recorded like that though

mw (May 16 2019 at 12:12, on Zulip):

Oh, I think at the moment all our profiles start on 1/1/1970 :P

simulacrum (May 16 2019 at 12:12, on Zulip):

https://github.com/serde-rs/serde/blob/5fcdf0ff2be83fc73e29d73ea69674f10e791ebc/serde/src/ser/impls.rs#L608

mw (May 16 2019 at 12:13, on Zulip):

https://github.com/rust-lang/measureme/blob/582b99b5c6c03411076a3c8950630fff380a799a/measureme/src/profiling_data.rs#L80-L81

Wesley Wiser (May 16 2019 at 12:13, on Zulip):

In Rust terms, I think we should record a (SystemTime, Instant) pair. The SystemTime is what will go in the JSON as the start_time. The Instant is used to measure RawEvent offsets from the start time.

simulacrum (May 16 2019 at 12:16, on Zulip):

As a maybe-helpful note, this is what perf stores in it's header: https://github.com/torvalds/linux/blob/master/tools/perf/Documentation/perf.data-file-format.txt#L58

simulacrum (May 16 2019 at 12:17, on Zulip):

interesting tidbits could be available memory, CPU type, number of threads

mw (May 16 2019 at 12:18, on Zulip):

yes, that's true

simulacrum (May 16 2019 at 12:19, on Zulip):

but at least some of that is likely relatively problematic for us to uniformly query (especially on Windows, or systems without /proc/ basically)

mw (May 16 2019 at 12:19, on Zulip):

yeah, I mean this can be done on a best effort basis

simulacrum (May 16 2019 at 12:19, on Zulip):

and I wouldn't make them required as such -- I can open an issue linking to the perf.data format though and saying "maybe we should add these fields at some point" (memory, cpu type, and thread count being the most interesting I think)

mw (May 16 2019 at 12:20, on Zulip):

sgtm

Wesley Wiser (May 16 2019 at 12:20, on Zulip):

:+1:

Wesley Wiser (May 16 2019 at 12:21, on Zulip):

At the current time, I'm most interested in start_time, pid, and process_args.

Wesley Wiser (May 16 2019 at 12:21, on Zulip):

process_args should be optional though

mw (May 16 2019 at 12:36, on Zulip):

https://github.com/rust-lang/measureme/issues/15#issuecomment-493049088

mw (May 16 2019 at 13:06, on Zulip):

@Amanjeev Sethi Let me know if you have any questions regarding the instructions above

Amanjeev Sethi (May 16 2019 at 17:31, on Zulip):

Sorry, just got around to checking Zulip again. I have not read all the comments and discussions yet but even if I had, I am sure I will have a ton of questions, @mw :smiley:

Amanjeev Sethi (May 16 2019 at 19:00, on Zulip):

Is issue #15 is what I am supposed to work or is that issue just to point me to this comment ? Please advise.

Wesley Wiser (May 16 2019 at 19:06, on Zulip):

Yeah #15 is what you should work on :)

Last update: Nov 15 2019 at 21:10UTC