1527340 - Surface additional payload information for file io

Status: NEW

(Core :: Gecko Profiler, enhancement, P2)

Description

[Greg Tatum [:gregtatum] (REO Fx132)]

From :jesup in Bug 1464506

You absolutely can take it over... however I really would want to have IO information from any captured thread - anyone doing perf work on a subsystem that does it's IO off-main-thread needs that (and we want all IO off the main thread).

If you don't mind modifying an existing payload, you could add fields to IOMarker. The FileIOMarkers I made would let you untangle what IO is for what file; how much data was read/written, etc.

Also see the WIP patch in Bug 1464506.

https://bugzilla.mozilla.org/attachment.cgi?id=9042202&action=diff

Comment 1

[Greg Tatum [:gregtatum] (REO Fx132)]

Hey :jesup, I was looking through your WIP patch, and I was wondering if I could some clarity on what the intention was on some of the fields.

Current FileIOMarkerPayload contains:

• operation (string) read, write, etc.
• source (string) The interposer source (e.g. "PoisionIOInterposer")
• filename (optional string) The filename

Your patch contains:

• id (uint64_t) - How is this generated? What's the idea here?
• type (FileIOType enum) This is the operation above, which is current stored as a duplicated string.
• name - Not sure, is this the source?
• flags - (int64_t FileIOType::OPEN only) Some kind of file open flags?
• mode - (int64_t, FileIOType::OPEN only) Some kind of open file mode?
• length - (size_t, FileIOType::READ | FileIOType::WRITE) Is this bytes read and written?

I'm investigating the PoisonIOInterposerMac to see how to start wiring more information in. I'm guessing the approach would be to access the values through the wrapped native functions, and add it onto the IOInterposeObserver::Observation class.

Comment 2

[Randell Jesup [:jesup] (needinfo me)]

id is so that you can match up markers for the same file/filehandle.
name is the name of the file (only set on Open/etc; mode likewise)
length is bytes read or written, which is quite handy at times to see; I've used this patch to look at those)

Comment 3

[Greg Tatum [:gregtatum] (REO Fx132)]

This is proving a bit more complicated than I originally thought. The payload definition update is pretty straightforward, but actually getting at the information is a bit more work, and requires more expertise. This information is different per platform, and per interposer source. After the information is accessed, and coerced into the proper format, it then needs to be wired through the existing interposer class.

Comment 4

[Greg Tatum [:gregtatum] (REO Fx132)]

I'm unassigning myself from this bug and making it a P2, as I don't plan on working on it right this moment.