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

Fixable memory leak in Chromium based browsers #164

Open
zsilbi opened this issue Apr 30, 2023 · 5 comments
Open

Fixable memory leak in Chromium based browsers #164

zsilbi opened this issue Apr 30, 2023 · 5 comments

Comments

@zsilbi
Copy link

zsilbi commented Apr 30, 2023

Hi!

I found a nice memory leak issue in Chromium based browsers due to a V8 engine string slicing problem.
More about it here: https://bugs.chromium.org/p/v8/issues/detail?id=2869

This causes a longer DVR live stream (tested with 10 hours long) to crash in 30-40 minutes in video.js 7+ due to extreme memory consuption.

235371930-1a7810ff-a42c-4610-aa49-d4a21186c960

235372738-9bb0a8fc-8502-4376-aae8-2075b417d5f6

A possible solution is just a small hack that's required for every parsed string.

For example:

if (line[0] !== "#") {
  this.trigger("data", {
    type: "uri",
    uri: (" " + line).substring(1), // <-------
  });
  return;
}
match = /^#EXT-X-PROGRAM-DATE-TIME:(.*)$/.exec(newLine);

if (match) {
  event = {
    type: 'tag',
    tagType: 'program-date-time'
  };

  if (match[1]) {
    event.dateTimeString = (" " + match[1]).substring(1);  // <-------
    event.dateTimeObject = new Date(match[1]);
  }

  this.trigger('data', event);
  return;
}

These modifications reduced the players memory consuption from 2 GB to 30 MB.

Best regards,
zsilbi

@gkatsev
Copy link
Member

gkatsev commented May 1, 2023

Thanks for the detailed issue! We've definitely had this come up before, but we didn't realize that the issue was the sliced strings!

Since we use RegExp to parse the m3u8, I wonder if the /./.exec("a") trick is the more "correct" one to fix this?

@zsilbi
Copy link
Author

zsilbi commented May 1, 2023

Since we use RegExp to parse the m3u8, I wonder if the /./.exec("a") trick is the more "correct" one to fix this?

I saw that, but I didn't have time to test that. To be honest I am in hurry atm, so after I had patched my parser I moved on to finish the stuff I am working on. Hopefully next week I can take a look at that.

I did one more test at night with a 12 hour DVR stream to make sure everything is OK and the heap snapshot was 26.8 MB in the morning (for the whole project).

@gkatsev
Copy link
Member

gkatsev commented May 1, 2023

Running a quick test locally with the exec thing, but I'm not sure that the DVR streams I have access too are long enough to make a big different.

It's possible that doing both would be even better, who knows. After your event, if you have local changes and are able to submit a PR, that would be amazing!

@zsilbi
Copy link
Author

zsilbi commented May 1, 2023

Running a quick test locally with the exec thing, but I'm not sure that the DVR streams I have access too are long enough to make a big different.

I sent you an email with my development stream test url.
If you open the link and take a heap snapshot you'll see this:

image

When the DVR stream is running live, a big raw m3u8 string appears after each parse, that's why it fills the memory so fast.

@gkatsev
Copy link
Member

gkatsev commented May 1, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants