Actor reading from IPC message is codegen'd with a lot of repeated code.
We can improve that by moving the core actor reading code out of
subclasses into IProtocolmanager. While we still need to codegen a bit
of code to cast the read actor to the proper type, the code overall is
smaller. The lone downside is that if we do encounter an error reading
the actor id out of the message, the precision of our crash messages is
reduced somewhat: we no longer have the protocol name doing the reading,
nor do we get crash report annotations, since we can't tell whether
we're in the parent or child process.
IProtocolManager is templated over some listener type. In our IPDL
code, that type is always IProtocol, which is a subclass of
MessageListener. It's also important to note that IProtocol uses
protected inheritance from MessageListener; the generated code takes
advantage of this inheritance structure when it reads actors:
// ChannelListener is typedef'd to MessageListener
// Lookup here is IProtocolManager::Lookup
ChannelListener* listener = Lookup(id);
Lookup returns a pointer to the type over which IProtocolManager is
templated. As mentioned above, that type is always IProtocol. But
thanks to the containing class inheriting from *both* IProtocolManager
and IProtocol, the returned pointer can be silently upcasted to
MessageListener thanks to C++ visibility rules.
It's not clear that this restricted inheritance structure is actually
benefitting anybody, or that the inheritance hierarchy of protocol
classes is the best way to do things. This particular implementation
detail is getting in the way for the next improvement, so let's make the
protected inheritance public instead.
We do this for the same reasons outlined in part 1: calls to
NS_RUNTIMEABORT are rather large and we generate a lot of them (~1000
left after part 1). This patch reduces .text size by ~20K on x86-64
Linux.
We don't do anything with it in terms of error reporting, we pass in 0
in the child process, and if you're in a debugger, presumably you can
figure out the other process's PID yourself.
The first step to eliminating all the generated Message subclasses the
IPDL compiler spits out is to move the functionality of their Log
methods someplace else. In addition to eliminating the need for the Log
methods, this change has the welcome effect of moving a bunch of code
that would be generated hundreds of times into a single place, which
should reduce code size a bit (debug builds only). We don't actually
remove the generation of the Log methods; that change will be done for a
future patch.
The bulk of this commit was generated with a script, executed at the top
level of a typical source code checkout. The only non-machine-generated
part was modifying MFBT's moz.build to reflect the new naming.
CLOSED TREE makes big refactorings like this a piece of cake.
# The main substitution.
find . -name '*.cpp' -o -name '*.cc' -o -name '*.h' -o -name '*.mm' -o -name '*.idl'| \
xargs perl -p -i -e '
s/nsRefPtr\.h/RefPtr\.h/g; # handle includes
s/nsRefPtr ?</RefPtr</g; # handle declarations and variables
'
# Handle a special friend declaration in gfx/layers/AtomicRefCountedWithFinalize.h.
perl -p -i -e 's/::nsRefPtr;/::RefPtr;/' gfx/layers/AtomicRefCountedWithFinalize.h
# Handle nsRefPtr.h itself, a couple places that define constructors
# from nsRefPtr, and code generators specially. We do this here, rather
# than indiscriminantly s/nsRefPtr/RefPtr/, because that would rename
# things like nsRefPtrHashtable.
perl -p -i -e 's/nsRefPtr/RefPtr/g' \
mfbt/nsRefPtr.h \
xpcom/glue/nsCOMPtr.h \
xpcom/base/OwningNonNull.h \
ipc/ipdl/ipdl/lower.py \
ipc/ipdl/ipdl/builtin.py \
dom/bindings/Codegen.py \
python/lldbutils/lldbutils/utils.py
# In our indiscriminate substitution above, we renamed
# nsRefPtrGetterAddRefs, the class behind getter_AddRefs. Fix that up.
find . -name '*.cpp' -o -name '*.h' -o -name '*.idl' | \
xargs perl -p -i -e 's/nsRefPtrGetterAddRefs/RefPtrGetterAddRefs/g'
if [ -d .git ]; then
git mv mfbt/nsRefPtr.h mfbt/RefPtr.h
else
hg mv mfbt/nsRefPtr.h mfbt/RefPtr.h
fi
--HG--
rename : mfbt/nsRefPtr.h => mfbt/RefPtr.h
A lot of existing code has variations on:
if (ManagedPFooChild().Length()) {
...(ManagedPFooChild()[0])...
}
// Do something with nullptr, or some other action.
It's pretty reasonable to repeat this code when the managed protocols
are stored in an array; the code gets much less nice when managed
protocols are stored in a hashtable. Let's write a small utility
function to handle those details for us. Then when we change the
underlying storage, we only need to update this function, rather than a
bunch of callsites.
ProtocolUtils.h is included by all the generated IPDL headers, so
LoneManagedOrNull should be available everywhere the above pattern would
be encountered.
The bulk of this commit was generated by running:
run-clang-tidy.py \
-checks='-*,llvm-namespace-comment' \
-header-filter=^/.../mozilla-central/.* \
-fix
Nathan Froyd
Cervantes Yu
Nathan Froyd
Lee Salzman
Bill McCloskey
Randell Jesup
Nathan Froyd
Birunthan Mohanathas
Jim Mathies
Phil Ringnalda
Andrew McCreight
Michael Layzell
Bob Owen
Bill McCloskey
Bill McCloskey
Ehsan Akhgari
Benoit Jacob
Ryan VanderMeulen
Ted Clancy
Thinker Lee ext:(%2C%20Cervantes%20Yu%20%3Ccyu%40mozilla.com%3E)
David Anderson
David Zbarsky
Chris Jones
Ms2ger
Ben Turner
Gervase Markham