search

jlfwong / speedscope

🔬 A fast, interactive web-based viewer for performance profiles.
https://www.speedscope.app
MIT License
5.58k stars 247 forks source link

Fix crash when importing big linux perf tool files #435

Closed Goose97 closed 1 year ago

Goose97 commented 1 year ago

Currently, importing files generated by linux perf tool whose some blocks exceed V8 strings limit can crash the application. This issue is similar to the one in #385.

This PR fixes it by changing parseEvents to work directly with lines instead of chunking lines into blocks first.

Fixes #433

coveralls commented 1 year ago

Coverage Status

coverage: 42.952% (-0.06%) from 43.012% when pulling e1dacfe9f625f15923085ccf57e07cd2384ef687 on Goose97:fix-large-file-crash into 26884c116c05ece8133c048ab32a63b12ccca3a5 on jlfwong:main.

jlfwong commented 1 year ago

I'm also curious how quickly such a large file loads, and what the performance bottlenecks of the load are

Goose97 commented 1 year ago

Also, just as a sanity check, this does now load the massive file you referenced in the issue, right?

Yes, it works ok now

jlfwong commented 1 year ago

Thank you!

Goose97 commented 1 year ago

I'm also curious how quickly such a large file loads, and what the performance bottlenecks of the load are

Ironically, I just realize that it's not a linux perf tool file but a Brendan Gregg-format file. Oh silly me. The code just accidentally hits the error part and crashes.

Anw, it took ~23 seconds to import the file and took about ~1 second to render. Out of 23 seconds, ~2 seconds was reading the file into the buffer. The rest is parsing. I have the Chrome profiler dump but it's too large to attach to Github. Do you want to have a look?

System info:

jlfwong commented 1 year ago

Ironically, I just realize that it's not a linux perf tool file but a Brendan Gregg-format file. Oh silly me. The code just accidentally hits the error part and crashes.

Ah jeez, I see -- for plaintext files with no known file extension, we always try importFromLinuxPerf before importFromBGFlameGraph.

If you can think of a way of avoiding that by doing some cheap heuristic detection on one format or the other, I'd take a PR to do that too.

To see if that's worthwhile for performance reasons, you could delete the lines that run importFromLinuxPerf, and try to import the massive file you're working with.