profile: update demo data to include trace-only run #1942
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Thanks. I’ll send out a fix for the relevant bug presently. |
wchargin
approved these changes
Mar 2, 2019
wchargin
changed the title
Contributor
|
Edited description to include test plan, per the PR template. |
wchargin
added a commit
that referenced
this pull request
Mar 2, 2019
Summary: The profile dashboard had an existing bug that was revealed by #1914: the loading progress bar never actually finishes. Prior to #1914, you could actually see the loading bar if you made your window narrow enough:  This was because the prior version of the dashboard rendered the main tool irrespective of whether the data had actually finished loading. As of #1914, the “loading” and “active” states are mutually exclusive, so instead of just having a zombie progress bar we actually block the view. The cause of the bug is a promise that never resolves. (The executor provided to the `Promise` constructor’s return value is ignored; only the `resolve` and `reject` callbacks can fulfill a promise.) This was not caught while testing #1914 because it cannot be reproduced with the demo data present at that commit; with that more complete set of data, there are other code paths that clobber the progress value to 100% (which is really a separate issue, but that’s not important right now), such as `_maybeUpdateData`. Test Plan: Regenerate profile plugin demo data if you haven’t done so since #1942, then launch TensorBoard: ``` $ rm -rf /tmp/profile_demo/ $ bazel run //tensorboard/plugins/profile:profile_demo $ bazel run //tensorboard -- --logdir /tmp/profile_demo ``` The trace viewer should be displayed (prior to this commit, it wasn’t). wchargin-branch: profile-broken-promises
wchargin
added a commit
that referenced
this pull request
Mar 2, 2019
Summary: The profile dashboard had an existing bug that was revealed by #1914: the loading progress bar never actually finishes. Prior to #1914, you could actually see the loading bar if you made your window narrow enough:  This was because the prior version of the dashboard rendered the main tool irrespective of whether the data had actually finished loading. As of #1914, the “loading” and “active” states are mutually exclusive, so instead of just having a zombie progress bar we actually block the view. The cause of the bug is a promise that never resolves. (The executor provided to the `Promise` constructor’s return value is ignored; only the `resolve` and `reject` callbacks can fulfill a promise.) This was not caught while testing #1914 because it cannot be reproduced with the demo data present at that commit; with that more complete set of data, there are other code paths that clobber the progress value to 100% (which is really a separate issue, but that’s not important right now), such as `_maybeUpdateData`. Test Plan: Regenerate profile plugin demo data if you haven’t done so since #1942, then launch TensorBoard: ``` $ rm -rf /tmp/profile_demo/ $ bazel run //tensorboard/plugins/profile:profile_demo $ bazel run //tensorboard -- --logdir /tmp/profile_demo ``` The trace viewer should be displayed (prior to this commit, it wasn’t). wchargin-branch: profile-broken-promises
wchargin
added a commit
to wchargin/tensorboard
that referenced
this pull request
Mar 5, 2019
Summary: The profile dashboard had an existing bug that was revealed by tensorflow#1914: the loading progress bar never actually finishes. Prior to tensorflow#1914, you could actually see the loading bar if you made your window narrow enough:  This was because the prior version of the dashboard rendered the main tool irrespective of whether the data had actually finished loading. As of tensorflow#1914, the “loading” and “active” states are mutually exclusive, so instead of just having a zombie progress bar we actually block the view. The cause of the bug is a promise that never resolves. (The executor provided to the `Promise` constructor’s return value is ignored; only the `resolve` and `reject` callbacks can fulfill a promise.) This was not caught while testing tensorflow#1914 because it cannot be reproduced with the demo data present at that commit; with that more complete set of data, there are other code paths that clobber the progress value to 100% (which is really a separate issue, but that’s not important right now), such as `_maybeUpdateData`. Test Plan: Regenerate profile plugin demo data if you haven’t done so since tensorflow#1942, then launch TensorBoard: ``` $ rm -rf /tmp/profile_demo/ $ bazel run //tensorboard/plugins/profile:profile_demo $ bazel run //tensorboard -- --logdir /tmp/profile_demo ``` The trace viewer should be displayed (prior to this commit, it wasn’t). wchargin-branch: profile-broken-promises
wchargin
added a commit
that referenced
this pull request
Mar 6, 2019
Summary: The profile dashboard had an existing bug that was revealed by #1914: the loading progress bar never actually finishes. Prior to #1914, you could actually see the loading bar if you made your window narrow enough:  This was because the prior version of the dashboard rendered the main tool irrespective of whether the data had actually finished loading. As of #1914, the “loading” and “active” states are mutually exclusive, so instead of just having a zombie progress bar we actually block the view. The cause of the bug is a promise that never resolves. (The executor provided to the `Promise` constructor’s return value is ignored; only the `resolve` and `reject` callbacks can fulfill a promise.) This was not caught while testing #1914 because it cannot be reproduced with the demo data present at that commit; with that more complete set of data, there are other code paths that clobber the progress value to 100% (which is really a separate issue, but that’s not important right now), such as `_maybeUpdateData`. Test Plan: Regenerate profile plugin demo data if you haven’t done so since #1942, then launch TensorBoard: ``` $ rm -rf /tmp/profile_demo/ $ bazel run //tensorboard/plugins/profile:profile_demo $ bazel run //tensorboard -- --logdir /tmp/profile_demo ``` The trace viewer should be displayed (prior to this commit, it wasn’t). wchargin-branch: profile-broken-promises
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
We make this change because we have a bug in profile plugin that will be
triggered if logdir only contains trace data.
Test Plan:
Remove any old profile demo data, then regenerate the data and launch
TensorBoard:
Prior to this commit, this would have appeared to work; now, it hangs
on the “Processing datasets” screen. Fix to come shortly.