Closed Bug 1198656 Opened 9 years ago Closed 9 years ago

Avoid sometimes unnecessary initial AudioBuffer allocations

Categories

(Core :: Web Audio, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: karlt, Assigned: karlt)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(5 files)

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: nobody → karlt
Status: NEW → ASSIGNED
Depends on: 1199559
Keywords: perf
Depends on: 1199560
Depends on: 1199561
No longer depends on: 1199560
Depends on: 1201855
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
bug 1198656 remove unnecessary reinterpret_casts r?padenot JS_StealArrayBufferContents() returns void*.
Attachment #8657093 - Flags: review?(padenot)
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)
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)
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)
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)
Attachment #8657093 - Flags: review?(padenot) → review+
Comment on attachment 8657093 [details] MozReview Request: bug 1198656 remove unnecessary reinterpret_casts r?padenot https://reviewboard.mozilla.org/r/18303/#review16495
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 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 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 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+
Could add a test for ConvolverNode output with null buffer.
Flags: in-testsuite?
Blocks: 937957
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: