Last Comment Bug 648490 - Remove the "heap-used/js/string-data" memory reporter because it is totally bogus
: Remove the "heap-used/js/string-data" memory reporter because it is totally b...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Nicholas Nethercote [:njn] (on vacation until July 11)
:
Mentors:
Depends on:
Blocks: DarkMatter
  Show dependency treegraph
 
Reported: 2011-04-08 00:02 PDT by Nicholas Nethercote [:njn] (on vacation until July 11)
Modified: 2011-05-11 21:03 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (against m-c 68986:e364ff60c2cb) (6.18 KB, patch)
2011-05-04 23:04 PDT, Nicholas Nethercote [:njn] (on vacation until July 11)
gal: review+
Details | Diff | Review

Description Nicholas Nethercote [:njn] (on vacation until July 11) 2011-04-08 00:02:17 PDT
JSRuntime::stringMemoryUsed purports to record how much heap memory is used by string chars.  But strings have undergone many upheavals recently and I don't trust it.  It needs to be audited.

(Actually, it would be better to find all these chars from the actual string headers, which involves scanning the GC heap to find all the strings.  Bug 625305 has some code along those lines.  But there might be some quick fixes in the meantime.)
Comment 1 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-04 17:05:48 PDT
The about:compartments style of measuring this (trace through the GC heap) seems better -- it doesn't rely on remembering to add/subtract some global counter in a dozen different places.
Comment 2 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-04 21:18:39 PDT
I have concrete confirmation of my distrust!  Bug 633653 comment 83 shows this
report after running Kraken:

> Used Heap Memory
> 121,174,804 B (100.0%) -- heap-used
> ├──4,277,801,142 B (3530.27%) -- js
> │  ├──4,234,772,742 B (3494.76%) -- string-data
> │  ├─────34,603,008 B (28.56%) -- gc-heap
> │  ├──────7,336,784 B (06.05%) -- tjit-data
> │  │      └──7,336,784 B (06.05%) -- allocators
> │  │         ├──5,550,000 B (04.58%) -- reserve
> │  │         └──1,786,784 B (01.47%) -- main
> │  └──────1,088,608 B (00.90%) -- mjit-data
> 
> with most of the string-data given back below:
> 
> ├────3,137,357 B (02.59%) -- layout
> │    ├──3,137,357 B (02.59%) -- all
> │    └──────────0 B (00.00%) -- bidi
> └──-4,192,137,179 B (-3459.58%) -- other

I am going to simply nuke this reporter.  It can be put back in later with an
about:compartments-style heap traversal.
Comment 3 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-04 23:04:33 PDT
Created attachment 530238 [details] [diff] [review]
patch (against m-c 68986:e364ff60c2cb)
Comment 4 Nicholas Nethercote [:njn] (on vacation until July 11) 2011-05-11 21:03:20 PDT
http://hg.mozilla.org/mozilla-central/rev/62073f1776e3

Note You need to log in before you can comment on or make changes to this bug.