Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Heavy memory usage by Shaka. Chrome tab crashes after playing for a while. #184

Closed
torerikal opened this issue Sep 15, 2015 · 8 comments
Closed
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@torerikal
Copy link

Shaka seems to accumulate the playing stream buffer/ranges in memory in Chrome, resulting in large amounts of memory spent by the browser tab process. For instance, after one hour of playback, 1,6 GB is used by the browser tab, with Shaka playing the test stream referred underneath.

We have had other tests where the memory usage easily exceeded 2,5 GB, according to the task manager in Chrome. For a lot of users, this means Chrome and/or the computer slows down, and the browser or tab is crashing.

By recording heap allocations in the Profiles tool of Chrome DevTools, it looks like most of the memory usage originates from ArrayBuffer structures. Does Shaka miss some cleanup of played ranges?

This live test stream can be used for reproducing the problem in Shaka:
http://hls-live.akamai.tv2.no/wzlive3/_definst_/amlst:GT.smil/manifest.mpd

The problem is also apparent on on demand streams. For instance, movies are long enough to make this a problem for end users.

@tdrews
Copy link
Contributor

tdrews commented Sep 15, 2015

Thanks for the report. The player should not be holding on to any segment data once it passes it on to the browser (via the MSE API). We'll have to investigate.

@torerikal
Copy link
Author

The Shaka project test manifests are rather short, while real-world usage patterns of the player very often means continuous playing times above a hour or more.

Anyway, the Sintel example is long enough to indicate the memory leak(?). We have run tests and compared with Dash.js. The memory usage is sampled from Chrome's task manager:

skjermbilde 2015-09-23 kl 10 09 27

I hope you can prioritize looking into it, as it is a real problem affecting real usage patterns for the player: For instance, end users watching movies or sports events, lasting 1 to 2 hours, or even more, do report the Chrome tab to crash.

@tdrews tdrews added this to the v1.6.0 milestone Sep 23, 2015
@tdrews
Copy link
Contributor

tdrews commented Sep 23, 2015

There appears to be two issues here:

Shaka Player buffers as much data as the browser allows (by design). The player appends data to the buffer; the browser evicts old data as it sees fit. On my machine, on Chrome, if I play Sintel at 1080p, the browser begins to evict data after ~5 minutes of data has been buffered, and at this time the tab consumes ~700 MB of memory. I don't believe all players rely on the browser to evict data, which explains some of Shaka Player's memory usage. (You can check the size of the video's buffer with video.buffered.end(0) - video.buffered.start(0) [assuming there is only one buffered range].)

However, it's unclear why memory usage continues to grow after the browser begins to evict data.

@tdrews tdrews self-assigned this Sep 23, 2015
@torerikal
Copy link
Author

I run the Google DevTools profiler again, and for the largest memory size spent, it points to a lot of ArrayBuffer instances, as earlier mentioned. A closer look reveals that these are being members of FailoverUri instances' fulfilled request promises. In clear text, that means the XHR responses from segment fetching.

All the FailoverUri instances are stored in SourceBufferManager's member array this.inserted_, through their SegmentReference memberships, and this looks like the source of accumulation.

It's a bit early to draw conclusions, but I made a change to FailoverUri where the this.requestPromise_ reference was set to null as soon as the request was fulfilled (which might mean some segments must be reloaded).

This seems to have a significantly good effect on the memory usage, and the ArrayBuffer instances disappear from the profiler results.

When completing playback of the 1080P Sintel example from start to end, a few tests show that the tab has spent around 450 MB of memory after this change, as opposed to almost 1 GB earlier.

However, the memory consumption still increases proportionally with playback time, so there might be more sources for memory leaks.

More testing will be done.

@tdrews
Copy link
Contributor

tdrews commented Sep 23, 2015

Yep, we just spotted this ourselves. We'll have a patch up shortly to set requestPromise_ to null when the request completes. We actually don't want to cache segments anyways because it can interfere with bandwidth estimation.

@tdrews tdrews added the type: bug Something isn't working correctly label Sep 23, 2015
@torerikal
Copy link
Author

Great. Here is an updated chart with my observations after applying the change. As you see, there is still an accumulation of used memory during the near 15 minutes long Sintel playback, but less than half the magnitude. We will do some more tests tomorrow with longer playback (1-2 hours) to see how this graph develops.

skjermbilde 2015-09-23 kl 22 03 50

tdrews pushed a commit that referenced this issue Sep 24, 2015
Issue #184

Change-Id: If7137f96ac4994d593615ef6baecadb668218403
@torerikal
Copy link
Author

Testing two hours playback now shows just a small increase in memory usage after it being stabilized the first minutes. This is satisfactory to us. Thanks.

skjermbilde 2015-09-24 kl 12 20 23

@tdrews
Copy link
Contributor

tdrews commented Sep 24, 2015

Great. Thanks for taking the time to do the analysis.

tdrews pushed a commit that referenced this issue Oct 7, 2015
Issue #184

Change-Id: If7137f96ac4994d593615ef6baecadb668218403
@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

4 participants