1530347 - devirtualize *Constructor methods

Status: NEW

(Core :: IPC, enhancement, P3)

Description

[Nathan Froyd [:froydnj]]

We currently unconditionally declare *Constructor methods as virtual:

https://searchfox.org/mozilla-central/rev/dc0adc07db3df9431a0876156f50c65d580010cb/ipc/ipdl/ipdl/lower.py#3144-3157

because we given them a default do-nothing implementation. This is convenient for a lot of classes, but it gets in the way of doing efficient things with the arguments to the *Constructor methods.

We should devirtualize these methods, just as we devirtualized Recv* methods. This might require adding a lot of code to fill in all those defaulted Constructor methods on classes, which would be somewhat tedious and boilerplate-y. Alternatively, we could add magic to the generated IPDL code to check for the existence of a RecvConstructor method and call it only if it exists. But that might be Too Much C++.

Comment 1

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

Can't the PFooSide superclasses for devirtualized actors just define empty methods that are non-virtually overridden if needed, and otherwise inherited?

Comment 2

[Nathan Froyd [:froydnj]]

(In reply to Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ from comment #1)

Can't the PFooSide superclasses for devirtualized actors just define empty methods that are non-virtually overridden if needed, and otherwise inherited?

Is that equivalent to just "remove the virtual keyword on those methods"? Then yes, I think so. Although "overriding" those methods also gets you into the vagaries of C++ overloading, and it seems possibly to subtly shoot yourself in the foot if the overloading algorithm doesn't turn out how you want and/or mistakes are made in declaring the argument list for the "overriding" functions.

Comment 3

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

…that was not an entirely well thought out comment on my part. Yes, non-virtually “overriding” with different argument types (which is most of the point of devirtualization) would be a footgun. I also wonder if template magic might have similar problems with not seeing a method when it exists.

I wonder if there's a simple way to find out how many of the constructor callbacks are overridden: if most of them are overridden it might make sense to just add in the empty implementations, or if overrides are uncommon then maybe we could make them opt-in via the extended attributes we don't have yet.

Comment 4

[Alex Gaynor [:Alex_Gaynor]]

Easiest way to compute how many Constructor methods are override vs. not might be to use semmle. If no one else is excited about doing that, I can take on doing that research.