reduce logging codesize by commoning LogModule conversions

RESOLVED FIXED in Firefox 57

Status

()

RESOLVED FIXED
a year ago
a year ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

unspecified
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(1 attachment)

The current definition of MOZ_LOG requires calling
LazyLogModule::operator() multiple times, which is unnecessary.  We can
avoid that with a bit of clever macro definition and a lengthy
explanation.
Created attachment 8898377 [details] [diff] [review]
reduce logging codesize by commoning LogModule conversions

The comment in Logging.h ideally tells you everything you need to know.

This reduces codesize by ~150K on Linux x86-64, but that measurement is on top
of bug 1386918, which I haven't yet gotten all the performance measurements
done on.  I think that means it should be even more effective without the other
bug's patch, but it would also decrease the impact of the other patch.

This patch's performance impact should be minimal to nonexistent,
unlike the other patch, since we had to call LazyLogModule::operator()
*anyway*; we're just making sure that the actual printing of the log message
doesn't have to call it again.
Attachment #8898377 - Flags: review?(erahm)

Comment 2

a year ago
Comment on attachment 8898377 [details] [diff] [review]
reduce logging codesize by commoning LogModule conversions

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

Whoa! Nice find, r=me.

::: xpcom/base/Logging.h
@@ +199,5 @@
>  //     ...compute things to log and log them...
>  //   }
>  //
>  // This also has the nice property that no special definition of MOZ_LOG is
>  // required when logging is disabled.

I guess this isn't true anymore...
Attachment #8898377 - Flags: review?(erahm) → review+

Comment 3

a year ago
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/770ed08655c3
reduce logging codesize by commoning LogModule conversions; r=erahm

Comment 4

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/770ed08655c3
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.