Closed Bug 1447744 Opened 2 years ago Closed 2 years ago

don't implement nsINamed on mozilla::Runnable #ifdef RELEASE_OR_BETA


(Core :: XPCOM, enhancement)

Not set



Tracking Status
firefox61 --- fixed


(Reporter: froydnj, Assigned: froydnj)


(Blocks 1 open bug)



(1 file)

mozilla::Runnable implements nsINamed so that we can collect telemetry on runnables, both for labeling purposes and for knowing how much time certain kinds of runnables take to execute.  But it implements this interface on all channels, when we only collect the necessary telemetry on #ifndef RELEASE_OR_BETA.  Implementing the interface increases the vtable size of every Runnable subclass by ~50% (72 bytes -> 120 bytes), and since we have hundreds of subclasses of Runnable, that overhead adds up pretty quick.

Local measurements says this saves ~100K (!) on x86-64 Linux.
We only use nsINamed on runnables for certain kinds of telemetry, and
those kinds of telemetry aren't being gathered on RELEASE_OR_BETA
builds.  We effectively make nsINamed::GetName a no-op when we're not
collecting said telemetry.  But implementing nsINamed does have a cost:
the vtable of every runnable (and we have hundreds of subclasses of
mozilla::Runnable) will contain pointers for GetName (and extra pointers
for QueryInterface/AddRef/Release), and all those pointers times all
those subclasses adds up quickly.  Let's not implement nsINamed when
nsINamed isn't going to be used.

This change saves ~100K of binary size on x86-64 Linux; the savings
should be similar on other 64-bit systems, and ~50K on 32-bit systems.
Attachment #8961072 - Flags: review?(erahm)
Comment on attachment 8961072 [details] [diff] [review]
don't implement nsINamed for mozilla::Runnable when it's unused

Review of attachment 8961072 [details] [diff] [review]:

That size savings is quite surprising! r=me
Attachment #8961072 - Flags: review?(erahm) → review+
Pushed by
don't implement nsINamed for mozilla::Runnable when it's unused; r=erahm
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.