[CID 1372416] Inefficient pass-by-value: HTMLMediaElement::SetMediaInfo(mozilla::MediaInfo)

RESOLVED FIXED in Firefox 51

Status

()

Core
Audio/Video: Playback
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: gerald, Assigned: gerald)

Tracking

50 Branch
mozilla51
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

a year ago
> *** CID 1372416:  Performance inefficiencies  (PASS_BY_VALUE)
> /dom/html/HTMLMediaElement.cpp: 6544 in
> mozilla::dom::HTMLMediaElement::SetMediaInfo(mozilla::MediaInfo)()
> 6538       }
> 6539     
> 6540       return false;
> 6541     }
> 6542     
> 6543     void
>   CID 1372416:  Performance inefficiencies  (PASS_BY_VALUE)
>   Passing parameter aInfo of type "mozilla::MediaInfo const"
>   (size 472 bytes) by value.
> 6544     HTMLMediaElement::SetMediaInfo(const MediaInfo aInfo)
> 6545     {
> 6546       mMediaInfo = aInfo;
> 6547       AudioCaptureStreamChangeIfNeeded();
> 6548     }

It should be non-controversial to pass by const-ref instead, since the argument is immediately copied into a member variable anyway. This would save one copy.

We might even benefit from move semantics, but that could be a later improvement if needed.
Comment hidden (mozreview-request)

Comment 2

a year ago
mozreview-review
Comment on attachment 8788037 [details]
Bug 1300459 - Pass MediaInfo by const-ref to SetMediaInfo -

https://reviewboard.mozilla.org/r/76584/#review74714
Attachment #8788037 - Flags: review?(jyavenard) → review+

Comment 3

a year ago
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6681da2c7663
Pass MediaInfo by const-ref to SetMediaInfo - r=jya

Comment 4

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6681da2c7663
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
(Assignee)

Comment 5

a year ago
Alastor, you might be interested in this change here, as you first introduced SetMediaInfo in https://hg.mozilla.org/mozilla-central/rev/e7a17f7ee53d for bug 1262053.
Blocks: 1262053
Thanks Gerald!
Duplicate of this bug: 1300500
You need to log in before you can comment on or make changes to this bug.