Expose allocation bytesize from devtools memory module

RESOLVED FIXED in Firefox 42

Status

()

Firefox
Developer Tools: Performance Tools (Profiler/Timeline)
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jsantell, Assigned: jsantell)

Tracking

(Blocks: 1 bug)

41 Branch
Firefox 42
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Updated

3 years ago
Blocks: 1191480
No longer blocks: 1184585
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
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
You need to log in before you can comment on or make changes to this bug.