1083020 - Add a (dummy) stats method to all LibHandles

Status: RESOLVED FIXED

(Core :: mozglue, defect)

Description

[Mike Hommey [:glandium]]

This is the exact opposite of bug 1082529, because that bug requires bug 1082529 and bug 1082527 first in order to happen.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Add a (dummy) stats method to all LibHandles

While here, avoid doing anything if debug logging is disabled.

Comment 2

[Nathan Froyd [:froydnj]]

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?

Comment 3

[Mike Hommey [:glandium]]

(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

[Nathan Froyd [:froydnj]]

(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

[Nathan Froyd [:froydnj]]

Oh, maybe the logging is compile-time determined, not runtime.  In that case, I now understand.

Comment 6

[Mike Hommey [:glandium]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f5fdd080b46

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1f5fdd080b46