Last Comment Bug 720219 - Clean up various sizeOf functions
: Clean up various sizeOf functions
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla12
Assigned To: Nicholas Nethercote [:njn]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-22 05:30 PST by Nicholas Nethercote [:njn]
Modified: 2012-01-25 07:02 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
objects (9.46 KB, patch)
2012-01-22 05:33 PST, Nicholas Nethercote [:njn]
bhackett1024: review+
Details | Diff | Review
strings (1.38 KB, patch)
2012-01-22 05:35 PST, Nicholas Nethercote [:njn]
bhackett1024: review+
Details | Diff | Review
shapes (2.21 KB, patch)
2012-01-22 05:35 PST, Nicholas Nethercote [:njn]
bhackett1024: review+
Details | Diff | Review
scripts (6.02 KB, patch)
2012-01-22 05:37 PST, Nicholas Nethercote [:njn]
bhackett1024: review+
Details | Diff | Review
type objects (1.60 KB, patch)
2012-01-22 05:38 PST, Nicholas Nethercote [:njn]
bhackett1024: review+
Details | Diff | Review

Description Nicholas Nethercote [:njn] 2012-01-22 05:30:16 PST
A lot of the JS engine's size measuring functions were written before I settled down on the current style and naming conventions (https://wiki.mozilla.org/Memory_Reporting).  This bug is about bringing some of them up to standard.
Comment 1 Nicholas Nethercote [:njn] 2012-01-22 05:33:44 PST
Created attachment 590561 [details] [diff] [review]
objects

Objects cleanup.  This also splits "object-slots" into "object-slots" and "object-elements" in about:memory.  But "object-elements" is always zero, AFAICT.  Does that sound right?
Comment 2 Nicholas Nethercote [:njn] 2012-01-22 05:35:16 PST
Created attachment 590562 [details] [diff] [review]
strings

Strings cleanup.
Comment 3 Nicholas Nethercote [:njn] 2012-01-22 05:35:45 PST
Created attachment 590563 [details] [diff] [review]
shapes

Shapes cleanup.
Comment 4 Nicholas Nethercote [:njn] 2012-01-22 05:37:38 PST
Created attachment 590564 [details] [diff] [review]
scripts

Scripts cleanup.
Comment 5 Nicholas Nethercote [:njn] 2012-01-22 05:38:10 PST
Created attachment 590565 [details] [diff] [review]
type objects

Type objects.
Comment 6 Brian Hackett (:bhackett) 2012-01-23 15:13:33 PST
Comment on attachment 590561 [details] [diff] [review]
objects

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

::: js/src/jsobjinlines.h
@@ +1227,3 @@
>              (js::ObjectElements::VALUES_PER_HEADER +
>               getElementsHeader()->capacity) * sizeof(js::Value);
> +        *elementsSize +=

This should use '=' rather than '+=', *elementsSize may be uninitialized.  The elementsSize should be non-zero for any dense array with dynamically allocated elements (including any dense array with more than 16 elements).  Have you checked to see if this branch is being hit?
Comment 7 Brian Hackett (:bhackett) 2012-01-23 15:15:16 PST
Comment on attachment 590562 [details] [diff] [review]
strings

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

::: js/src/vm/String.h
@@ +407,4 @@
>  
>      /* Gets the number of bytes that the chars take on the heap. */
>  
> +    JS_FRIEND_API(size_t) sizeOfExcludingThis(JSMallocSizeOfFun mallocSizeOf);

This doesn't need to be JS_FRIEND_API anymore.
Comment 8 Brian Hackett (:bhackett) 2012-01-23 15:22:08 PST
Comment on attachment 590564 [details] [diff] [review]
scripts

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

::: js/src/jsscript.h
@@ +684,4 @@
>       * (which can be larger than the in-use size).
>       */
> +    JS_FRIEND_API(size_t) computedSizeOfData();
> +    JS_FRIEND_API(size_t) sizeOfData(JSMallocSizeOfFun mallocSizeOf);

These don't need to be JS_FRIEND_API anymore.
Comment 9 Brian Hackett (:bhackett) 2012-01-23 15:23:48 PST
Comment on attachment 590565 [details] [diff] [review]
type objects

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

::: js/public/MemoryMetrics.h
@@ +201,3 @@
>                                     JSMallocSizeOfFun mallocSizeOf);
>  
>  extern JS_PUBLIC_API(void)

This doesn't need to be JS_PUBLIC_API anymore.

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