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

optimize memory view using lttb sample #776

Closed
wants to merge 3 commits into from
Closed

Conversation

lh-ycx
Copy link
Contributor

@lh-ycx lh-ycx commented Jun 29, 2023

Enhancement for Issue #760.

Hey guys, I've optimized the speed of the memory view using the LTTB sampling (Largest-Triangle-Three-Buckets sampling, which is able to downsample time series–like data while retaining the overall shape and variability in the data).

I've tested this using a PyTorch profiler trace of 2G, and the memory view page will not get crashed and the scaling operation is smooth and rather acceptable to me.

@lh-ycx
Copy link
Contributor Author

lh-ycx commented Jun 30, 2023

@chaekit @aaronenyeshi I checked the failed test (test_tensorboard_end2end.py). This test compares the response from the tensorboard frontend with the manually generated and verified result_check_file.txt.

Since the memory curve has been sampled, it cannot match the previous manually generated result. As a result, the test fails.

Shall I regenerate the result_check_file.txt? Or, is there any other method to let the test pass?

@aaronenyeshi
Copy link
Member

@lh-ycx , yes please feel free to re-generate the file. can you please share screenshots of the before and after views of this change?

@lh-ycx
Copy link
Contributor Author

lh-ycx commented Jul 11, 2023

OK, I'll work on it this week.

@lh-ycx
Copy link
Contributor Author

lh-ycx commented Jul 24, 2023

Hi, @aaronenyeshi. Sorry for the late response.

CI failures

I've carefully checked the test case and found that the test would always fail in Python 3.9 and 3.10 (please refer to #785). I also find my commit is consistent with the content in result_check_file.txt. So there is no need to regenerate the file. Could you please rerun the CI? Thanks a lot.

Screenshots of the before and after views

I use a 329M trace to show the before and after views as well as the memory footprint. (A larger trace (up to GBs) can be selected but the unoptimized view would crash.)

Before view:
image

After view:
image

Before memory footprint:
image

After memory footprint:
image

Copy link
Member

@aaronenyeshi aaronenyeshi left a comment

Choose a reason for hiding this comment

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

LGTM, the previous CI issues were unrelated to the changes in this diff.

@facebook-github-bot
Copy link
Contributor

@aaronenyeshi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@aaronenyeshi merged this pull request in 170d45a.

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.

3 participants