Closed Bug 908208 Opened 6 years ago Closed 6 years ago

slim down FatalError IPC code

Categories

(Core :: IPC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla26

People

(Reporter: froydnj, Unassigned)

Details

Attachments

(3 files)

No description provided.
Declaring a method MOZ_NEVER_INLINE is better than declaring it virtual to avoid inlining
it.  MOZ_NEVER_INLINE is supported by all our major compilers.  This patch just adds the
backend guts.
Attachment #794028 - Flags: review?(bent.mozilla)
Less space required in vtables and a faster calling sequence.
Attachment #794029 - Flags: review?(bent.mozilla)
FatalError is rather large in all the protocol classes; there's no need to duplicate
its code over and over.  This patch moves the guts of the function out-of-line so that
the implementation in the protocol classes becomes a one-liner.  It could be slightly
shorter if OtherProcess() always returns non-zero in the parent; then the ProcessHandle
could serve double duty as the bit for whether we're calling this from the child or the
parent.  Would appreciate guidance on that.

The whole patch series saves about 12K of text on ARM.
Attachment #794030 - Flags: review?(bent.mozilla)
Attachment #794028 - Flags: review?(bent.mozilla) → review+
Attachment #794029 - Flags: review?(bent.mozilla) → review+
Comment on attachment 794030 [details] [diff] [review]
part 3 - move guts of FatalError out-of-line to ProtocolUtils.cpp

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

r=me, thanks!

::: ipc/glue/ProtocolUtils.cpp
@@ +134,5 @@
>  }
>  
> +void
> +FatalError(const char* aProtocolName, const char* aMsg,
> +           ProcessHandle aHandle, bool isParent)

Nit: aIsParent.

@@ +154,5 @@
> +    formattedMessage.AppendLiteral("\". abort()ing as a result.");
> +    NS_RUNTIMEABORT(formattedMessage.get());
> +  }
> +
> +}

Nit: Nuke extra newline.

::: ipc/glue/ProtocolUtils.h
@@ +183,5 @@
> +ProtocolErrorBreakpoint(const char* aMsg) MOZ_NEVER_INLINE;
> +
> +void
> +FatalError(const char* aProtocolName, const char* aMsg,
> +           base::ProcessHandle aHandle, bool isParent) MOZ_NEVER_INLINE;

Nit: aIsParent.
Attachment #794030 - Flags: review?(bent.mozilla) → review+
And a needs-clobber followup, possibly premature, but definitely can't hurt:

https://hg.mozilla.org/integration/mozilla-inbound/rev/5afea7f06923
You need to log in before you can comment on or make changes to this bug.