1191452 - IPDL: code generation inserts unnecessary moves

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Nika Layzell [:nika] (ooo, ni? for response)]

The code generation right now produces tons of unnecessary move calls, even some which conflict with each other. 

Example: https://dxr.allizom.org/mozilla-central/source/obj-x86_64-unknown-linux-gnu/ipc/ipdl/PMobileConnectionChild.cpp#86

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Inspired by the thread on mozilla.dev.platform, I took a look at this, and I have a patch that seems to work.

Comment 2

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: bug1191452-ipdl-move-fix-hg0.diff

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8748969 [details] [diff] [review]
bug1191452-ipdl-move-fix-hg0.diff

Review of attachment 8748969 [details] [diff] [review]:
-----------------------------------------------------------------

::: ipc/ipdl/ipdl/lower.py
@@ +971,4 @@
>              cxxargs.extend([ ExprMove(p.var()) for p in self.params ])
> +        elif paramsems is 'in':
> +            cxxargs.extend([ p.var() for p in self.params ])
> +        else: assert False

Please put this on a new line.

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: Diff of IPDL generated code (don't review).

I made a diff of the IPDL compiler output but forgot to attach it.  Probably not useful at this point, but attaching it in case anyone's interested.

Comment 5

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

(In reply to Bill McCloskey (:billm) from comment #3)
> > +        else: assert False
> 
> Please put this on a new line.

Sure.  (Normally I wouldn't write it like that; I was following the existing code's style.  Although not to the point of using 0 and 1 as booleans like it does.)

Comment 6

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Attachment: bug1191452-ipdl-move-fix-hg1.diff

Carrying over r+.
Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb44a9444ef2
Try run combined with bug 1186706: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ece184e7d5c2

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/be435567c858

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/be435567c858