Closed Bug 1231763 Opened 5 years ago Closed 5 years ago

Extract breakdown parsing from JS::ubi::Census

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: fitzgen, Assigned: fitzgen)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

No description provided.
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Attachment #8697348 - Attachment is obsolete: true
Attachment #8697348 - Flags: review?(jimb)
Comment on attachment 8697471 [details] [diff] [review]
Extract breakdown parsing from JS::ubi::Census

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

Now that I've looked this over, it seems like a move in the right direction. The different operations (parse, traverse) take arguments that are reasonable for their role, and the dependency on Census is gone, as required. Thanks very much!

Just one hunk needs to come out, I think:

::: js/public/Utility.h
@@ +306,5 @@
> +    template <class T, typename... Args>                        \
> +    QUALIFIERS T *                                              \
> +    NEWNAME(Args&&... args) MOZ_HEAP_ALLOCATOR {                \
> +        void* memory = ALLOCATOR(sizeof(T));                    \
> +        return MOZ_LIKELY(memory)                               \

I don't think this change is controversial, but I think it should be put in a separate bug simply because it's so central. Terrence might have something to add, for example.
Attachment #8697471 - Flags: review?(jimb) → review+
Attachment #8697471 - Attachment is obsolete: true
Split the MOZ_LIKELY in js_new out to bug 1233101.
Fix the static analysis bustage by adding explicit constructors where needed.
Attachment #8699047 - Flags: review+
Attachment #8699026 - Attachment is obsolete: true
Pretty sure those hazard failures are infra as I'm seeing them in other unrelated pushes as well.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a87d841e91aa
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.