Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support streaming trace viewer. #1128

Merged
merged 1 commit into from
Apr 12, 2018
Merged

support streaming trace viewer. #1128

merged 1 commit into from
Apr 12, 2018

Conversation

trisolaran
Copy link
Contributor

trace viewer normally involve huge amount of data which browser can not
handle. streaming trace viewer use grpc to connect to a tpu profiler
analysis service to get small part of whole trace and display to user.
the amount of data depends on current view port (defined by a start and
end timestamp) and resolution (defined by zoom level).
currently the server reside on google owned virtual machine. there are
plans to open source those part.

to avoid confusing user, the patch is done in a backward compatible
fashion. user are expected to use old trace viewer (which limit the
event to 1 million), if the user specify the MASTER_TPU environment to
the IP address of the green VM, we will try to estabilish a grpc stub to
that IP:8466 port. only if stub is successfully connected we will use
new tools 'trace_viewer@' to OVERRIDE the old trace_viewer. Override
will hide old trace viewer tools to avoid confusing user.

Upon receiving the tool/data probing in python, we will determine which
tools to use. using 'trace_viewer@' will signal the downstream
componenet that streaming trace viewer is in use.

Most of the trace viewer related code already exist within google3. I
removed host filters etc for the reason that we don't support multiple
hosts yet.

Copy link
Contributor

@jart jart left a comment

Choose a reason for hiding this comment

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

I'm very happy with this pull request. Thank you for taking the time to put these finishing touches on #1096.

requestURL.searchParams.set("end_time_ms", requestedRange.max);
}

return new Promise(function(resolve, reject) {
Copy link
Contributor

Choose a reason for hiding this comment

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

[no action required] Thank you for refactoring this XHR logic. This code is crystal clear.

Please note that, while polyfills exist, I might remove Promise in the future. See recent tf-beholder-info.html refactor to do XHR / Timer cleanup on Polymer::detached(). We likely agree, since we've both written production code at Google, which has always thrown its support more towards imperative OOP threading with 64kb stacks, versus asynchronous i/o or NodeJS-influenced callbacks. Please feel no obligation in the future to use more modern conventions when contributing to TensorBoard.

@jart jart merged commit b42b9f0 into tensorflow:master Apr 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants