Closed Bug 1192335 Opened 4 years ago Closed 4 years ago

Expose allocation bytesize from devtools memory module

Categories

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

41 Branch
defect
Not set

Tracking

(firefox42 fixed)

RESOLVED FIXED
Firefox 42
Tracking Status
firefox42 --- fixed

People

(Reporter: jsantell, Assigned: jsantell)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
Blocks: perf-tools-fx43
No longer blocks: perf-tools-fx42
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.
Attachment #8645171 - Flags: review?(nfitzgerald) → review+
https://hg.mozilla.org/mozilla-central/rev/1c3965ae8dee
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.