Closed mgmacias95 closed 4 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 92.92%. Comparing base (
41248ed
) to head (be0a687
). Report is 70 commits behind head on main.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hello @godlygeek,
I plan working on this by the end of the week. Thank you for your review! :)
Hey @godlygeek,
So when I said I would work on this by the end of the week, I meant by the end of the day 😅 😂
I fixed the issues you pointed out. Can you please review it again?
Thanks!
It looks like, while it's in "merge threads" mode, the header winds up incorrectly showing a thread count again the first time a new thread is created after entering "merge threads" mode. See this screenshot, for instance: You'll see that it shows "Thread 1 of 23" at the top, but "M Unmerge threads" at the bottom, so it's still in merge threads mode, but incorrectly showing a thread index and thread count.
Try using this test program:
import mmap
import threading
import time
def allocate():
with mmap.mmap(-1, 1024 * 1024):
time.sleep(1)
threads = [threading.Thread(target=allocate) for _ in range(3)]
for thread in threads:
time.sleep(3)
thread.start()
and pressing "m" to merge threads as soon as the TUI shows up. You'll see that the heading switches to saying "Displaying all threads." but as soon as a new thread shows up, it switches back to displaying a thread count.
Hello @godlygeek,
Thank you for your review again, I just pushed a commit fixing that problem. Can you take another look? Thanks!
Cheers!
Hello @godlygeek,
I have fixed some conflicts that appeared in 2f50fa7b0ec605b1fa99ef69515d4db2de34bda6. Can you take a look again please? 🥺
Thanks!
Issue number of the reported bug or feature request: https://github.com/bloomberg/memray/discussions/587
Describe your changes This PR adds a new option in the live view called "merge threads" that summarizes all allocations in a single view. It also hides the buttons ">" and "<" because you can't move between threads in that moment.
In addition, I added some missing docs to the readme.
Testing performed Manual testing + unit tests included in this PR