Closed Bug 720219 Opened 8 years ago Closed 8 years ago

Clean up various sizeOf functions

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: njn, Assigned: njn)

Details

Attachments

(5 files)

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.
Attached patch objectsSplinter Review
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?
Attachment #590561 - Flags: review?(bhackett1024)
Attached patch stringsSplinter Review
Strings cleanup.
Attachment #590562 - Flags: review?(bhackett1024)
Attached patch shapesSplinter Review
Shapes cleanup.
Attachment #590563 - Flags: review?(bhackett1024)
Attached patch scriptsSplinter Review
Scripts cleanup.
Attachment #590564 - Flags: review?(bhackett1024)
Attached patch type objectsSplinter Review
Type objects.
Attachment #590565 - Flags: review?(bhackett1024)
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?
Attachment #590561 - Flags: review?(bhackett1024) → review+
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.
Attachment #590562 - Flags: review?(bhackett1024) → review+
Attachment #590563 - Flags: review?(bhackett1024) → review+
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.
Attachment #590564 - Flags: review?(bhackett1024) → review+
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.
Attachment #590565 - Flags: review?(bhackett1024) → review+
You need to log in before you can comment on or make changes to this bug.