For every protocol's RemoveManagee method, and every sub-protocol that
protocol manages, we generate:
MOZ_DIAGNOSTIC_ASSERT(mManagedPSubProtcolChild.Contains(actor), "...");
which dumps strings into the binary like:
(mManagedPAsmJSCacheEntryChild).Contains(actor) (actor not managed by this!)
MOZ_RELEASE_ASSERT((mManagedPAsmJSCacheEntryChild).Contains(actor)) (actor not managed by this!)
The linker is capable of merging multiple strings together, but
including the sub-protocol in every assert expression effectively
defeats this linker optimization, resulting in ~40KB of unnecessary
strings.
We can improve this situation by taking a reference to the managee
container, and using that reference in the assertion expression. All
the assertion expressions are identical, and the linker can perform the
expected string merging, for a savings of ~40KB.
Most calls to MaybeDestroy() have their return value checked, but those that
take a T__None argument legitimately don't. Coverity gets upset by this, so
let's cast those calls to void to make Coverity happier.
--HG--
extra : rebase_source : 798c852371a682372c61cae953cc13c160b9ff9a
The IPDL compiler generates code like this in DestroySubtree methods:
// Recursively shutting down PAPZ kids
nsTArray<PAPZChild*> kids((mManagedPAPZChild).Count());
// Accumulate kids into a stable structure to iterate over
ManagedPAPZChild(kids);
for (uint32_t i = 0; (i) < ((kids).Length()); (++(i))) {
// Guarding against a child removing a sibling from the list during the iteration.
if ((mManagedPAPZChild).Contains(kids[i])) {
(kids[i])->DestroySubtree(subtreewhy);
}
}
There are three problems with this code:
1) We initialize |kids| with a capacity, which is wasted work;
ManagedPAPZChild() is going to resize the array for us anyway.
2) We're constantly reading from |kids.Length()| in the loop, when we
only really need to read from it once.
3) We're repeatedly accessing kids[i], and doing needless bounds checks.
Let's fix those three problems.
For better or worse, the IPDL compiler, for protocols that send or
receive arrays, generates Write methods for transferring those arrays,
rather than going through the standard ParamTraits specializations.
These generated methods perform unnecessary bounds checks when accessing
elements; such checks can be safely bypassed because we know the exact
length of all the arrays involved. (Some compilers are not smart enough
to eliminate the bounds checks on their own.)
For better or worse, the IPDL compiler, for protocols that send or
receive arrays, generates Read methods for transferring those arrays,
rather than going through the standard ParamTraits specializations.
These generated methods perform unnecessary bounds checks when accessing
elements; such checks can be safely bypassed because we know the exact
length of all the arrays involved. (Some compilers are not smart enough
to eliminate the bounds checks on their own.)
This addition makes many of the upcoming patches significantly easier to
write, and enables us to avoid unpleasantness trying to fiddle with
ipdl.py's notions of C++ types. (For instance, there's no good way,
when you have a type in-hand, to say the moral equivalent of
std::add_pointer<T>::type.)
The only thing we needed opened actor tracking for was the ability to
clone all the actors. But now that we no longer have support for
cloning actors, we no longer need to track the actors that we've cloned,
which makes a number of things significantly simpler.
CloneOpenedToplevels, which is never called, is the only interesting
caller of CloneToplevel. And CloneToplevel, in turn, is the only
interesting caller of CloneManagees. Which means we can ditch all this
code for a decent amount of space savings, both in code and writable
static data (no more useless virtual function entries in vtables).
The standard placement new function requires a null check, implicitly
generated by the compiler, on its returned value. For places we know
don't deal with null pointers, such as all the generated methods of IPDL
code, we can use an overloaded operator new that doesn't require the
null check.
We construct Trigger structures for every IPDL send and recv to run
the (currently defunct) IPDL state machine for each protocol. These
structures are 64 bits to hold an enum that could be represented in 1
bit and an IPDL message enum that can easily be represented in 31 bits.
Therefore, we can make the Trigger structure smaller by storing the
members as bitfields, rather than full-width integers. (The message
enums are formed from a 16-bit protocol enum and a 16-bit
protocol-specific message enum; since the protocol enum goes in the
upper bits, we'd need 32768 protocols to overflow the bitfield we're
using in Trigger...a number that we're not going to hit anytime soon.)
This change saves ~15K of space on x86-64 Linux.
The calls to IPDL Transition() functions consistently look like:
Transition(VAR, ..., &VAR);
We can save space by only passing &VAR and deriving the state we're
coming from by loading VAR in Transition itself. It's not great using
inout parameters here, but we call Transition enough times that this
change saves a reasonable amount of space: about 10K on x86-64 Linux.
The unsightliness of inout parameters here is lessened by the only
callers of Transition being generated code.
There's no reason to generate identical code for every kind of managed
protocol; we can have a single instance that operates on void* and cast
our way to victory. This change saves ~60K of text on x86-64 Linux.
The patch is generated from following command:
rgrep -l unused.h|xargs sed -i -e s,mozilla/unused.h,mozilla/Unused.h,
MozReview-Commit-ID: AtLcWApZfES
--HG--
rename : mfbt/unused.h => mfbt/Unused.h
This removes the unnecessary setting of c-basic-offset from all
python-mode files.
This was automatically generated using
perl -pi -e 's/; *c-basic-offset: *[0-9]+//'
... on the affected files.
The bulk of these files are moz.build files but there a few others as
well.
MozReview-Commit-ID: 2pPf3DEiZqx
--HG--
extra : rebase_source : 0a7dcac80b924174a2c429b093791148ea6ac204
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
Bill McCloskey
Nathan Froyd
Matt Woodrow
Aaron Klotz
Nicholas Nethercote
Nathan Froyd
Kan-Ru Chen
David Anderson
Tom Tromey
Gabor Krizsanits
Cervantes Yu
Jonathan Watt
Bob Owen
Chris Peterson
Jed Davis
Kyle Huey
Andrew McCreight
Andrea Marchesini