Skip to content

Add trace events interface#5

Merged
jackm321 merged 2 commits intohouseroad:masterfrom
jackm321:add_trace_events
Mar 14, 2019
Merged

Add trace events interface#5
jackm321 merged 2 commits intohouseroad:masterfrom
jackm321:add_trace_events

Conversation

@jackm321
Copy link
Collaborator

Add a new interface for transmitting trace event information collected during calls to onnxSetIOAndRunGraph.

*/

ONNXIFI_PUBLIC ONNXIFI_CHECK_RESULT onnxStatus ONNXIFI_ABI
onnxReleaseTraceEvents(onnxTraceEventList* traceEvents);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my understanding is that events are created outside of the ONNXIFI backend implementation (e.g., onnxifiGlow).
Why do we need release here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that the backend will allocate an array of onnxTraceEvent and return a pointer to it, then onnxReleaseTraceEvents can free that array. Maybe allocating individual onnxTraceEvents and then onnxReleaseTraceEvents contains onnxTraceEvent **traceEvents; would be easier to work with for the backend and more clear? Either way I'll add some more comments here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the events have to be created in the Backend because the number of events is not known before the run occurs.

/**
* The number of events in traceEvents.
*/
int32_t numEvents;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be uint32_t.

* A list of of onnxTraceEvents, the length of which is indicated by
* numEvents.
*/
onnxTraceEvent *traceEvents;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will change this to onnxTraceEvent **traceEvents for easier use

@ipiszy
Copy link

ipiszy commented Mar 14, 2019

Do we want to support something like event trees? In this case we can have traces for call graphs.

@kimishpatel
Copy link

Hey Jack, Just curious to know what is the idea behind onnxTraceEvents? Is it track time spent in each op of the onnxified net, so as to enable getting timeline?

@jackm321
Copy link
Collaborator Author

@ipiszy Glow doesn't support this now but it's an interesting idea, we could probably extend this to including some stack metadata for each event

@jackm321
Copy link
Collaborator Author

@kimishpatel yes it's for enabling looking into timeline of events. Events can be from executing each op as well as things that happen inside of Glow runtime.

@jackm321 jackm321 merged commit ba2fdcb into houseroad:master Mar 14, 2019
jackm321 added a commit to jackm321/foxi that referenced this pull request Mar 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants