bloomberg / memray

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

Flamegraph view gets stuck #396

Closed mgmacias95 closed 1 year ago

mgmacias95 commented 1 year ago

Is there an existing issue for this?

Current Behavior

I made this simple script to test the new temporary flamegraphs feature:

import random
import string
from collections import Counter

def set_up():
  b = random.choice(string.ascii_letters)
  return Counter(b)

def run():
  c = set_up()
  key = random.choice(list(c.keys()))
  print(f'{key}: {c[key]}')

run()

I executed the following commands:

$ python -m memray run test.py                                   
Writing profile results into memray-test.py.73578.bin
o: 1
[memray] Successfully generated profile results.

You can now generate reports from the stored allocation records.
Some example commands to generate reports:

/Users/mgmacias/Documents/GitHub/memray/env/bin/python -m memray flamegraph memray-test.py.73578.bin
$ python -m memray flamegraph memray-test.py.73578.bin --temporal 
Wrote memray-flamegraph-test.py.73578.html

Then opened the flamegraph on my chrome browser. The flamegraph gets stuck on this loading view:

image

When checking the chrome dev console I can see the following error message:

image

Expected Behavior

An error message or a flamegraph.

Steps To Reproduce

  1. Save the script to a python file
  2. Run the commands mentioned above
  3. Open the flamegraph in chrome

Memray Version

1.8.0

Python Version

3.11

Operating System

macOS

Anything else?

No response

pablogsal commented 1 year ago

There are two possible problems that this bug surfaces:

pablogsal commented 1 year ago

This seems to happen because in this case high_water_mark_by_snapshot is an empty array.

pablogsal commented 1 year ago

I think at the very least this:

https://github.com/bloomberg/memray/blob/c2c8d53393de25670a9da0325c34027a7ed4c015/src/memray/reporters/assets/temporal_flamegraph.js#L55

should be

  if (hwms && hwms.length > 0) {

but I am curious if the fact that that is empty is expected @godlygeek. In any case, there is something more because there are more undefined stuff.

pablogsal commented 1 year ago

I think this is enough to fix it:

diff --git a/src/memray/reporters/assets/temporal_flamegraph.js b/src/memray/reporters/assets/temporal_flamegraph.js
index d43a509..259e5f0 100644
--- a/src/memray/reporters/assets/temporal_flamegraph.js
+++ b/src/memray/reporters/assets/temporal_flamegraph.js
@@ -52,7 +52,7 @@ function packedDataToTree(packedData, rangeStart, rangeEnd) {
   }

   const hwms = packedData.high_water_mark_by_snapshot;
-  if (hwms) {
+  if (hwms && hwms.length > 0 && memory_records.length > 0) {
     console.log("finding highest high water mark in range");
     let hwmSnapshot = rangeStart;
     let hwmBytes = hwms[rangeStart];