836186 - Investigate memory overhead for providers and measurements

Status: RESOLVED FIXED

(Firefox Health Report Graveyard :: Client: Desktop, defect)

Description

[Gregory Szorc [:gps]]

I temporarily disabled all FHR providers (current count is 6) in a local build and observed their shared JS compartment decrease in size by ~133kb. This surprised me as ~22kb per compartment seemed a bit steep to me. I wonder how much of this was per-object overhead vs overhead for compiled code, etc.

I think we should investigate whether just-in-time creating providers (at least the ones that provide constant data) nets a significant memory win. If so, this should be a relatively low hanging fruit.

If per JS object overhead is high, we should see about reducing it, as the number of providers will only grow over time.

Comment 1

[Gregory Szorc [:gps]]

Attachment: Only load constant providers at collection time, v1

This saves ~53k by my measurements.

While I was measuring the effect, I obtained the following numbers:

464 as currently implemented
384 don't create provider instances
389 create and delete instances
419 constructor and populate prefs and other small properties
411 this patch

Comment 2

[Richard Newman [:rnewman]]

Comment on attachment 708202 [details] [diff] [review]
Only load constant providers at collection time, v1

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

Nice.

::: services/healthreport/healthreporter.jsm
@@ +591,5 @@
> +        try {
> +          let provider = this.initProviderFromType(providerType);
> +          yield this.registerProvider(provider);
> +        } catch (ex) {
> +          this._log.warn("Error registering constant only provider: " +

Hyphenate constant-only for clarity.

@@ +598,5 @@
> +      }
> +
> +      let result;
> +      try {
> +        result = yield this._collector.collectConstantData();

Something to think about: can we get to the point of doing this only once, so that we can collect this once, and afterwards discard _constantOnlyProviders and avoid re-building each time?

(And is it worth doing so?)

@@ +605,5 @@
> +          if (!provider.constantOnly) {
> +            continue;
> +          }
> +
> +          this._log.info("Shutting down constant only provider: " +

Hyphenate constant-only for clarity.

Comment 3

[Gregory Szorc [:gps]]

https://hg.mozilla.org/services/services-central/rev/3446cda2c8b8

Comment 4

[Gregory Szorc [:gps]]

(In reply to Richard Newman [:rnewman] from comment #2)
> @@ +598,5 @@
> > +      }
> > +
> > +      let result;
> > +      try {
> > +        result = yield this._collector.collectConstantData();
> 
> Something to think about: can we get to the point of doing this only once,
> so that we can collect this once, and afterwards discard
> _constantOnlyProviders and avoid re-building each time?
> 
> (And is it worth doing so?)

Yes. However, at current, collectConstantData really means "collect data at upload time" and this is being used as "collect data every ~24h."

We need to introduce some new collect* APIs.

Comment 5

[Gregory Szorc [:gps]]

https://hg.mozilla.org/mozilla-central/rev/3446cda2c8b8