1163958 - Reduce the allocation in MediaStreamGraph

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[Andrea Marchesini [:baku]]

Attachment: e.patch

Often we create temporary arrays to store part of the graph.
We should try to avoid this because that can cause allocation.

Comment 1

[Andrea Marchesini [:baku]]

This first patch contains also some indentation things. If you prefer I can split it in another patch.

Comment 2

[Andrea Marchesini [:baku]]

Attachment: f.patch

This patch manly introduces StreamBuffer::GetAndResetTracksDirty() and that is used to do or not to do all the MediaStreamGraphImpl::CreateOrDestroyAudio logic.

Comment 3

[Andrea Marchesini [:baku]]

Attachment: f.patch

Comment 4

[Andrea Marchesini [:baku]]

Attachment: f.patch

This patch reverts some changes done in previous patches.
The new logic was not fully correct. (but the tests pass...)

Comment 5

[Andrea Marchesini [:baku]]

Attachment: e.patch

Ah.. my fault. The previous comment was about this patch. f.patch is ok.

Comment 6

[Paul Adenot (:padenot)]

Comment on attachment 8604707 [details] [diff] [review]
f.patch

Review of attachment 8604707 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/StreamBuffer.cpp
@@ +82,5 @@
> +
> +    if (mTracks[middle]->GetID() > aID) {
> +      right = middle - 1;
> +    } else {
> +      left = middle + 1;

Maybe we can simply use https://dxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTArray.h?from=nsTArray.h#1146 since mTracks is an nsTArray ?

Comment 7

[Andrea Marchesini [:baku]]

> Maybe we can simply use
> https://dxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTArray.
> h?from=nsTArray.h#1146 since mTracks is an nsTArray ?

That returns the offset of a known item. I didn't find a binary search implementation for nsTArray that matches this case.

Comment 8

[Andrea Marchesini [:baku]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/7353d8b80065
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/f0fdc3d257f5

Comment 9

[Andrea Marchesini [:baku]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fb80461d81fb

Comment 10

[Nathan Froyd [:froydnj]]

(In reply to Andrea Marchesini (:baku) from comment #7)
> > Maybe we can simply use
> > https://dxr.mozilla.org/mozilla-central/source/xpcom/glue/nsTArray.
> > h?from=nsTArray.h#1146 since mTracks is an nsTArray ?
> 
> That returns the offset of a known item. I didn't find a binary search
> implementation for nsTArray that matches this case.

I'm pretty sure you can use the nsTArray version, but even if you couldn't, you could at least use the underlying implementation it's based on:

http://mxr.mozilla.org/mozilla-central/source/mfbt/BinarySearch.h#45

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7353d8b80065
https://hg.mozilla.org/mozilla-central/rev/f0fdc3d257f5
https://hg.mozilla.org/mozilla-central/rev/fb80461d81fb