To enable string sharing, we're going to have helpful functions that
take a small, distinguishable, sharable string and construct a more
complete error message out of that. To do that easily with checkedRead,
we need to be able to pass custom parameters into the error function.
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.
checkedRead is set up to single-quote whatever message is passed in.
This scheme works great for all existing messages, but it makes some
callsites a little surprising ("where's the matching quote?") and
doesn't work well with message changes to be made in future patches.
Let's move the quoting out to client code.
Similar to part 1, this change enables the strings passed to
ProtocolErrorBreakpoint to be collapsed into a single string, saving
~60K of read-only data (!). This change does affect debuggability
slightly, but given that ProtocolErrorBreakpoint only tries to throw the
passed-in string to stderr, I don't think it's a huge deal.
We have better ways of getting the protocol name at the point of the
error (e.g. backtraces). Removing it means the error message can be
condensed to a single string by the compiler/linking, saving ~8k of
read-only data.
The original changeset that is being backed out had comment:
Bug 1023941 - Part 5: Loader hook to redirect the missing import.
The changes made in bug 1023941 were to work around the fact that with VS2013, msvcr120.dll imports kernel32!GetLogicalProcessorInformation, which is not available on Windows XP SP2.
In VS2015, the GetLogicalProcessorInformation requirement has moved into concrt140.dll (concurrency runtime), which we don't use.
So, now that our build infra is building with VS2015, we can remove the hooking and static runtime linking required to get the VS2013 fix to work.
In addition we need to do that to be able us to link the Chromium sandbox code into firefox.exe and get it to build and run with both VS2015 and VS2013.
MozReview-Commit-ID: 1tlXaYJ8dHH
--HG--
extra : rebase_source : 49b216e34fc5c77af96df1f67ee44c46d5368bfe
Specifically, it's important not to try to use Move() in the
Alloc+SendConstructor convenience method, because that Move()s the same
value twice (as discovered in bug 1186706), and neither callee
requires rvalue references in that case anyway.
This also removes the previous feature of calling makeCxxArgs with
params=0 to omit the message's input parameters, because it's not being
used and doesn't appear to have ever been used; it could be restored
(e.g., as paramsems=None) if needed.
It's an annotation that is used a lot, and should be used even more, so a
shorter name is better.
MozReview-Commit-ID: 1VS4Dney4WX
--HG--
extra : rebase_source : b26919c1b0fcb32e5339adeef5be5becae6032cf
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.
NS_RUNTIMEABORT generates rather a lot of code, since we have to load up
five arguments, two of which are strings, and then call NS_DebugBreak.
Instead of doing this, let's just call each protocol's FatalError, which
only requires loading one string argument. Each protocol's FatalError
calls mozilla::ipc::FatalError, which communicates exactly the same
information as the inlined NS_RUNTIMEABORT would (e.g. crash reporter
annotations), but at a significant code savings: this patch reduces
.text by ~80K on Linux x86-64.
In bug 1259428, we changes a bunch of things that were previously
classes to refer to functions instead. Jed suggested in bug 1259428
comment 10 that we might want to rename things to reflect the new world
order as a followup. Consider it done.
All we use our Message subclasses for nowadays is the constructor, so we
might as well strip the class away and just have functions that perform
the construction for us. This change eliminates unnecessary vtables as
well as making the included headers somewhat smaller, which is always
nice.
Various bits of IPDL code would do something like:
Message* m = ...;
if (m.type() == particular message type) {
static_cast<ParticularMessageType*>(m)->name();
}
The static_cast is a remnant of having to do the downcast to access the
Log() method on the concrete subclass. Since name() is defined on
Message, there's no need for these casts anymore, so let's remove them.
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.
IPDL processing takes ~9.4s on my i7-6700K and is the long pole in the
"export" tier for clobber --disable-compile-environment builds. IPDL
shouldn't be needed for these builds.
Disabling IPDL makes artifact builds ~4s faster on my machine.
--HG--
extra : rebase_source : 60aeec636e18380c3258f054e90d1b6c3a16fadf
