facebook / memlab

A framework for finding JavaScript memory leaks and analyzing heap snapshots
https://facebook.github.io/memlab/
MIT License
4.3k stars 116 forks source link

takeHeapSnapshot timed out #107

Closed Arooba-git closed 5 months ago

Arooba-git commented 6 months ago

Hi :) Thanks for this handy tool!

This is more of a question than an issue: I have used memlab with over 40 projects to get the heap size and memory leaks without much problem 👍 . Recently I have come across cases where heap size exceeds 1 GB and I'm getting 'Page crashed!' along with the suggestion to ' Increase the 'protocolTimeout' setting in launch/connect calls for a higher timeout if needed.' I want to know where to set this timeout value.. Thanks!

Screen Shot 2024-01-12 at 9 49 28 PM
JacksonGL commented 6 months ago

Thanks for using memlab. I can increase the default protocol timeout (currently it's 180s based on puppeteer's default setting) and perhaps adding a new option to allow setting protocol timeout via CLI.

Arooba-git commented 6 months ago

Yes, that would be great, thanks. I would appreciate it if you could notify me once it's done, as we need Memlab's result for an urgent task :)

Arooba-git commented 6 months ago

@JacksonGL Sorry to bother you again, but could I get an estimate of when the above issue will be resolved? Thanks.

JacksonGL commented 6 months ago

@Arooba-git I have a diff draft ready, but Monday is a holiday in the US (need my co-worker's review to ship it). So hopefully it will be landed on Tuesday. The code change will be linked to this GitHub issue, so GitHub should send a notification once the change is synced here.

In the meantime, if you need this earlier, please make this change here to your local git clone of memlab to unblock:

this.puppeteerConfig = {
  protocolTimeout: 5 * 60 * 1000, // set timeout to be 5 minutes
  ...
}

Then add proper typing annotations and build the local copy

JacksonGL commented 6 months ago

@Arooba-git The default protocol timeout has been increased from 3 minutes to 5 minutes. You can also use the new CLI flag --protocol-timeout <timeout-in-ms> to further increase the timeout in memlab v1.1.43.

Arooba-git commented 6 months ago

@JacksonGL Thanks for the prompt action. I'll try it out today 👍

Arooba-git commented 6 months ago

@JacksonGL Unfortunately I am getting the same error.. even after passing the --protocol-timeout with 300+ minutes (20000000 ms) :(. If I provide you with the scenario files it is happening on, would you be able to perhaps assist in finding out the root cause since I don't have much experience with Puppeteer, and it seems like the library's internal issue?

Screen Shot 2024-01-18 at 1 22 13 PM Screen Shot 2024-01-18 at 10 07 25 AM Screen Shot 2024-01-18 at 10 08 24 AM Screen Shot 2024-01-18 at 10 13 26 AM
JacksonGL commented 6 months ago

@Arooba-git I tested the new --protocol-timeout flag and it does timeout when I set the timeout to be very small, so the code change here indeed increased the protocol timeout.

The exception from your screenshot shows that it is thrown from Puppeteer's code; and, therefore it is outside of MemLab's scope. This is likely related to the connection between puppeteer and the headless browser. I would recommend filing a bug report on the Puppeteer's GitHub repo.

My guess is that either 1) 1.4GB of memory is too much for Chrome to capture the heap snapshot (you could try taking the 1.4GB heap snapshot manually in Chrome DevTools to see if the Browser DevTools times out) or 2) the connection between Puppeteer and Chrome was not designed to handle such a large heap snapshot.

Moreover, if a web app uses more than 1GB JS heap, it has some serious memory issues. I would recommend testing on a scenario where the memory leak is less severe.

Arooba-git commented 6 months ago

Alright, I'll do that then. 👍 Thank you for the guidance! You may close this issue.

P.S: While we are here, could you kindly take a look at this issue. I have spent 2 days trying to figure it out, and though it is not directly related to Memlab, I thought you might have some suggestions for this roadblock. Thanks.

JacksonGL commented 5 months ago

MemLab disables notification in browser by default:

To quickly see if it unblocks, you can comment out this flag and build it locally: https://github.com/facebook/memlab/blob/73c9fa3addd87eb50bc13129224ee467ad895d4b/packages/core/src/lib/Config.ts#L304-L305

Feel free to open a separate issue about this.