Closed
Bug 746714
Opened 13 years ago
Closed 12 years ago
Report telemetry overhead in about:memory
Categories
(Toolkit :: Telemetry, defect)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: taras.mozilla, Assigned: froydnj)
References
(Blocks 1 open bug)
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file)
8.57 KB,
patch
|
taras.mozilla
:
review+
n.nethercote
:
review+
|
Details | Diff | Splinter Review |
Telemetry use is expanding rapidly. We have histograms, hashtables, lots of decent size strings(as keys for hose hashtables). Would be good to get this surfaced in about:memory to see if it's a problem.
Updated•13 years ago
|
Blocks: DarkMatter
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment 1•13 years ago
|
||
Taras says the memory usage should be mostly there after only a short time -- we don't need to wait long.
Assignee | ||
Comment 2•12 years ago
|
||
I will poke at this; the main pain is setting up helper functions for all the hash tables we have.
Assignee: nobody → nfroyd
OS: Windows 7 → All
Hardware: x86_64 → All
Assignee | ||
Comment 3•12 years ago
|
||
The ipc/ parts of this are a little ugly; I didn't think adding an dependency on xpcom would be a good idea, so the histogram bits have a manual definition of the moral equivalent of nsMallocSizeOfFun. Not checked for double-counting with DMD< but I'm reasonably certain there's no double-counting here. The good news is that on startup with a couple of tabs, Telemetry is taking a minimal amount of memory (~50K or so). So not a big chunk of the 15% of heap-unclassified I see nowadays on my Linux box, but progress nonetheless.
Attachment #665739 -
Flags: review?(taras.mozilla)
Attachment #665739 -
Flags: review?(n.nethercote)
Comment 4•12 years ago
|
||
Comment on attachment 665739 [details] [diff] [review] patch Review of attachment 665739 [details] [diff] [review]: ----------------------------------------------------------------- ::: ipc/chromium/src/base/histogram.cc @@ +414,5 @@ > +{ > + size_t n = 0; > + n += aMallocSizeOf(this); > + // We're not allowed to do deep dives into STL data structures. This > + // is as close as we can get to measuring this array. We use STL data structures in this code? Yikes. ::: ipc/chromium/src/base/histogram.h @@ +344,5 @@ > > bool Serialize(Pickle* pickle) const; > bool Deserialize(void** iter, const Pickle& pickle); > > + size_t SizeOfExcludingThis(size_t (*aMallocSizeOf)(const void*)); Can you create a |MallocSizeOf| typedef within Histogram? And maybe mention in a comment why you're not using the standard |nsMallocSizeOf|.
Attachment #665739 -
Flags: review?(n.nethercote) → review+
Reporter | ||
Comment 5•12 years ago
|
||
Comment on attachment 665739 [details] [diff] [review] patch Cool. Looks good to me.
Reporter | ||
Comment 6•12 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #4) > Comment on attachment 665739 [details] [diff] [review] > patch > > Review of attachment 665739 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: ipc/chromium/src/base/histogram.cc > @@ +414,5 @@ > > +{ > > + size_t n = 0; > > + n += aMallocSizeOf(this); > > + // We're not allowed to do deep dives into STL data structures. This > > + // is as close as we can get to measuring this array. > > We use STL data structures in this code? Yikes. What's wrong with STL in this case?
Reporter | ||
Updated•12 years ago
|
Attachment #665739 -
Flags: review?(taras.mozilla) → review+
Comment 7•12 years ago
|
||
> What's wrong with STL in this case?
Doesn't Mozilla code generally avoid STL because it uses exceptions and we don't?
Assignee | ||
Comment 8•12 years ago
|
||
(In reply to Nicholas Nethercote [:njn] from comment #7) > > What's wrong with STL in this case? > > Doesn't Mozilla code generally avoid STL because it uses exceptions and we > don't? There was a short discussion about this on IRC the other day. vector and string seem to be OK (!); there was at least one other header that was deemed OK (deque?). I'm not really sure how infallible new and exceptions thereof figure in to that, though. =/ In any event, both the ipc code and the webrtc code use them pretty liberally...
Assignee | ||
Comment 9•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9859a8c68136
Status: NEW → ASSIGNED
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9859a8c68136
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in
before you can comment on or make changes to this bug.
Description
•