1192335 - Expose allocation bytesize from devtools memory module

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1192335-bytesize.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=eb982882b8e8

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Comment on attachment 8645171 [details] [diff] [review]
1192335-bytesize.patch

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

Looks great! Thanks!

::: toolkit/devtools/server/tests/mochitest/test_memory_allocations_07.html
@@ +35,5 @@
> +    yield memory.stopRecordingAllocations();
> +
> +    ok(response.allocationSizes, "The response should have bytesizes.");
> +    is(response.allocationSizes.length, response.allocations.length,
> +       "There should be a bytesize for every allocation.");

Let's additionally assert that the allocations length is >= 3.

@@ +36,5 @@
> +
> +    ok(response.allocationSizes, "The response should have bytesizes.");
> +    is(response.allocationSizes.length, response.allocations.length,
> +       "There should be a bytesize for every allocation.");
> +    ok(response.allocationSizes.every(s => typeof s === "number"), "every bytesize is a number");

Lets assert that it is not only a number, but a positive, non-zero number.

::: toolkit/devtools/shared/memory.js
@@ +289,5 @@
>  
>        // Safe because SavedFrames are frozen/immutable.
>        let waived = Cu.waiveXrays(stack);
>  
>        // Ensure that we have a form, count, and index for new allocations

Missed this in the last review, but "count" should be "size" now.

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/1c3965ae8dee

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/1c3965ae8dee