Clean up various sizeOf functions

RESOLVED FIXED in mozilla12

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

unspecified
mozilla12
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
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?
Attachment #590561 - Flags: review?(bhackett1024)
(Assignee)

Comment 2

6 years ago
Created attachment 590562 [details] [diff] [review]
strings

Strings cleanup.
Attachment #590562 - Flags: review?(bhackett1024)
(Assignee)

Comment 3

6 years ago
Created attachment 590563 [details] [diff] [review]
shapes

Shapes cleanup.
Attachment #590563 - Flags: review?(bhackett1024)
(Assignee)

Comment 4

6 years ago
Created attachment 590564 [details] [diff] [review]
scripts

Scripts cleanup.
Attachment #590564 - Flags: review?(bhackett1024)
(Assignee)

Comment 5

6 years ago
Created attachment 590565 [details] [diff] [review]
type objects

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+
(Assignee)

Comment 10

6 years ago
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
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
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
You need to log in before you can comment on or make changes to this bug.