bloomberg / memray

Memray is a memory profiler for Python
https://bloomberg.github.io/memray/
Apache License 2.0
13.17k stars 392 forks source link

Add pause button to live view #418

Closed bilbofroggins closed 1 year ago

bilbofroggins commented 1 year ago

Issue number of the reported bug or feature request: #404

Describe your changes P key to pause, and when paused, U key to unpause. Couldn't see anything built in to rich live that will automatically do this, so I just split the TUI class into a layout class (TUI) and a data class (TUIData) so that we can snapshot the data when the user pauses and show the snapshot while continuing to write to the live version of data in the background (in update_snapshot)

Previously used to put the whole _snapshot (Iterable[AllocationRecord]) within the TUI class and then extract what we cared about in the render stage (get_body) but there's a bunch of extra stuff in there it seems like is not needed. Instead, extracting the data we need in update_snapshot and then get_body can just pull from TUIData.

Testing performed Tested an application that has a couple threads - made sure you can still arrow through them. Sorting still works and carries over paused/unpaused states. Ran pytest locally.

Closes: https://github.com/bloomberg/memray/pull/418

pablogsal commented 1 year ago

I have not reviewed the code yet, but I noticed that when running memray run --live -m test test_asyncio and pausing, hitting U or P to unpause doesn't continue the process and the table does not keep updating for me.

pablogsal commented 1 year ago

Update: seems that it only updates when I keep pressing U

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: +0.13 :tada:

Comparison is base (3ca2337) 84.85% compared to head (37bc373) 84.99%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #418 +/- ## ========================================== + Coverage 84.85% 84.99% +0.13% ========================================== Files 29 29 Lines 3631 3631 ========================================== + Hits 3081 3086 +5 + Misses 550 545 -5 ``` | Flag | Coverage Ξ” | | |---|---|---| | cpp | `84.99% <ΓΈ> (+0.13%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bloomberg#carryforward-flags-in-the-pull-request-comment) to find out more. [see 3 files with indirect coverage changes](https://app.codecov.io/gh/bloomberg/memray/pull/418/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bloomberg)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

bilbofroggins commented 1 year ago

Update: seems that it only updates when I keep pressing U

Oh interesting, I'm not seeing that on my side but I just put in a flag to prevent data from being copied a bunch while holding the keys down - and pushed that up. Possibly related?

pablogsal commented 1 year ago

Update: seems that it only updates when I keep pressing U

Oh interesting, I'm not seeing that on my side but I just put in a flag to prevent data from being copied a bunch while holding the keys down - and pushed that up. Possibly related?

~Unfortunately still happens on my end after the latest commit~ Hummm, I think my terminal was busted or something because after retrying it works as expected! Sorry for the confusion @bilbofroggins

pablogsal commented 1 year ago

Thanks a lot @bilbofroggins this looks fantastic. Very good job! πŸš€

Can you maybe add a small test that checks the pause() and unpause() functions simulating an update in the middle?

bilbofroggins commented 1 year ago

Thanks a lot @bilbofroggins this looks fantastic. Very good job! πŸš€

Can you maybe add a small test that checks the pause() and unpause() functions simulating an update in the middle?

πŸ‘ Added

pablogsal commented 1 year ago

Will make another pass tomorrow! Thanks for the ping