Closed Bug 1399758 Opened 2 years ago Closed 2 years ago
stylo: Measure Image
They're significant for gmail.
Oh, please ignore the "Measure PropertyDeclaration more thoroughly." patch. That's https://github.com/servo/servo/pull/18495, which has been r+'d and will land soon; the patch got pulled into this bug because of multi-repo shenanigans.
Comment on attachment 8907970 [details] Bug 1399758 - Measure ImageValue objects. . https://reviewboard.mozilla.org/r/179642/#review184880 ::: layout/style/ServoBindings.cpp:1530 (Diff revision 1) > RefPtr<css::ImageValue> value( > new css::ImageValue(url, do_AddRef(aURI.mExtraData))); > return value.forget().take(); > } > > +MOZ_DEFINE_MALLOC_ENCLOSING_SIZE_OF(GeckoImageValueMallocSizeOf) Why ENCLOSING?
Attachment #8907970 - Flags: review?(cam) → review+
> > +MOZ_DEFINE_MALLOC_ENCLOSING_SIZE_OF(GeckoImageValueMallocSizeOf) > > Why ENCLOSING? Because I auto-completed and didn't pay sufficient attention :) I will change it to MOZ_DEFINE_MALLOC_SIZE_OF.
https://github.com/servo/servo/pull/18515 is the Servo side.
Attachment #8907969 - Attachment is obsolete: true
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/1536fa69bad4 Measure ImageValue objects. r=heycam.
Backed out for Hazard failure: https://hg.mozilla.org/integration/autoland/rev/f7b896fc877805e3d3a51376f5f22b522f2e448a https://hg.mozilla.org/integration/autoland/rev/a7f12e31f7e02368b00d72ac73bf3c661d55d0ad Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=1536fa69bad4d6ee7f1f4c265fab57504f6263d9&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=131210045&repo=autoland [31.80s] #121 Analyzing Gecko_ImageValue_SizeOfIncludingThis ... Error: Indirect call aMallocSizeOf Location: _ZNK7mozilla3css10ImageValue19SizeOfIncludingThisEPFmPKvE$uint64 mozilla::css::ImageValue::SizeOfIncludingThis((uint64)(void*)*) const @ layout/style/nsCSSValue.cpp#3081 Stack Trace: Gecko_ImageValue_SizeOfIncludingThis @ layout/style/ServoBindings.cpp#1535
Flags: needinfo?(n.nethercote) → needinfo?(sphink)
Yeah, but it cannot reason abut a random function, and aMallocSizeOf could (in theory) do all sorts of random stuff. We could whitelist it manually, I guess, but you can also MOZ_ASSERT(NS_IsMainThread()) to make the analysis happy.
https://github.com/servo/servo/pull/18550 for the second attempt.
We have about 11,500 of these when loading gmail in a Stylo-enabled build, from SpecifiedUrls; the objects themselves account for about 1.3 MiB of memory, and the strings within them about 2.9 MiB. We also have a very small number of them on the Gecko side.
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
heycam: the "take 2" patch should be pushed to autoland once #18550 merges. Thanks!
Sorry for missing this needinfo. emilio gave you better advice than I probably would have.
You need to log in before you can comment on or make changes to this bug.