Closed Bug 1487994 Opened 2 years ago Closed 2 years ago

webdriver: Avoid unnecessary passing by value

Categories

(Testing :: geckodriver, enhancement)

enhancement
Not set

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: ato, Assigned: ato)

Details

Attachments

(1 file)

In some places we pass variables by value unnecessarily.  In these
cases it would be more memory efficient to pass a reference so that
we avoid copying.
Assignee: nobody → ato
Status: NEW → ASSIGNED
In some places we pass variables by value unnecessarily.  In these
cases it would be more memory efficient to pass a reference so that
we avoid copying.
Attachment #9005825 - Flags: review?(hskupin)
Pushed by ato@sny.no:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2f8b8c24ed1
webdriver: avoid unnecessary passing by value; r=whimboo
https://hg.mozilla.org/mozilla-central/rev/d2f8b8c24ed1
https://hg.mozilla.org/mozilla-central/rev/47f0ff650a27
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
FWIW I don't think this is a striaghtforward win. In general passing T vs &T should be decided based on semantics unless there's a performance issue i.e. if you are giving up ownership of the variable you should pass T, if you are lending it out you should pass &T. In terms of performance there are several considerations

* Pointer chasing has a cost, so dereferencing an &T isn't free.

* If size_of<T>() is less than the size of a pointer, passing a T is cheaper than passing an &T.

* LLVM can often optimise cases where the argument is moved into the callee and elide the copy.

So in general I would be wary of this kind of change unless there's specific profiling indicating a problem, or the semantics of the code seem wrong.
clippy warns about cases where something is being copied where it
can feasibly be borrowed, but I sympathise with deciding ownership
on semantic grounds.  Maybe some of the cases could have been more
elegantly solved by deriving Copy, e.g. Method and Route.

Dereferencing a pointer does not come at zero cost, but neither
does memory allocation or copying.  It is true that—perhaps with
the exception of msg_recv—there’s very little overhead involved
either way on a modern computer, and that LLVM in all likelihood
optimises the code either way.  That’s really very hard to tell
without looking at the IR or the instruction set.
You need to log in before you can comment on or make changes to this bug.