852011 - Reserve 5 elements in TimeVarying::mChanges in order to avoid excessive cost when dealing with removals from it

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Description

[(no longer active)]

Attachment: Patch (v1)

In my testcase, we had at most 3 members in mChanges, but it's small enough that 5 should be a good guesstimate-based staring value.  This eliminates about half of the nsTArray badness I was seeing.

Comment 1

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 725985 [details] [diff] [review]
Patch (v1)

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

::: content/media/TimeVarying.h
@@ +213,5 @@
>      // The time at which the value changes to mValue
>      Time mTime;
>      T mValue;
>    };
> +  nsAutoTArray<Entry, 5> mChanges;

I'd prefer this parameter to be exposed as a parameter to the TimeVarying template so we can vary it by user.

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 725985 [details] [diff] [review]
Patch (v1)

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

I guess I meant -

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Please figure out which TimeVarying values actually need auto elements, and how many, and then attach patches to set those values here. Thanks.

Comment 4

[(no longer active)]

It seems like most of the badness comes from the mBlocked member, we don't need to reserve storage for other TimeVarying's.

Comment 5

[(no longer active)]

Attachment: Patch (v2)

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 726169 [details] [diff] [review]
Patch (v2)

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

OK. The fact that we have entries in the mBlocking array indicates that we must be stuttering, which is really bad, so we need to fix it. Once we've fixed it we can probably put this back to zero.

Comment 7

[(no longer active)]

Yes, we are indeed stuttering...

Comment 8

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2cc397611921

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2cc397611921

Comment 10

[(no longer active)]

Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.