1346486 - Add memory reporters for DataStorage

Status: RESOLVED FIXED

(Core :: Security: PSM, enhancement)

Description

[(no longer active)]

Apparently DataStorage can be expected to be ~1MB or so, we need to add a memory reporter for it.

Comment 1

[(no longer active)]

FWIW after I implemented the memory reporter and measured the memory usage, it's actually not bad at all.  On a new profile when loading nytimes.com in a content process in a new profile in a debug build, the entire memory usage of DataStorage in both the parent and child process is less than 3KB in each process.

Comment 2

[(no longer active)]

Attachment: Add a memory reporter for PSM DataStorage caches

Comment 3

[Dana Keeler (she/her) (use needinfo) [:keeler]]

So I compiled and ran this, and the data-storage items in about:memory always show up as 0.00 MB for me, even after visiting some sites and checking that e.g. SiteSecurityServiceState.txt gets some entries (and restarting Firefox, etc.). What am I doing wrong? Or rather, how do I confirm that this is working as intended?

Comment 4

[(no longer active)]

(In reply to David Keeler [:keeler] (use needinfo?) from comment #3)
> So I compiled and ran this, and the data-storage items in about:memory
> always show up as 0.00 MB for me, even after visiting some sites and
> checking that e.g. SiteSecurityServiceState.txt gets some entries (and
> restarting Firefox, etc.). What am I doing wrong? Or rather, how do I
> confirm that this is working as intended?

Hmm... I have in my about:memory:

├────────3,056 B (00.00%) -- data-storage
│        ├──2,384 B (00.00%) ── SiteSecurityServiceState.txt
│        ├────336 B (00.00%) ── AlternateServices.txt
│        └────336 B (00.00%) ── SecurityPreloadState.txt

$ cat objdir/tmp/scratch_user/SiteSecurityServiceState.txt
snippets.cdn.mozilla.net:HSTS	1	17239	1520988504628,1,0
self-repair.mozilla.org:HPKP	1	17239	1489539175442,1,0,WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18=r/mIkG3eEpVdm+u/ko/cwxzOMo1bk4TyHIlByibiA5E=YLh1dUR9y6Kja30RrAn7JKnbQG/uEtLMkBgFF2Fuihg=sRHdihwgkaib1P1gxX8HFszlD+7/gTfNvuAybgLPNis=
services.addons.mozilla.org:HSTS	0	17236	1520798310636,1,0
services.addons.mozilla.org:HPKP	0	17236	1494446310650,1,1,WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18=r/mIkG3eEpVdm+u/ko/cwxzOMo1bk4TyHIlByibiA5E=
versioncheck-bg.addons.mozilla.org:HSTS	0	17236	1489262613285,1,0
5290727.fls.doubleclick.net:HSTS	0	17236	1489284133160,1,0
self-repair.mozilla.org:HSTS	1	17239	1520988775422,1,1
connect.facebook.net:HSTS	0	17236	1504814423396,1,0
versioncheck-bg.addons.mozilla.org:HPKP	0	17236	1494446313287,1,1,WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18=r/mIkG3eEpVdm+u/ko/cwxzOMo1bk4TyHIlByibiA5E=
aus5.mozilla.org:HSTS	0	17236	1520798553474,1,0
www.google-analytics.com:HSTS	0	17236	1500148402957,1,1
www.mozilla.org:HSTS	1	17239	1520988787289,1,0
analytics.twitter.com:HSTS	0	17236	2120401062202,1,0
www.youtube.com:HSTS	0	17236	1520798643514,1,0

I just run Firefox with ./mach run without any arguments...

If you do the exact same thing as above, any chance you can please add some printfs to DataStorage::SizeOfIncludingThis() to see if that gets called and if so, what sizes the hashtables have there?  Thanks!

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8846342 [details] [diff] [review]
Add a memory reporter for PSM DataStorage caches

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

Ah, I think what's happening is about:memory is stuck on using only MB for display purposes for me - when I used a ~50k SiteSecurityServiceState.txt it showed up as 0.05 MB, so it looks like I'm not seeing anything smaller than 10KB (I also confirmed that this code is getting called and that it is returning non-zero values for the sizes of the tables). So that appears to be a different bug (or perhaps a feature!).
Looks good to me - I just noted some nits.

::: security/manager/ssl/DataStorage.cpp
@@ +55,5 @@
> +      RefPtr<DataStorage> ds = DataStorage::Get(file);
> +      size_t amount = ds->SizeOfIncludingThis(MallocSizeOf);
> +      nsPrintfCString path("explicit/data-storage/%s",
> +                           NS_ConvertUTF16toUTF8(file).get());
> +      aHandleReport->Callback(EmptyCString(), path, KIND_HEAP, UNITS_BYTES,

nit: let's add "Unused << " to be clear we're ignoring the return value here

@@ +69,1 @@
>  NS_IMPL_ISUPPORTS(DataStorage,

nit: if you feel like making the formatting here consistent, that might be nice (not a big deal either way - I think having these all on one line looks nicer, though)

Comment 6

[(no longer active)]

(In reply to David Keeler [:keeler] (use needinfo?) from comment #5)
> Ah, I think what's happening is about:memory is stuck on using only MB for
> display purposes for me - when I used a ~50k SiteSecurityServiceState.txt it
> showed up as 0.05 MB, so it looks like I'm not seeing anything smaller than
> 10KB (I also confirmed that this code is getting called and that it is
> returning non-zero values for the sizes of the tables). So that appears to
> be a different bug (or perhaps a feature!).

Oh, I think I know why the difference is: try checking the verbose checkbox!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/038dece45ca8
Add a memory reporter for PSM DataStorage caches; r=keeler

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/038dece45ca8