Closed
Bug 1175621
Opened 10 years ago
Closed 10 years ago
use moves inside WrapRunnable* functions
Categories
(Core :: WebRTC, defect)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla42
Tracking | Status | |
---|---|---|
firefox42 | --- | fixed |
backlog | parking-lot |
People
(Reporter: froydnj, Assigned: froydnj)
References
Details
Attachments
(1 file)
6.10 KB,
patch
|
ekr
:
review+
|
Details | Diff | Splinter Review |
Using moves inside WrapRunnable* when creating the actual runnable has two benefits:
1. We avoid some unnecessary copying of things. (Any necessary copies are already done by the call to WrapRunnable*, assuming I understand C++ semantics.)
2. We correctly handle objects that have move constructors, but don't have copy constructors.
#2 is especially important to me in the context of bug 1162026, since TemporaryRef<T> has a copy constructor, but converting TemporaryRef<T> usages to already_AddRefed<T> causes issues, since already_AddRefed<T> doesn't have a copy constructor.
![]() |
Assignee | |
Comment 1•10 years ago
|
||
Calls to WrapRunnable* copy their arguments already; we don't need to
copy them a second time when constructing the actual runnable. In
addition to making things more efficient, this change also permits calls
to WrapRunnable to correctly handle objects that can only be moved, and
not copied.
Attachment #8623806 -
Flags: review?(ekr)
Updated•10 years ago
|
backlog: --- → parking-lot
![]() |
Assignee | |
Comment 2•10 years ago
|
||
Comment on attachment 8623806 [details] [diff] [review]
make WrapRunnable* more efficient by utilizing moves in wrapper functions
Review of attachment 8623806 [details] [diff] [review]:
-----------------------------------------------------------------
jesup suggested Byron might be a lower-latency reviewer.
Attachment #8623806 -
Flags: review?(ekr) → review?(docfaraday)
Comment 3•10 years ago
|
||
Comment on attachment 8623806 [details] [diff] [review]
make WrapRunnable* more efficient by utilizing moves in wrapper functions
Review of attachment 8623806 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM
Attachment #8623806 -
Flags: review?(docfaraday) → review+
Comment 5•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
status-firefox42:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in
before you can comment on or make changes to this bug.
Description
•