Closed
Bug 1083020
Opened 7 years ago
Closed 7 years ago
Add a (dummy) stats method to all LibHandles
Categories
(Core :: mozglue, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: glandium, Assigned: glandium)
References
Details
Attachments
(1 file)
4.10 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
This is the exact opposite of bug 1082529, because that bug requires bug 1082529 and bug 1082527 first in order to happen.
Assignee | ||
Comment 1•7 years ago
|
||
While here, avoid doing anything if debug logging is disabled.
Attachment #8505267 -
Flags: review?(nfroyd)
![]() |
||
Comment 2•7 years ago
|
||
Comment on attachment 8505267 [details] [diff] [review] Add a (dummy) stats method to all LibHandles Review of attachment 8505267 [details] [diff] [review]: ----------------------------------------------------------------- How about we do: class LibHandle { protected: // or statsGuts, or whatever. virtual void printStats(const char* when) { }; public: void stats(const char* when) { if (MOZ_LIKELY(!Logging::IsVerbose())) { return; } printStats(when); } }; That way we don't have to think about making sure all the stats() methods DTRT. WDYT?
Attachment #8505267 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to Nathan Froyd (:froydnj) from comment #2) > That way we don't have to think about making sure all the stats() methods > DTRT. WDYT? Unfortunately, the compiler doesn't skip the loop in ElfLoader::stats without the check before it. The check in MappableSeekableZStream::stats can be removed, actually, which just leaves the one in CustomElf::Load as extra weight.
![]() |
||
Comment 4•7 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #3) > (In reply to Nathan Froyd (:froydnj) from comment #2) > > That way we don't have to think about making sure all the stats() methods > > DTRT. WDYT? > > Unfortunately, the compiler doesn't skip the loop in ElfLoader::stats > without the check before it. Wouldn't the loop get skipped because that method never gets called? Either way, doesn't make much difference to me. OK to land as is.
![]() |
||
Comment 5•7 years ago
|
||
Oh, maybe the logging is compile-time determined, not runtime. In that case, I now understand.
Assignee | ||
Comment 6•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f5fdd080b46
Comment 7•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1f5fdd080b46
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•