Growing memory usage with DASH live streams #6610
Comments
|
Trouble shooting Since the older version doesn't show this behavior I started to test different releases to track down when the problem started to occur. Seems to be between v4.6.0 and v4.6.1, more precisely: If this row is removed in v4.8.3, the huge growing memory usage disappear. This row is not needed for the stream that cause this problem. |
|
Thoughts and attempts to fix Well I try to get rid of this growing memory, by changing this row. It turned out to be problematic. Here is a summary of testing (by playing in Cobalt browser): These helps: I prefer the last, put in a new function. I would appreciate getting links to live streams that trigger calls to the appendTemplateInfo function. |
|
More tests Same test condition, but our stream: Test in Chromium (124.0.6367.155) with default shaka config (MB from heap snapshot with developer tools), stream from cloudfront: Test in Chromium with our stream: Please don't compare MB (MegaByte) values between Cobalt and Chromium, they show different memory. The fix: --- a/lib/dash/segment_template.js
+++ b/lib/dash/segment_template.js
@@ -714,7 +714,11 @@ shaka.dash.TimelineSegmentIndex = class extends shaka.media.SegmentIndex {
} else {
const currentTimeline = this.templateInfo_.timeline;
- this.templateInfo_.mediaTemplate = info.mediaTemplate;
+ {
+ // Create a new string, which has no reference to the source
+ const newString = '_' + info.mediaTemplate;
+ this.templateInfo_.mediaTemplate = newString.substring(1);
+ }
// Append timeline
const lastCurrentEntry = currentTimeline[currentTimeline.length - 1];The pull request will also include an if-statement (to avoid unnecessary assignment/copy of string) and a function. |
|
Conclusion So right now I'm little confused. Shall I put up a pull request, or not. Any ideas are welcome. I believe I need to think about this for a while, but guess the fix would be good. Maybe I will trouble shoot again, but with a different approach. Maybe it would be possible to solve the issue at another place in the code. |
|
Chrome and Firefox have different JS engines (V8 and Spidermonkey, respectively), so it's very possible that GC issues could differ by browser. However, the workaround above (build new string and take a substring) looks very fishy. A string value should not hold any references to a parent object that contains it. I have a hard time imagining a scenario where it's not an independent memory allocation. Obviously, solving this issue would be good, but I'm hesitant to put in the fix you posted above, because of all of the following:
I would like to see this solved, but I feel strongly that we need to understand and document the root cause or the mechanism behind the workaround. |
|
I agree with you. Cobalt using V8, but old 8.8 version (Chromium have 12.4). |
|
I can add, as info, that for our problematic stream the template string is always the same. I have tested more browsers, here is a summary: The above "ok" is a check if the JS heap size reported by development tools in the browser doesn't show a big increase. However while testing Cobalt I don't have the possibility to check JS heap size. Instead I have looked at the memory used by the cobalt process. To do similar with Chromium (Ubuntu) I started two instances of Chromium. One running with the fix and one without the fix. Monitor the VmRSS value like this: So now I'm starting to think that chromium also suffers from this problem. // To be continued (at low speed) |
|
@MichaelSweden do you have any update? Thanks! |
|
This is curious because static htmlUnescape(input) {
// Used to map HTML entities to characters.
const htmlUnescapes = {
'&': '&',
'<': '<',
'>': '>',
'"': '"',
''': '\'',
''': '\'',
' ': '\u{a0}',
'‎': '\u{200e}',
'‏': '\u{200f}',
};
// Used to match HTML entities and HTML characters.
const reEscapedHtml = /&(?:amp|lt|gt|quot|apos|#(0+)?39|nbsp|lrm|rlm);/g;
const reHasEscapedHtml = RegExp(reEscapedHtml.source);
// This check is an optimization, since replace always makes a copy
if (input && reHasEscapedHtml.test(input)) {
return input.replace(reEscapedHtml, (entity) => {
// The only thing that might not match the dictionary above is the
// single quote, which can be matched by many strings in the regex, but
// only has a single entry in the dictionary.
return htmlUnescapes[entity] || '\'';
});
}
return input || '';
}So, this should not be an issue if the |
|
I have seen that part. Our problematic stream don't have any that will be unescaped. Did this test: --- a/lib/util/string_utils.js
+++ b/lib/util/string_utils.js
@@ -300,9 +300,9 @@ shaka.util.StringUtils = class {
// Used to match HTML entities and HTML characters.
const reEscapedHtml = /&(?:amp|lt|gt|quot|apos|#(0+)?39|nbsp|lrm|rlm);/g;
- const reHasEscapedHtml = RegExp(reEscapedHtml.source);
+ // const reHasEscapedHtml = RegExp(reEscapedHtml.source);
// This check is an optimization, since replace always makes a copy
- if (input && reHasEscapedHtml.test(input)) {
+ if (input/* && reHasEscapedHtml.test(input)*/) {
return input.replace(reEscapedHtml, (entity) => {
// The only thing that might not match the dictionary above is the
// single quote, which can be matched by many strings in the regex, butMemory still grow. I guess that "since replace always makes a copy" might not be true (as for some other javascript functions that is specified like that). I have traced the string in question and have these notes: Let do some more testing. We will take help by this new function: --- a/lib/util/string_utils.js
+++ b/lib/util/string_utils.js
@@ -312,6 +312,22 @@ shaka.util.StringUtils = class {
}
return input || '';
}
+
+ /**
+ * Creates a copy of a string.
+ * The new returned string do not have any reference to the source string.
+ *
+ * @param {?string} str
+ * @return {string}
+ * @export
+ */
+ static createNewString(str) {
+ if (!str || str.length < 1) {
+ return '';
+ }
+ const newString = '_' + str;
+ return newString.substring(1);
+ }
};
Let us test with this: --- a/lib/dash/segment_template.js
+++ b/lib/dash/segment_template.js
@@ -201,8 +201,8 @@ shaka.dash.SegmentTemplate = class {
const segmentInfo =
MpdUtils.parseSegmentInfo(context, SegmentTemplate.fromInheritance_);
- const media = MpdUtils.inheritAttribute(
- context, SegmentTemplate.fromInheritance_, 'media');
+ const media = StringUtils.createNewString(MpdUtils.inheritAttribute(
+ context, SegmentTemplate.fromInheritance_, 'media'));
const index = MpdUtils.inheritAttribute(
context, SegmentTemplate.fromInheritance_, 'index');
No huge memory grow. I will do more testing. I have had very much to do last week and also this week. But I will be back with more information. |
|
I also tested this: --- a/lib/dash/dash_parser.js
+++ b/lib/dash/dash_parser.js
@@ -1891,6 +1891,12 @@ shaka.dash.DashParser = class {
const segmentBase = TXml.findChild(elem, 'SegmentBase');
const segmentTemplate = TXml.findChild(elem, 'SegmentTemplate');
+ if (segmentTemplate && segmentTemplate.attributes) {
+ if (segmentTemplate.attributes['media']) {
+ segmentTemplate.attributes['media'] = shaka.util.StringUtils
+ .createNewString(segmentTemplate.attributes['media']);
+ }
+ }
// The availabilityTimeOffset is the sum of all @availabilityTimeOffset
// values that apply to the adaptation set, via BaseURL, SegmentBase,and memory usage will not grow huge. I have some ideas about what going on, but need to do more tests, investigate and think it through. |
|
This will not fix growing memory: --- a/lib/dash/segment_template.js
+++ b/lib/dash/segment_template.js
@@ -54,6 +54,10 @@ shaka.dash.SegmentTemplate = class {
/** @type {shaka.dash.SegmentTemplate.SegmentTemplateInfo} */
const info = SegmentTemplate.parseSegmentTemplateInfo_(context);
+ if (info.mediaTemplate) {
+ info.mediaTemplate =
+ shaka.util.ObjectUtils.cloneObject(info.mediaTemplate);
+ }
SegmentTemplate.checkSegmentTemplateInfo_(context, info);
But this... --- a/lib/dash/segment_template.js
+++ b/lib/dash/segment_template.js
@@ -54,6 +54,10 @@ shaka.dash.SegmentTemplate = class {
/** @type {shaka.dash.SegmentTemplate.SegmentTemplateInfo} */
const info = SegmentTemplate.parseSegmentTemplateInfo_(context);
+ if (info.mediaTemplate) {
+ info.mediaTemplate =
+ shaka.util.StringUtils.createNewString(info.mediaTemplate);
+ }
SegmentTemplate.checkSegmentTemplateInfo_(context, info);
will not grow memory (i.e. cloneObject vs createNewString). |
|
I have done some investigation about the memory (heap) content to see if I can verify what I think going on. I have played the stream one hour with 3 different variants of Shaka player. Let us call them: MainZero :: MainFix1 :: MainFix2 (I will use this syntax with double colon to present all 3 results) MainFix1: --- a/lib/dash/segment_template.js
+++ b/lib/dash/segment_template.js
@@ -54,6 +54,10 @@ shaka.dash.SegmentTemplate = class {
/** @type {shaka.dash.SegmentTemplate.SegmentTemplateInfo} */
const info = SegmentTemplate.parseSegmentTemplateInfo_(context);
+ if (info.mediaTemplate) {
+ info.mediaTemplate =
+ shaka.util.StringUtils.createNewString(info.mediaTemplate);
+ }
SegmentTemplate.checkSegmentTemplateInfo_(context, info);
--- a/lib/util/string_utils.js
+++ b/lib/util/string_utils.js
@@ -312,6 +312,22 @@ shaka.util.StringUtils = class {
}
return input || '';
}
+
+ /**
+ * Creates a copy of a string.
+ * The new returned string do not have any reference to the source string.
+ *
+ * @param {?string} str
+ * @return {string}
+ * @export
+ */
+ static createNewString(str) {
+ if (!str || str.length < 1) {
+ return '';
+ }
+ const newString = '_' + str;
+ return newString.substring(1);
+ }
};
MainFix2 is the above patch, plus this: --- a/lib/dash/segment_template.js
+++ b/lib/dash/segment_template.js
@@ -714,7 +718,9 @@ shaka.dash.TimelineSegmentIndex = class extends shaka.media.SegmentIndex {
} else {
const currentTimeline = this.templateInfo_.timeline;
- this.templateInfo_.mediaTemplate = info.mediaTemplate;
+ if (this.templateInfo_.mediaTemplate !== info.mediaTemplate) {
+ this.templateInfo_.mediaTemplate = info.mediaTemplate;
+ }
// Append timeline
const lastCurrentEntry = currentTimeline[currentTimeline.length - 1];After run for one hour I use the "kill -s SIGSEGV " command to terminate the browser process (Cobalt) and a core dump file being created. I didn't find out how to dump the heap in gdb. Instead inspect all the core dump file. The core dump file sizes are 478 :: 357 :: 360 MB. The memory usage at kill was 198 :: 75 :: 60 MB (approximately, they goes up and down). The "media" template is 112 characters long in our manifest and exist 5 times in each manifest. First I searched for the media template string. Then adding little bit more in front of this string, the way it is written in manifest. The matches of this search will actually be all manifest. Please note that these hits are not all true. It can be deleted objects left in memory (garbage), but that is not important for the result of this test. Third I searched for the template string with a underscore in front of it. This will hit strings created by my new createNewString function. Sum the second and third row will give value on first row, for each column. Keep in mind that these numbers are very approximately, since some of them are garbage (I can see it by looking in the files). The result show how javascript engine handle these two rows in my new function: So the xml parsing part and levels above that will have references to the big manifest string. This causes the row "this.templateInfo_.mediaTemplate = info.mediaTemplate;" to hold a reference to the big manifest. In other words: String objects that have their origin of a bigger string, will hold a reference to the big string. This is in contrast to having its own new buffer with only the short string in it. |
|
I realized the same when I did a heap capture on Chrome. There are This shouldn't be a problem since, in general, we should trust in the optimization the JS engine does. The problem here is probably that we are keeping references to old strings, not allowing the GC to collect them (and in consequence the long strings are also kept) In particular, the --- a/lib/dash/segment_template.js
+++ b/lib/dash/segment_template.js
@@ -917,7 +917,6 @@ shaka.dash.TimelineSegmentIndex = class extends shaka.media.SegmentIndex {
let ref = this.references[correctedPosition];
if (!ref) {
- const mediaTemplate = this.templateInfo_.mediaTemplate;
const range = this.templateInfo_.timeline[correctedPosition];
const segmentReplacement = range.segmentPosition;
const timeReplacement = this.templateInfo_
@@ -990,9 +989,12 @@ shaka.dash.TimelineSegmentIndex = class extends shaka.media.SegmentIndex {
if (range.partialSegments > 0) {
return [];
}
+ if (!this.templateInfo_) {
+ return [];
+ }
return shaka.dash.TimelineSegmentIndex
.createUris_(
- mediaTemplate,
+ this.templateInfo_.mediaTemplate,
this.representationId_,
segmentReplacement,
this.bandwidth_, |
|
Great proposal, it seems to fix it :-) All tests take time, I will be back tomorrow with more details. |
|
Here are result from previous test of 3 variants and now added 4 more. The core dump file sizes are 478 :: 357 :: 360 ::: 356 :: 364 :: 358 :: 360 MB. We can see that the new get-patch lower the amount of manifest in memory. It might be a good idea to redo this kind of test several times. Also run for longer time. |
|
I have executed more of the last test. All variants contains the get-patch. I expected to see no difference in CA3 compare to CA0. However the 2 hour runs show some, but probably just a coincidence. I think this memory graph for the 2 hours run don't say any valuable to us. |
avelad
added
component: DASH
|
Here are result from 8 hours runs. Should have been 4 runs for each shaka variant, but got problem with the stream at end. It might have been problem with stream some times during test. The result have some indication of that. Three eight hours run for each: We can compare the CA0 and CA3, with the earlier four hours run: I guess we can conclude that it doesn't grow more at 8 compare with 4 hours run. I guess that all variants will have the same amount of manifests in memory. Please remember that the way I inspect memory content is very unreliable. If we look at the browser process memory usage in Linux. I believe we cannot make any conclusion from it. All CA0 is red (4), CA1 green (4), CA2 blue (3), CA3 magenta (3). PS. I will come back with more info. |
|
In version 4.10.6 we have introduced some improvements, can you validate if it is enough or do we need more improvements? Thanks! |
avelad
added
the
status: waiting on response
|
I have done some test runs. Four variants and three times of each. Every run was one hour long. Code of two versions of source code, the v4.10.6 release and main branch from some days ago (before the effective commit). This issue is about memory usage, so let us take a look of browser memory usage. Red: v4.10.6 Let us also check the search in memory I done earlier. After mediatemplate string and manifest. During one hour run approximate 1800 manifests would have been downloaded and parsed. Each contaning four instances of the mediatemplate string. In v4.10.6 this commit is included: c1480c7 fix(DASH): Improve memory usage with live streams (#7039) I consider this issue now being solved by #7039 and can be closed. Thanks! |
shaka-bot
removed
the
status: waiting on response
Have you read the FAQ and checked for duplicate open issues?
Yes
If the problem is related to FairPlay, have you read the tutorial?
No
What version of Shaka Player are you using?
v4.8.3
Can you reproduce the issue with our latest release version?
Yes
Can you reproduce the issue with the latest code from
main?Yes
Are you using the demo app or your own custom app?
Own
If custom app, can you reproduce the issue using our demo app?
Not done, but tested with a minimal html page and default shaka config.
What browser and OS are you using?
Cobalt browser, Linux
For embedded devices (smart TVs, etc.), what model and firmware version are you using?
N/A
What are the manifest and license server URIs?
Sorry, have not found any free public streams showing this problem.
What configuration are you using? What is the output of
player.getConfiguration()?Not relevant, since problem also occurs with default config.
What did you do?
Upgrade shaka from v4.5.0 to v4.8.3 and playing a dash live stream.
What did you expect to happen?
Be able to play for long time.
What actually happened?
After a few hours memory usage have grown very much, several hundred megabytes. Later in our platform oom-killer kicks in and terminate the browser process.
Are you planning send a PR to fix it?
Yes
The text was updated successfully, but these errors were encountered: