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

Doc: change suggested way of starting the profiler #4120

Merged
merged 1 commit into from
Aug 21, 2020

Conversation

wrzadkow
Copy link
Contributor

When using TensorBoard profiler on Google Compute Platform VM, I noticed that

import jax.profiler
jax.profiler.start_server(9999)	   

results in TensorBoard not "seeing" the profiling server, i.e. displaying "Failed to capture profile: empty trace result." message. This seems to be fixed after simply using:

import jax.profiler
server = jax.profiler.start_server(9999)	   

Background is that the server is alive as long as the object returned by jax.profiler.start_server(9999) is not destroyed and I think without assigning it to a variable that might happen immediately.

This PR aims at changing the suggested way of starting the server as well as making users aware of server's lifetime.

@google-cla google-cla bot added the cla: yes label Aug 21, 2020
@froystig froystig added the bug Something isn't working label Aug 21, 2020
@froystig
Copy link
Member

Maybe we should keep the resulting object alive within the profiler module? Then this requirement of caller behavior (and documentation) won't be needed, if I understand correctly.

@skye
Copy link
Collaborator

skye commented Aug 21, 2020

Thanks @wrzadkow for tracking this down! I'm not sure how it ever worked for me...

I'm gonna commit the documentation change. I think we should expose the underlying server object in case someone does wanna destroy it for some reason (to restart it?), and while we could also expose a separate function that keeps it alive, I think it's only slightly inconvenient to keep it alive oneself. Also, in the long run, we'd like to expose functionality to collect a trace without manually starting a server at all.

@skye
Copy link
Collaborator

skye commented Aug 21, 2020

The above is a lightly-held position though, if anyone feels strongly that start_server should keep the server alive let's do that! I'm gonna commit this for now but we can revert if someone wants to change the behavior.

@skye skye merged commit 9d733dd into jax-ml:master Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants