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

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

See bug 1204167 comment 3.

Comment 1

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

Attachment: 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.

Comment 2

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

Verified that this also fixes bug 1164344 and that the category information is indeed passed along now.

Comment 3

[Kannan Vijayan [:djvj]]

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)

??

Comment 4

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

Attachment: 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.

Comment 5

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

Attachment: 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.

Comment 6

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

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=08d5c8ba076d

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f33142322571

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f33142322571