Closed
Bug 1198656
Opened 9 years ago
Closed 9 years ago
Avoid sometimes unnecessary initial AudioBuffer allocations
Categories
(Core :: Web Audio, defect)
Core
Web Audio
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: karlt, Assigned: karlt)
References
(Blocks 1 open bug)
Details
(Keywords: perf)
Attachments
(5 files)
40 bytes,
text/x-review-board-request
|
padenot
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
padenot
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
padenot
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
padenot
:
review+
|
Details |
40 bytes,
text/x-review-board-request
|
padenot
:
review+
|
Details |
These changes save 50% on the "Empty testcase" benchmark when increasing
buffer period to 2000s for semi-repeatable results.
The changes appear to remove most of the variability in this test's results.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=304a78625ab2
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → karlt
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•9 years ago
|
||
Bug 1199559 tracked the buffer copy removal in the changes in comment 0.
Summary: Avoid unnecessary AudioBuffer allocations and buffer copies → Avoid sometimes unnecessary initial AudioBuffer allocations
Assignee | ||
Comment 2•9 years ago
|
||
bug 1198656 remove unnecessary reinterpret_casts r?padenot
JS_StealArrayBufferContents() returns void*.
Attachment #8657093 -
Flags: review?(padenot)
Assignee | ||
Comment 3•9 years ago
|
||
bug 1198656 interpret null ConvolverNode mBuffer as a buffer of zeros r?padenot
Zero output even when normalize is set is consistent with
http://webaudio.github.io/web-audio-api/#widl-ConvolverNode-normalize
due to the specified isinf() test.
Attachment #8657094 -
Flags: review?(padenot)
Assignee | ||
Comment 4•9 years ago
|
||
bug 1198656 refactor acquiring the content into an object method r?
This makes it clearer that the algorithm is intentionally aborted when any of
the buffers have been neutered and that the stolen data has the correct
length.
It also makes mJSChannels available for clearing in a subsequent changeset.
Attachment #8657095 -
Flags: review?(padenot)
Assignee | ||
Comment 5•9 years ago
|
||
bug 1198656 clear references in mJSChannels on successful content acquire r?
The array buffers are no longer available and mJSChannels will be overwritten
in RestoreJSChannelData() before it is used again. This is consistent with
"Attach ArrayBuffers containing copies of the data to the AudioBuffer, to be
returned by the next call to getChannelData."
Attachment #8657096 -
Flags: review?(padenot)
Assignee | ||
Comment 6•9 years ago
|
||
bug 1198656 delay AudioBuffer allocation until required r?padenot
This saves an allocation and zeroing for buffers generated by AudioNodes and
avoids allocation altogether for empty buffers.
Incidentally, RestoreJSChannelData() now avoids unnecessary recreation of
Float32Arrays if they already exist after a previous call failed.
Attachment #8657097 -
Flags: review?(padenot)
Updated•9 years ago
|
Attachment #8657093 -
Flags: review?(padenot) → review+
Comment 7•9 years ago
|
||
Comment on attachment 8657093 [details]
MozReview Request: bug 1198656 remove unnecessary reinterpret_casts r?padenot
https://reviewboard.mozilla.org/r/18303/#review16495
Comment 8•9 years ago
|
||
Comment on attachment 8657094 [details]
MozReview Request: bug 1198656 interpret null ConvolverNode mBuffer as a buffer of zeros r?padenot
https://reviewboard.mozilla.org/r/18305/#review16497
Attachment #8657094 -
Flags: review?(padenot) → review+
Comment 9•9 years ago
|
||
Comment on attachment 8657095 [details]
MozReview Request: bug 1198656 refactor acquiring the content into an object method r?
https://reviewboard.mozilla.org/r/18307/#review16499
Attachment #8657095 -
Flags: review?(padenot) → review+
Comment 10•9 years ago
|
||
Comment on attachment 8657096 [details]
MozReview Request: bug 1198656 clear references in mJSChannels on successful content acquire r?
https://reviewboard.mozilla.org/r/18309/#review16501
Attachment #8657096 -
Flags: review?(padenot) → review+
Comment 11•9 years ago
|
||
Comment on attachment 8657097 [details]
MozReview Request: bug 1198656 delay AudioBuffer allocation until required r?padenot
https://reviewboard.mozilla.org/r/18311/#review16503
Attachment #8657097 -
Flags: review?(padenot) → review+
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b9a5c8a0a4a
https://hg.mozilla.org/integration/mozilla-inbound/rev/b27cf1d88807
https://hg.mozilla.org/integration/mozilla-inbound/rev/5376e4042cf3
https://hg.mozilla.org/integration/mozilla-inbound/rev/d325ca7fa94c
https://hg.mozilla.org/integration/mozilla-inbound/rev/e4fb01e4e3cd
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2b9a5c8a0a4a
https://hg.mozilla.org/mozilla-central/rev/b27cf1d88807
https://hg.mozilla.org/mozilla-central/rev/5376e4042cf3
https://hg.mozilla.org/mozilla-central/rev/d325ca7fa94c
https://hg.mozilla.org/mozilla-central/rev/e4fb01e4e3cd
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Assignee | ||
Comment 14•9 years ago
|
||
Could add a test for ConvolverNode output with null buffer.
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•