Psuedo frames created by js::AutoSPSEntry don't get their category propogated

RESOLVED FIXED in Firefox 43

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: fitzgen, Assigned: fitzgen)

Tracking

unspecified
mozilla43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Created attachment 8662083 [details] [diff] [review]
Ensure that entries created by AutoSPSEntry propogate their category information

This commit renames ProfileEntry::set{Js,Cpp}Frame methods to
ProfileEntry::init{Js,Cpp}Frame to highlight the fact that they are intended to
initialize the entry, and that setting other flags should happen after one of
these calls.
Attachment #8662083 - Flags: review?(kvijayan)
Verified that this also fixes bug 1164344 and that the category information is indeed passed along now.
Comment on attachment 8662083 [details] [diff] [review]
Ensure that entries created by AutoSPSEntry propogate their category information

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

::: js/public/ProfilingStack.h
@@ +134,5 @@
>          return flags_ & CATEGORY_MASK;
>      }
>      void setCategory(Category c) volatile {
> +        MOZ_ASSERT(c != Category::FIRST);
> +        MOZ_ASSERT(c != Category::LAST);

This assert seems wrong to me.  FIRST and LAST are valid categories (aliases for OTHER and EVENTS).

Shouldn't it be something like:

  MOZ_ASSERT(c >= Category::FIRST && c <= Category::LAST)

??
See Also: → bug 1205782
Created attachment 8662583 [details] [diff] [review]
Ensure that entries created by AutoSPSEntry propogate their category information

This commit renames ProfileEntry::set{Js,Cpp}Frame methods to
ProfileEntry::init{Js,Cpp}Frame to highlight the fact that they are intended to
initialize the entry, and that setting other flags should happen after one of
these calls.
Attachment #8662083 - Attachment is obsolete: true
Attachment #8662083 - Flags: review?(kvijayan)
Attachment #8662583 - Flags: review?(kvijayan)
Created attachment 8662596 [details] [diff] [review]
Ensure that entries created by AutoSPSEntry propogate their category information

This commit renames ProfileEntry::set{Js,Cpp}Frame methods to
ProfileEntry::init{Js,Cpp}Frame to highlight the fact that they are intended to
initialize the entry, and that setting other flags should happen after one of
these calls.
Attachment #8662583 - Attachment is obsolete: true
Attachment #8662583 - Flags: review?(kvijayan)
Attachment #8662596 - Flags: review?(kvijayan)

Updated

3 years ago
Attachment #8662596 - Flags: review?(kvijayan) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f33142322571
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox43: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Depends on: 1209779

Updated

2 years ago
Duplicate of this bug: 1210521
You need to log in before you can comment on or make changes to this bug.