720219 - Clean up various sizeOf functions

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

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 [inactive]]

Attachment: 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 [inactive]]

Attachment: strings

Strings cleanup.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: shapes

Shapes cleanup.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: scripts

Scripts cleanup.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: type objects

Type objects.

Comment 6

[Brian Hackett [Laid off!]]

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 [Laid off!]]

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 [Laid off!]]

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 [Laid off!]]

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.

Comment 10

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c83476fb4257
https://hg.mozilla.org/integration/mozilla-inbound/rev/a818c5a6d021
https://hg.mozilla.org/integration/mozilla-inbound/rev/4af91c6b9316
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6849eb97d82
https://hg.mozilla.org/integration/mozilla-inbound/rev/27583d9d31b4

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/c83476fb4257
https://hg.mozilla.org/mozilla-central/rev/a818c5a6d021
https://hg.mozilla.org/mozilla-central/rev/4af91c6b9316
https://hg.mozilla.org/mozilla-central/rev/a6849eb97d82
https://hg.mozilla.org/mozilla-central/rev/27583d9d31b4