Add MemoryMultiReporter, a helper class that reduces some boilerplate, and convert all existing multi-reporters to use it.

RESOLVED FIXED in mozilla28

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla28
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
We already have MemoryUniReporter, which reduces the amount of boilerplate for
uni-reporters.  This bug aims to do the same for multi-reporters.
(Assignee)

Comment 1

5 years ago
Created attachment 826572 [details] [diff] [review]
Add MemoryMultiReporter, a helper class that reduces some boilerplate, and convert all existing multi-reporters to use it.

wchen, I havne't asked you for review before, but this is a pretty simple
(albeit repetitive) change.

Changes to each class.

- Inherit from mozilla::MemoryMultiReporter instead of nsIMemoryReporter.

- Set the reporter name via the constructor's call to the super-class
  constructor, and remove GetName().  (This requires adding a constructor if
  there isn't already one.)

- Remove NS_DECL_ISUPPORTS and NS_DECL_NSIMEMORY_REPORTER from within the
  class.  (And add a decl for CollectReports() if necessary.)

- Remove NS_IMPL_ISUPPORTS1(...) outside the class.


Other things to note:

- I left nsWindowMemoryReporter because it inherits from multiple classes and I
  didn't want to screw up the NS_IMPL_SUPPORTS somehow.

- I did a few other minor tweaks:  some minor renamings, trailing whitespace
  removal, etc.

- 21 files changed, 167 insertions(+), 250 deletions(-)
Attachment #826572 - Flags: review?(wchen)
Comment on attachment 826572 [details] [diff] [review]
Add MemoryMultiReporter, a helper class that reduces some boilerplate, and convert all existing multi-reporters to use it.

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

Changes look good to me. I've mostly just pointed out nits where the code doesn't match the local coding style.

::: content/canvas/src/WebGLContextReporter.cpp
@@ +15,4 @@
>  {
>    public:
> +    WebGLMemoryReporter()
> +      : MemoryMultiReporter("webgl")

4 space indent. While you are here, you should probably remove the indent on 'public' because it is not indented in other WebGL files.

::: dom/ipc/ContentParent.cpp
@@ +244,4 @@
>  {
>  public:
> +    ContentParentsMemoryReporter()
> +      : MemoryMultiReporter("content-parents")

4 space indent.

::: gfx/thebes/gfxASurface.cpp
@@ +635,4 @@
>  {
>  public:
>      SurfaceMemoryReporter()
> +      : MemoryMultiReporter("gfx-surface")

4 space indent.

::: gfx/thebes/gfxFont.h
@@ +954,4 @@
>      {
>      public:
> +        MemoryReporter()
> +          : MemoryMultiReporter("font-cache")

4 space indent.

::: gfx/thebes/gfxPlatformFontList.cpp
@@ +74,4 @@
>  NS_MEMORY_REPORTER_MALLOC_SIZEOF_FUN(FontListMallocSizeOf)
>  
> +gfxPlatformFontList::MemoryReporter::MemoryReporter()
> +  : MemoryMultiReporter("font-list")

4 space indent. The braces below can also go on the same line to be consistent.

::: xpcom/base/nsCycleCollector.cpp
@@ +2391,4 @@
>  {
>    public:
>      CycleCollectorReporter(nsCycleCollector* aCollector)
> +      : MemoryMultiReporter("cycle-collector")

4 space indent, and the convention in this file is to have the comma after the previous item in the initialization list so it should read:

: MemoryMultiReporter("cycle-collector"),
  mCollector(aCollector)

::: xpcom/base/nsIMemoryReporter.idl
@@ +530,5 @@
>  
> +// The following base class reduces the amount of boilerplate code required for
> +// memory multi-reporters.  You just need to provide the following.
> +// - The constant value: name.  It is an argument to the MemoryMultiReporter
> +//   constructor.

It might be useful to mention that the name has to be unique since that's a requirement for the nsIMemoryReporter name.

::: xpcom/ds/nsObserverService.cpp
@@ +46,4 @@
>  {
>  public:
> +    ObserverServiceReporter()
> +      : MemoryMultiReporter("observer-service")

4 space indent.
Attachment #826572 - Flags: review?(wchen) → review+
https://hg.mozilla.org/mozilla-central/rev/f08890ce0591
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.