1462784 - Improve the profiler's category annotations

Status: RESOLVED FIXED

(Core :: Gecko Profiler, enhancement, P1)

Description

[Markus Stange [:mstange]]

ProfilingStackFrames have a category field, but that field is currently only being used by the old devtools performance panel which is on its way out.

We'd like to make use of category information in perf.html but it's a bit lacking, for three reasons:
 - There is no "Idle" category.
 - The set of categories is a bit awkward - GC and CC are two different categories, the name "CSS" is used for what we usually call "Layout" in gecko, and "STORAGE" has its own category even though the main thread usually doesn't spend any time in IndexedDB code.
 - The set of categories is hardcoded and needs to be kept in sync between multiple different places.

I'd like to make the following changes in this bug:
 - Make the category enum not be a bitfield any more. We never need to have multiple categories on a single stack frame, so category enum values really don't need to be bit-disjoint.
 - Consolidate the set of categories and add an "Idle" category.
 - Include the list of categories in the profile JSON, so that we don't need to keep the profile consumers in sync with Gecko in the future when it comes to categories.
 - Sprinkle more profiler stack labels with categories over the code base.
 - Update the old devtools performance panel for these changes.

Comment 1

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Change the enum ProfilingStackFrame::Category from a bitfield to a regular enum.

Review commit: https://reviewboard.mozilla.org/r/245216/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245216/

Comment 2

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Merge GC and CC categories into one category.

They're very similar as far as most users of the profiler are concerned, I'd
say, and I don't believe it's worth giving them two different colors in the
activity graphs.

Review commit: https://reviewboard.mozilla.org/r/245218/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245218/

Comment 3

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Remove EVENTS category.

Categories are useful to indicate: This much % of time was spent in this category.

The EVENTS category isn't a very good match for this. This category is currently
only set on labels of functions that handle the processing of an event. But
those functions are usually closer to the base of the stack, and the actual CPU
work during the processing of an event is usually in another category closer to
the top of the stack, e.g. in JS if we're running an event handler, or in LAYOUT
if we're hit testing the position of the event.

This changeset removes the EVENTS category and replaces all uses of it with the
OTHER category.

Review commit: https://reviewboard.mozilla.org/r/245220/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245220/

Comment 4

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Rename the CSS category to LAYOUT.

The next changeset is going to move over more annotations that Gecko developers
would count as "layout" into the LAYOUT category, and which is currently marked
as GRAPHICS.
We can add a subcategory for style resolution once we have subcategories, but
for now I think it makes more sense to put style resolution into the same bucket
as reflow and display list building.

Review commit: https://reviewboard.mozilla.org/r/245222/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245222/

Comment 5

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category.

Review commit: https://reviewboard.mozilla.org/r/245224/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245224/

Comment 6

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Add a DOM category and use it for the WebIDL binding stack labels.

Review commit: https://reviewboard.mozilla.org/r/245226/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245226/

Comment 7

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Remove the STORAGE category.

This was used to label IndexedDB work and work in storage/mozStorage*.
I don't think this deserves its own category; categories are most useful for
the main thread, and most of the time-consuming database-related work happens
on helper threads. The main thread pieces are mostly for asynchronicity-
coordination and don't usually take up time.

This patch labels IndexedDB work as DOM instead (which is maybe debatable) and
the rest as OTHER.

Review commit: https://reviewboard.mozilla.org/r/245228/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245228/

Comment 8

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Relabel various other stuff as being OTHER.

Any more specific work that is happening in these methods will have its own
specific category labeling in that specific code. The instances touched in this
patch are more on the outside and don't really know what kind of code is going
to be running inside.

Review commit: https://reviewboard.mozilla.org/r/245230/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245230/

Comment 9

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Add an IDLE category.

Review commit: https://reviewboard.mozilla.org/r/245232/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245232/

Comment 10

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Annotate idle stacks in the chromium event loop.

Review commit: https://reviewboard.mozilla.org/r/245234/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245234/

Comment 11

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Annotate idle stacks in the main thread event loop.

Review commit: https://reviewboard.mozilla.org/r/245236/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245236/

Comment 12

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Annotate idle stacks in the native event loop on macOS.

Review commit: https://reviewboard.mozilla.org/r/245238/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245238/

Comment 13

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Add the list of categories into the profile JSON and bump the version to 11.

Review commit: https://reviewboard.mozilla.org/r/245240/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245240/

Comment 14

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Annotate idle stacks in the native event loop on Windows.

Review commit: https://reviewboard.mozilla.org/r/245242/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245242/

Comment 15

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Annotate idle stacks in the native event loop on Linux.

Review commit: https://reviewboard.mozilla.org/r/245244/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245244/

Comment 16

[Panos Astithas (he/him) [:past] (please ni?)]

I'm curious how the work in bug 1420437 (which Randell is going to work on next) impacts this. Do you think that information would add more value to a STORAGE category?

Comment 17

[Markus Stange [:mstange]]

Thanks for the pointer to that bug. We can add a new category called DISKIO for it.

Comment 18

[Markus Stange [:mstange]]

Attachment: screenshot of thread activity graphs

Here's a screenshot that demonstrates the kind of visualization that can be created with the data from these categories.

Comment 19

[Nicholas Nethercote [inactive]]

Comment on attachment 8977381 [details]
Bug 1462784 - Change the enum ProfilingStackFrame::Category from a bitfield to a regular enum.

https://reviewboard.mozilla.org/r/245216/#review251284

Comment 20

[Nicholas Nethercote [inactive]]

Comment on attachment 8977382 [details]
Bug 1462784 - Merge GC and CC categories into one category.

https://reviewboard.mozilla.org/r/245218/#review251286

I'm not totally convinced this is a good idea, but I'll give you the benefit of the doubt.

Comment 21

[Nicholas Nethercote [inactive]]

Comment on attachment 8977383 [details]
Bug 1462784 - Remove EVENTS category.

https://reviewboard.mozilla.org/r/245220/#review251288

Comment 22

[Nicholas Nethercote [inactive]]

Comment on attachment 8977384 [details]
Bug 1462784 - Rename the CSS category to LAYOUT.

https://reviewboard.mozilla.org/r/245222/#review251290

Comment 23

[Nicholas Nethercote [inactive]]

Comment on attachment 8977385 [details]
Bug 1462784 - Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category.

https://reviewboard.mozilla.org/r/245224/#review251292

Comment 24

[Nicholas Nethercote [inactive]]

Comment on attachment 8977386 [details]
Bug 1462784 - Add a DOM category and use it for the WebIDL binding stack labels.

https://reviewboard.mozilla.org/r/245226/#review251294

Comment 25

[Nicholas Nethercote [inactive]]

Comment on attachment 8977387 [details]
Bug 1462784 - Remove the STORAGE category.

https://reviewboard.mozilla.org/r/245228/#review251296

Comment 26

[Nicholas Nethercote [inactive]]

Comment on attachment 8977388 [details]
Bug 1462784 - Relabel various other stuff as being OTHER.

https://reviewboard.mozilla.org/r/245230/#review251298

Comment 27

[Nicholas Nethercote [inactive]]

Comment on attachment 8977389 [details]
Bug 1462784 - Add an IDLE category.

https://reviewboard.mozilla.org/r/245232/#review251300

Comment 28

[Nicholas Nethercote [inactive]]

Comment on attachment 8977393 [details]
Bug 1462784 - Add the list of categories into the profile JSON and bump the version to 11.

https://reviewboard.mozilla.org/r/245240/#review251302

Comment 29

[Nathan Froyd [:froydnj]]

Comment on attachment 8977390 [details]
Bug 1462784 - Annotate idle stacks in the chromium event loop.

https://reviewboard.mozilla.org/r/245234/#review251432

Is this going to make it so in the profiler it doesn't look like otherwise idle threads are consuming 100% of CPU time, because the profiler can present the information more intelligently?  I have seen (non-Gecko engineers) concerned that threads are spending so much time waiting in event loops before...

Comment 30

[Nathan Froyd [:froydnj]]

Comment on attachment 8977395 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Linux.

https://reviewboard.mozilla.org/r/245244/#review251436

Comment 31

[Nathan Froyd [:froydnj]]

Comment on attachment 8977394 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Windows.

https://reviewboard.mozilla.org/r/245242/#review251438

Comment 32

[Nathan Froyd [:froydnj]]

Comment on attachment 8977391 [details]
Bug 1462784 - Annotate idle stacks in the main thread event loop.

https://reviewboard.mozilla.org/r/245236/#review251442

Have you tested profiles to see if things look reasonable with the nsThread change?

Comment 33

[Nathan Froyd [:froydnj]]

Comment on attachment 8977392 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on macOS.

https://reviewboard.mozilla.org/r/245238/#review251444

I think the `nextEventMatchingMask` label is correct, but I am very unsure about the other two.  Is there someone else (spohl, I think?) who can review this patch?

::: widget/cocoa/nsAppShell.mm:134
(Diff revision 1)
>  
>  @implementation GeckoNSApplication
>  
>  - (void)sendEvent:(NSEvent *)anEvent
>  {
> +  AUTO_PROFILER_LABEL("-[GeckoNSApplication sendEvent:]", OTHER);

I don't know enough about the Mac event loop to say whether this label is useful or not; is `sendEvent` synchronous in some way?

::: widget/cocoa/nsAppShell.mm:704
(Diff revision 1)
> +  AUTO_PROFILER_LABEL("nsAppShell::Run", IDLE);
> +

I don't think this one is actually useful/doing anything, because we enter `Run()` at the toplevel of the event loop.  What was the rationale for adding something here?

Comment 34

[Markus Stange [:mstange]]

(In reply to Nicholas Nethercote [:njn] from comment #20)
> Comment on attachment 8977382 [details]
> Bug 1462784 - Merge GC and CC categories into one category.
> 
> https://reviewboard.mozilla.org/r/245218/#review251286
> 
> I'm not totally convinced this is a good idea, but I'll give you the benefit
> of the doubt.

I'm not 100% sure yet either, I guess we'll try and and see. One of the goals with this bug is to make it cheaper to tweak these categories in the future.

That said, I recently found out that the old performance devtool, which is currently the only user of these categories, also combines GC and CC into one displayed category:
https://searchfox.org/mozilla-central/rev/9769f2300a17d3dfbebcfb457b1244bd624275e3/devtools/client/performance/modules/categories.js#62,64
It takes a full page of rather generic code to do this, and it took me quite a while to understand it. I think this is also the reason why the categories used to be laid out as a bitfield.

Comment 35

[Markus Stange [:mstange]]

(In reply to Nathan Froyd [:froydnj] from comment #29)
> Comment on attachment 8977390 [details]
> Bug 1462784 - Annotate idle stacks in the chromium event loop.
> 
> https://reviewboard.mozilla.org/r/245234/#review251432
> 
> Is this going to make it so in the profiler it doesn't look like otherwise
> idle threads are consuming 100% of CPU time, because the profiler can
> present the information more intelligently?  I have seen (non-Gecko
> engineers) concerned that threads are spending so much time waiting in event
> loops before...

Yes, that's the goal.

(In reply to Nathan Froyd [:froydnj] from comment #32)
> Comment on attachment 8977391 [details]
> Bug 1462784 - Annotate idle stacks in the main thread event loop.
> 
> https://reviewboard.mozilla.org/r/245236/#review251442
> 
> Have you tested profiles to see if things look reasonable with the nsThread
> change?

(In reply to Nathan Froyd [:froydnj] from comment #33)
> Comment on attachment 8977392 [details]
> Bug 1462784 - Annotate idle stacks in the native event loop on macOS.
> 
> https://reviewboard.mozilla.org/r/245238/#review251444
> 
> I think the `nextEventMatchingMask` label is correct, but I am very unsure
> about the other two.  Is there someone else (spohl, I think?) who can review
> this patch?

I'll ask spohl for review on the updated patch.

> ::: widget/cocoa/nsAppShell.mm:134
> (Diff revision 1)
> >  
> >  @implementation GeckoNSApplication
> >  
> >  - (void)sendEvent:(NSEvent *)anEvent
> >  {
> > +  AUTO_PROFILER_LABEL("-[GeckoNSApplication sendEvent:]", OTHER);
> 
> I don't know enough about the Mac event loop to say whether this label is
> useful or not; is `sendEvent` synchronous in some way?

Yes it is - this function is on the way from the native event loop to our event handlers e.g. in nsChildView.

> ::: widget/cocoa/nsAppShell.mm:704
> (Diff revision 1)
> > +  AUTO_PROFILER_LABEL("nsAppShell::Run", IDLE);
> > +
> 
> I don't think this one is actually useful/doing anything, because we enter
> `Run()` at the toplevel of the event loop.  What was the rationale for
> adding something here?

You're right, I've removed this instance.

Comment 36

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Add idle annotations for DOM worker threads.

Review commit: https://reviewboard.mozilla.org/r/246316/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246316/

Comment 37

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Annotate idle stacks in nsThreadPool.

Review commit: https://reviewboard.mozilla.org/r/246318/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246318/

Comment 38

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Update the devtools performance panel for the new category list.

Review commit: https://reviewboard.mozilla.org/r/246320/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246320/

Comment 39

[Markus Stange [:mstange]]

Comment on attachment 8977392 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on macOS.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245238/diff/1-2/

Comment 40

[Markus Stange [:mstange]]

Comment on attachment 8977394 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245242/diff/1-2/

Comment 41

[Markus Stange [:mstange]]

Comment on attachment 8977395 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Linux.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245244/diff/1-2/

Comment 42

[Markus Stange [:mstange]]

Comment on attachment 8977393 [details]
Bug 1462784 - Add the list of categories into the profile JSON and bump the version to 11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245240/diff/1-2/

Comment 43

[Markus Stange [:mstange]]

(In reply to Nathan Froyd [:froydnj] from comment #32)
> Comment on attachment 8977391 [details]
> Bug 1462784 - Annotate idle stacks in the main thread event loop.
> 
> https://reviewboard.mozilla.org/r/245236/#review251442
> 
> Have you tested profiles to see if things look reasonable with the nsThread
> change?

Which nsThread change are you referring to? Do you mean the change that this patch makes to ThreadEventQueue<InnerQueueT>::GetEvent?

Comment 44

[Markus Stange [:mstange]]

Comment on attachment 8980165 [details]
Bug 1462784 - Annotate idle stacks in nsThreadPool.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246318/diff/1-2/

Comment 45

[Markus Stange [:mstange]]

Comment on attachment 8977393 [details]
Bug 1462784 - Add the list of categories into the profile JSON and bump the version to 11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245240/diff/2-3/

Comment 46

[Markus Stange [:mstange]]

Comment on attachment 8980166 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246320/diff/1-2/

Comment 47

[Stephen A Pohl [:spohl]]

Comment on attachment 8977392 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on macOS.

https://reviewboard.mozilla.org/r/245238/#review252608

lgtm, thanks!

Comment 48

[Nathan Froyd [:froydnj]]

Comment on attachment 8980164 [details]
Bug 1462784 - Add idle annotations for DOM worker threads.

https://reviewboard.mozilla.org/r/246316/#review252610

Comment 49

[Nathan Froyd [:froydnj]]

Comment on attachment 8980165 [details]
Bug 1462784 - Annotate idle stacks in nsThreadPool.

https://reviewboard.mozilla.org/r/246318/#review252612

To be clear, the below is not a requirement for this patch series, or even a followup bug.  But given that `CondVar::Wait` already knows about `AUTO_PROFILER_THREAD_SLEEP`, maybe it makes sense to move more profiler-related stuff there?  Or to make sleeping automatically label things as `IDLE`?  Or something else?

::: xpcom/threads/nsThreadPool.cpp:216
(Diff revision 2)
>            if (wasIdle) {
>              --mIdleCount;
>            }
>            shutdownThreadOnExit = mThreads.RemoveObject(current);
>          } else {
> +          AUTO_PROFILER_LABEL("nsThreadPool::Run::Wait", IDLE);

I think this is reasonable.  I wonder whether it makes sense to provide something like:

```
// Or template these to ensure people provide string constants...
ProfilerWait(const char*);
ProfilerWait(const char*, TimeDuration);
```

as `CondVar` methods, so then the `AUTO_PROFILER_LABEL` bits could be moved into that.  WDYT?

Comment 50

[Markus Stange [:mstange]]

Comment on attachment 8980166 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246320/diff/2-3/

Comment 51

[Markus Stange [:mstange]]

Comment on attachment 8980166 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246320/diff/3-4/

Comment 52

[Markus Stange [:mstange]]

Comment on attachment 8977381 [details]
Bug 1462784 - Change the enum ProfilingStackFrame::Category from a bitfield to a regular enum.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245216/diff/1-2/

Comment 53

[Markus Stange [:mstange]]

Comment on attachment 8977382 [details]
Bug 1462784 - Merge GC and CC categories into one category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245218/diff/1-2/

Comment 54

[Markus Stange [:mstange]]

Comment on attachment 8977383 [details]
Bug 1462784 - Remove EVENTS category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245220/diff/1-2/

Comment 55

[Markus Stange [:mstange]]

Comment on attachment 8977384 [details]
Bug 1462784 - Rename the CSS category to LAYOUT.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245222/diff/1-2/

Comment 56

[Markus Stange [:mstange]]

Comment on attachment 8977385 [details]
Bug 1462784 - Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245224/diff/1-2/

Comment 57

[Markus Stange [:mstange]]

Comment on attachment 8977386 [details]
Bug 1462784 - Add a DOM category and use it for the WebIDL binding stack labels.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245226/diff/1-2/

Comment 58

[Markus Stange [:mstange]]

Comment on attachment 8977387 [details]
Bug 1462784 - Remove the STORAGE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245228/diff/1-2/

Comment 59

[Markus Stange [:mstange]]

Comment on attachment 8977388 [details]
Bug 1462784 - Relabel various other stuff as being OTHER.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245230/diff/1-2/

Comment 60

[Markus Stange [:mstange]]

Comment on attachment 8977389 [details]
Bug 1462784 - Add an IDLE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245232/diff/1-2/

Comment 61

[Markus Stange [:mstange]]

Comment on attachment 8977390 [details]
Bug 1462784 - Annotate idle stacks in the chromium event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245234/diff/1-2/

Comment 62

[Markus Stange [:mstange]]

Comment on attachment 8977391 [details]
Bug 1462784 - Annotate idle stacks in the main thread event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245236/diff/1-2/

Comment 63

[Markus Stange [:mstange]]

Comment on attachment 8977392 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on macOS.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245238/diff/2-3/

Comment 64

[Markus Stange [:mstange]]

Comment on attachment 8977394 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245242/diff/2-3/

Comment 65

[Markus Stange [:mstange]]

Comment on attachment 8977395 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Linux.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245244/diff/2-3/

Comment 66

[Markus Stange [:mstange]]

Comment on attachment 8980164 [details]
Bug 1462784 - Add idle annotations for DOM worker threads.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246316/diff/1-2/

Comment 67

[Markus Stange [:mstange]]

Comment on attachment 8980165 [details]
Bug 1462784 - Annotate idle stacks in nsThreadPool.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246318/diff/2-3/

Comment 68

[Markus Stange [:mstange]]

Comment on attachment 8977393 [details]
Bug 1462784 - Add the list of categories into the profile JSON and bump the version to 11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245240/diff/3-4/

Comment 69

[Markus Stange [:mstange]]

Comment on attachment 8980166 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246320/diff/4-5/

Comment 70

[Greg Tatum [:gregtatum]]

Comment on attachment 8980166 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

https://reviewboard.mozilla.org/r/246320/#review253080

It looks like this breaks the tool for any older saved profiles. Loading in a previously saved JSON file from the devtools causes the call tree to be incorrect. I think we're going to need some migration code for upgrading older profiles. Otherwise the rest of the code looks fine, and it seems to map the categories correctly.

Comment 71

[Markus Stange [:mstange]]

Attachment: Bug 1462784 - Update the devtools performance panel for the new category list.

Review commit: https://reviewboard.mozilla.org/r/247284/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/247284/

Comment 72

[Markus Stange [:mstange]]

Good catch. I've added another patch on top that fixes that case.

I looked for a good place to do the conversion but couldn't find one. Simply treating all unknown categories as "other" (which will be displayed as "Gecko") is a lot simpler and works well enough in my opinion, so I did that instead.

Comment 73

[Markus Stange [:mstange]]

Attachment: screenshot of uncollapsed Gecko categories

Falling back to the other / "Gecko" category can cause some identical category nodes to be displayed in the simplified call tree view. This happens because the frameKey is still based on the integer category value. This isn't perfect either but it really only happens when opening old profiles. Let me know if you think this is worth fixing.

Comment 74

[Greg Tatum [:gregtatum]]

I'm still getting some errors in the JS Flame Graph when loading a profile imported from before the change. The Waterfall and JS Flame chart seem to work fine.


> CATEGORIES[frameKey] is undefined[Learn More] FlameGraph.js:1295:13

https://searchfox.org/mozilla-central/rev/5a744713370ec47969595e369fd5125f123e6d24/devtools/client/shared/widgets/FlameGraph.js#1295

> TypeError: dataSource is undefined[Learn More] FlameGraph.js:667:1

https://searchfox.org/mozilla-central/rev/5a744713370ec47969595e369fd5125f123e6d24/devtools/client/shared/widgets/FlameGraph.js#667

Comment 75

[Markus Stange [:mstange]]

Comment on attachment 8977385 [details]
Bug 1462784 - Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245224/diff/2-3/

Comment 76

[Markus Stange [:mstange]]

Comment on attachment 8977386 [details]
Bug 1462784 - Add a DOM category and use it for the WebIDL binding stack labels.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245226/diff/2-3/

Comment 77

[Markus Stange [:mstange]]

Comment on attachment 8977387 [details]
Bug 1462784 - Remove the STORAGE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245228/diff/2-3/

Comment 78

[Markus Stange [:mstange]]

Comment on attachment 8977388 [details]
Bug 1462784 - Relabel various other stuff as being OTHER.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245230/diff/2-3/

Comment 79

[Markus Stange [:mstange]]

Comment on attachment 8977389 [details]
Bug 1462784 - Add an IDLE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245232/diff/2-3/

Comment 80

[Markus Stange [:mstange]]

Comment on attachment 8977390 [details]
Bug 1462784 - Annotate idle stacks in the chromium event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245234/diff/2-3/

Comment 81

[Markus Stange [:mstange]]

Comment on attachment 8977391 [details]
Bug 1462784 - Annotate idle stacks in the main thread event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245236/diff/2-3/

Comment 82

[Markus Stange [:mstange]]

Comment on attachment 8977392 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on macOS.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245238/diff/3-4/

Comment 83

[Markus Stange [:mstange]]

Comment on attachment 8977394 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245242/diff/3-4/

Comment 84

[Markus Stange [:mstange]]

Comment on attachment 8977395 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Linux.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245244/diff/3-4/

Comment 85

[Markus Stange [:mstange]]

Comment on attachment 8980164 [details]
Bug 1462784 - Add idle annotations for DOM worker threads.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246316/diff/2-3/

Comment 86

[Markus Stange [:mstange]]

Comment on attachment 8980165 [details]
Bug 1462784 - Annotate idle stacks in nsThreadPool.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246318/diff/3-4/

Comment 87

[Markus Stange [:mstange]]

Comment on attachment 8977393 [details]
Bug 1462784 - Add the list of categories into the profile JSON and bump the version to 11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245240/diff/4-5/

Comment 88

[Markus Stange [:mstange]]

Comment on attachment 8981200 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247284/diff/1-2/

Comment 89

[Greg Tatum [:gregtatum]]

Comment on attachment 8981200 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

https://reviewboard.mozilla.org/r/247284/#review254536

I think we're good to go now! Thanks for fixing it up for older profiles.

Comment 90

[Markus Stange [:mstange]]

Comment on attachment 8977381 [details]
Bug 1462784 - Change the enum ProfilingStackFrame::Category from a bitfield to a regular enum.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245216/diff/2-3/

Comment 91

[Markus Stange [:mstange]]

Comment on attachment 8977382 [details]
Bug 1462784 - Merge GC and CC categories into one category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245218/diff/2-3/

Comment 92

[Markus Stange [:mstange]]

Comment on attachment 8977383 [details]
Bug 1462784 - Remove EVENTS category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245220/diff/2-3/

Comment 93

[Markus Stange [:mstange]]

Comment on attachment 8977384 [details]
Bug 1462784 - Rename the CSS category to LAYOUT.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245222/diff/2-3/

Comment 94

[Markus Stange [:mstange]]

Comment on attachment 8977385 [details]
Bug 1462784 - Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245224/diff/3-4/

Comment 95

[Markus Stange [:mstange]]

Comment on attachment 8977386 [details]
Bug 1462784 - Add a DOM category and use it for the WebIDL binding stack labels.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245226/diff/3-4/

Comment 96

[Markus Stange [:mstange]]

Comment on attachment 8977387 [details]
Bug 1462784 - Remove the STORAGE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245228/diff/3-4/

Comment 97

[Markus Stange [:mstange]]

Comment on attachment 8977388 [details]
Bug 1462784 - Relabel various other stuff as being OTHER.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245230/diff/3-4/

Comment 98

[Markus Stange [:mstange]]

Comment on attachment 8977389 [details]
Bug 1462784 - Add an IDLE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245232/diff/3-4/

Comment 99

[Markus Stange [:mstange]]

Comment on attachment 8977390 [details]
Bug 1462784 - Annotate idle stacks in the chromium event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245234/diff/3-4/

Comment 100

[Markus Stange [:mstange]]

Comment on attachment 8977391 [details]
Bug 1462784 - Annotate idle stacks in the main thread event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245236/diff/3-4/

Comment 101

[Markus Stange [:mstange]]

Comment on attachment 8977392 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on macOS.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245238/diff/4-5/

Comment 102

[Markus Stange [:mstange]]

Comment on attachment 8977394 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245242/diff/4-5/

Comment 103

[Markus Stange [:mstange]]

Comment on attachment 8977395 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Linux.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245244/diff/4-5/

Comment 104

[Markus Stange [:mstange]]

Comment on attachment 8980164 [details]
Bug 1462784 - Add idle annotations for DOM worker threads.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246316/diff/3-4/

Comment 105

[Markus Stange [:mstange]]

Comment on attachment 8980165 [details]
Bug 1462784 - Annotate idle stacks in nsThreadPool.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246318/diff/4-5/

Comment 106

[Markus Stange [:mstange]]

Comment on attachment 8977393 [details]
Bug 1462784 - Add the list of categories into the profile JSON and bump the version to 11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245240/diff/5-6/

Comment 107

[Markus Stange [:mstange]]

Comment on attachment 8981200 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247284/diff/2-3/

Comment 108

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/52ed9a039ad2
Change the enum ProfilingStackFrame::Category from a bitfield to a regular enum. r=njn
https://hg.mozilla.org/integration/autoland/rev/8a06c0ba42f7
Merge GC and CC categories into one category. r=njn
https://hg.mozilla.org/integration/autoland/rev/ea05ff70a51d
Remove EVENTS category. r=njn
https://hg.mozilla.org/integration/autoland/rev/2ac65d100fa2
Rename the CSS category to LAYOUT. r=njn
https://hg.mozilla.org/integration/autoland/rev/cd39588aece4
Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category. r=njn
https://hg.mozilla.org/integration/autoland/rev/e670f156a603
Add a DOM category and use it for the WebIDL binding stack labels. r=njn
https://hg.mozilla.org/integration/autoland/rev/036e6f64e224
Remove the STORAGE category. r=njn
https://hg.mozilla.org/integration/autoland/rev/8f06963c7c6f
Relabel various other stuff as being OTHER. r=njn
https://hg.mozilla.org/integration/autoland/rev/2faf787fbbdf
Add an IDLE category. r=njn
https://hg.mozilla.org/integration/autoland/rev/3901070e2e60
Annotate idle stacks in the chromium event loop. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/4055eb6c3bc6
Annotate idle stacks in the main thread event loop. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/268a72b7c3c4
Annotate idle stacks in the native event loop on macOS. r=spohl
https://hg.mozilla.org/integration/autoland/rev/2fe10732076c
Annotate idle stacks in the native event loop on Windows. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/ad3802ffb780
Annotate idle stacks in the native event loop on Linux. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/8c8d30fa406c
Add idle annotations for DOM worker threads. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/cde492240e99
Annotate idle stacks in nsThreadPool. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/c8192175f360
Add the list of categories into the profile JSON and bump the version to 11. r=njn
https://hg.mozilla.org/integration/autoland/rev/a74d36598442
Update the devtools performance panel for the new category list. r=gregtatum

Comment 109

[Noemi Erli[:noemi_erli]]

Backed out 18 changesets (bug 1462784) for failures in devtools/client/performance/test/unit/test_tree-model-08.js on a CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=a74d365984427c6a7c1e8c4c37ee13aea14142bf&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified&selectedJob=181221394

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=181221394&repo=autoland&lineNumber=1829

Backout: https://hg.mozilla.org/integration/autoland/rev/e8eba439b33ea5acdc140c4d11ffd3650dff09b7

Comment 110

[Markus Stange [:mstange]]

Comment on attachment 8981200 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247284/diff/3-4/

Comment 111

[Markus Stange [:mstange]]

Comment on attachment 8981200 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247284/diff/4-5/

Comment 112

[Mozilla Autoland]

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s a3037f25e8f3e6248fcb508cc58107659d0189e7 -d 1fbf7e0d341d: rebasing 466478:a3037f25e8f3 "Bug 1462784 - Change the enum ProfilingStackFrame::Category from a bitfield to a regular enum. r=njn"
merging js/public/ProfilingStack.h
rebasing 466479:f172424042fa "Bug 1462784 - Merge GC and CC categories into one category. r=njn"
merging dom/base/nsDOMWindowUtils.cpp
merging dom/base/nsJSEnvironment.cpp
merging js/public/ProfilingStack.h
merging js/src/gc/GC.cpp
merging xpcom/base/nsCycleCollector.cpp
rebasing 466480:34fb2ca94151 "Bug 1462784 - Remove EVENTS category. r=njn"
merging dom/base/nsDOMWindowUtils.cpp
merging dom/base/nsFrameMessageManager.cpp
merging dom/events/EventDispatcher.cpp
merging dom/events/EventListenerManager.cpp
merging dom/events/EventStateManager.cpp
merging dom/ipc/ContentChild.cpp
merging dom/ipc/TabChild.cpp
merging dom/ipc/TabParent.cpp
merging dom/ipc/nsIContentChild.cpp
merging dom/ipc/nsIContentParent.cpp
merging js/public/ProfilingStack.h
merging view/nsViewManager.cpp
merging widget/android/nsAppShell.cpp
merging widget/cocoa/nsAppShell.mm
rebasing 466481:53ce9912d321 "Bug 1462784 - Rename the CSS category to LAYOUT. r=njn"
merging js/public/ProfilingStack.h
merging layout/base/RestyleManager.cpp
merging layout/style/ComputedStyle.cpp
merging tools/profiler/tests/gtest/GeckoProfiler.cpp
rebasing 466482:2f6327eddde7 "Bug 1462784 - Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category. r=njn"
merging layout/base/PresShell.cpp
merging layout/base/nsLayoutUtils.cpp
merging layout/base/nsRefreshDriver.cpp
rebasing 466483:4a1bd948e65c "Bug 1462784 - Add a DOM category and use it for the WebIDL binding stack labels. r=njn"
merging dom/bindings/Codegen.py
merging js/public/ProfilingStack.h
rebasing 466484:16abc3c4bd8e "Bug 1462784 - Remove the STORAGE category. r=njn"
merging dom/indexedDB/ActorsChild.cpp
merging dom/indexedDB/ActorsParent.cpp
merging dom/storage/StorageDBThread.cpp
merging js/public/ProfilingStack.h
merging storage/mozStorageConnection.cpp
merging storage/mozStorageStatement.cpp
merging tools/profiler/tests/gtest/GeckoProfiler.cpp
rebasing 466485:35362bc94068 "Bug 1462784 - Relabel various other stuff as being OTHER. r=njn"
merging dom/base/nsFrameLoader.cpp
merging gfx/thebes/gfxWindowsPlatform.cpp
merging js/xpconnect/src/XPCComponents.cpp
merging widget/cocoa/nsChildView.mm
merging widget/gtk/nsWindow.cpp
merging widget/uikit/nsWindow.mm
merging xpcom/io/FileUtilsWin.cpp
rebasing 466486:d79d4da6d51a "Bug 1462784 - Add an IDLE category. r=njn"
merging js/public/ProfilingStack.h
merging widget/android/nsAppShell.cpp
rebasing 466487:8328ecf80068 "Bug 1462784 - Annotate idle stacks in the chromium event loop. r=froydnj"
merging ipc/chromium/src/base/message_pump_default.cc
rebasing 466488:ec639d76a359 "Bug 1462784 - Annotate idle stacks in the main thread event loop. r=froydnj"
merging xpcom/threads/Scheduler.cpp
merging xpcom/threads/ThreadEventQueue.cpp
rebasing 466489:28e5e421b82e "Bug 1462784 - Annotate idle stacks in the native event loop on macOS. r=spohl"
merging widget/cocoa/nsAppShell.mm
rebasing 466490:9e6c0c0f2fad "Bug 1462784 - Annotate idle stacks in the native event loop on Windows. r=froydnj"
merging widget/windows/nsAppShell.cpp
rebasing 466491:83271e841a84 "Bug 1462784 - Annotate idle stacks in the native event loop on Linux. r=froydnj"
merging widget/gtk/nsAppShell.cpp
rebasing 466492:93f1bea71694 "Bug 1462784 - Add idle annotations for DOM worker threads. r=froydnj"
merging dom/workers/WorkerPrivate.cpp
rebasing 466493:9d5a9c01ca0f "Bug 1462784 - Annotate idle stacks in nsThreadPool. r=froydnj"
merging xpcom/threads/nsThreadPool.cpp
rebasing 466494:0db0496060d4 "Bug 1462784 - Add the list of categories into the profile JSON and bump the version to 11. r=njn"
merging tools/profiler/core/platform.cpp
rebasing 466495:c35ec77677ba "Bug 1462784 - Update the devtools performance panel for the new category list. r=gregtatum" (tip)
merging devtools/client/locales/en-US/performance.properties
merging devtools/client/performance/modules/categories.js
merging devtools/client/performance/modules/logic/frame-utils.js
merging devtools/client/performance/test/browser_perf-tree-view-02.js
merging devtools/client/performance/test/browser_perf-tree-view-08.js
merging devtools/client/performance/test/helpers/synth-utils.js
merging devtools/client/performance/test/unit/test_profiler-categories.js
merging devtools/client/performance/test/unit/test_tree-model-07.js
merging devtools/client/performance/test/unit/test_tree-model-08.js
merging devtools/client/performance/test/unit/test_tree-model-09.js
merging devtools/client/shared/widgets/FlameGraph.js
warning: conflicts while merging devtools/client/performance/modules/categories.js! (edit, then use 'hg resolve --mark')
warning: conflicts while merging devtools/client/performance/test/unit/test_profiler-categories.js! (edit, then use 'hg resolve --mark')
warning: conflicts while merging devtools/client/performance/test/unit/test_tree-model-08.js! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)

Comment 113

[Markus Stange [:mstange]]

Comment on attachment 8977381 [details]
Bug 1462784 - Change the enum ProfilingStackFrame::Category from a bitfield to a regular enum.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245216/diff/3-4/

Comment 114

[Markus Stange [:mstange]]

Comment on attachment 8977382 [details]
Bug 1462784 - Merge GC and CC categories into one category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245218/diff/3-4/

Comment 115

[Markus Stange [:mstange]]

Comment on attachment 8977383 [details]
Bug 1462784 - Remove EVENTS category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245220/diff/3-4/

Comment 116

[Markus Stange [:mstange]]

Comment on attachment 8977384 [details]
Bug 1462784 - Rename the CSS category to LAYOUT.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245222/diff/3-4/

Comment 117

[Markus Stange [:mstange]]

Comment on attachment 8977385 [details]
Bug 1462784 - Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245224/diff/4-5/

Comment 118

[Markus Stange [:mstange]]

Comment on attachment 8977386 [details]
Bug 1462784 - Add a DOM category and use it for the WebIDL binding stack labels.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245226/diff/4-5/

Comment 119

[Markus Stange [:mstange]]

Comment on attachment 8977387 [details]
Bug 1462784 - Remove the STORAGE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245228/diff/4-5/

Comment 120

[Markus Stange [:mstange]]

Comment on attachment 8977388 [details]
Bug 1462784 - Relabel various other stuff as being OTHER.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245230/diff/4-5/

Comment 121

[Markus Stange [:mstange]]

Comment on attachment 8977389 [details]
Bug 1462784 - Add an IDLE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245232/diff/4-5/

Comment 122

[Markus Stange [:mstange]]

Comment on attachment 8977390 [details]
Bug 1462784 - Annotate idle stacks in the chromium event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245234/diff/4-5/

Comment 123

[Markus Stange [:mstange]]

Comment on attachment 8977391 [details]
Bug 1462784 - Annotate idle stacks in the main thread event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245236/diff/4-5/

Comment 124

[Markus Stange [:mstange]]

Comment on attachment 8977392 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on macOS.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245238/diff/5-6/

Comment 125

[Markus Stange [:mstange]]

Comment on attachment 8977394 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245242/diff/5-6/

Comment 126

[Markus Stange [:mstange]]

Comment on attachment 8977395 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Linux.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245244/diff/5-6/

Comment 127

[Markus Stange [:mstange]]

Comment on attachment 8980164 [details]
Bug 1462784 - Add idle annotations for DOM worker threads.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246316/diff/4-5/

Comment 128

[Markus Stange [:mstange]]

Comment on attachment 8980165 [details]
Bug 1462784 - Annotate idle stacks in nsThreadPool.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246318/diff/5-6/

Comment 129

[Markus Stange [:mstange]]

Comment on attachment 8977393 [details]
Bug 1462784 - Add the list of categories into the profile JSON and bump the version to 11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245240/diff/6-7/

Comment 130

[Markus Stange [:mstange]]

Comment on attachment 8981200 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247284/diff/5-6/

Comment 131

[Markus Stange [:mstange]]

Comment on attachment 8977381 [details]
Bug 1462784 - Change the enum ProfilingStackFrame::Category from a bitfield to a regular enum.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245216/diff/4-5/

Comment 132

[Markus Stange [:mstange]]

Comment on attachment 8977382 [details]
Bug 1462784 - Merge GC and CC categories into one category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245218/diff/4-5/

Comment 133

[Markus Stange [:mstange]]

Comment on attachment 8977383 [details]
Bug 1462784 - Remove EVENTS category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245220/diff/4-5/

Comment 134

[Markus Stange [:mstange]]

Comment on attachment 8977384 [details]
Bug 1462784 - Rename the CSS category to LAYOUT.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245222/diff/4-5/

Comment 135

[Markus Stange [:mstange]]

Comment on attachment 8977385 [details]
Bug 1462784 - Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245224/diff/5-6/

Comment 136

[Markus Stange [:mstange]]

Comment on attachment 8977386 [details]
Bug 1462784 - Add a DOM category and use it for the WebIDL binding stack labels.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245226/diff/5-6/

Comment 137

[Markus Stange [:mstange]]

Comment on attachment 8977387 [details]
Bug 1462784 - Remove the STORAGE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245228/diff/5-6/

Comment 138

[Markus Stange [:mstange]]

Comment on attachment 8977388 [details]
Bug 1462784 - Relabel various other stuff as being OTHER.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245230/diff/5-6/

Comment 139

[Markus Stange [:mstange]]

Comment on attachment 8977389 [details]
Bug 1462784 - Add an IDLE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245232/diff/5-6/

Comment 140

[Markus Stange [:mstange]]

Comment on attachment 8977390 [details]
Bug 1462784 - Annotate idle stacks in the chromium event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245234/diff/5-6/

Comment 141

[Markus Stange [:mstange]]

Comment on attachment 8977391 [details]
Bug 1462784 - Annotate idle stacks in the main thread event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245236/diff/5-6/

Comment 142

[Markus Stange [:mstange]]

Comment on attachment 8977392 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on macOS.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245238/diff/6-7/

Comment 143

[Markus Stange [:mstange]]

Comment on attachment 8977394 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245242/diff/6-7/

Comment 144

[Markus Stange [:mstange]]

Comment on attachment 8977395 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Linux.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245244/diff/6-7/

Comment 145

[Markus Stange [:mstange]]

Comment on attachment 8980164 [details]
Bug 1462784 - Add idle annotations for DOM worker threads.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246316/diff/5-6/

Comment 146

[Markus Stange [:mstange]]

Comment on attachment 8980165 [details]
Bug 1462784 - Annotate idle stacks in nsThreadPool.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246318/diff/6-7/

Comment 147

[Markus Stange [:mstange]]

Comment on attachment 8977393 [details]
Bug 1462784 - Add the list of categories into the profile JSON and bump the version to 11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245240/diff/7-8/

Comment 148

[Markus Stange [:mstange]]

Comment on attachment 8981200 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247284/diff/6-7/

Comment 149

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2911a626671
Change the enum ProfilingStackFrame::Category from a bitfield to a regular enum. r=njn
https://hg.mozilla.org/integration/autoland/rev/692017229de6
Merge GC and CC categories into one category. r=njn
https://hg.mozilla.org/integration/autoland/rev/1d38a4cf5a4a
Remove EVENTS category. r=njn
https://hg.mozilla.org/integration/autoland/rev/99b4a433a8e5
Rename the CSS category to LAYOUT. r=njn
https://hg.mozilla.org/integration/autoland/rev/106967fc29d2
Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category. r=njn
https://hg.mozilla.org/integration/autoland/rev/0663d1a6d2da
Add a DOM category and use it for the WebIDL binding stack labels. r=njn
https://hg.mozilla.org/integration/autoland/rev/e09c38853172
Remove the STORAGE category. r=njn
https://hg.mozilla.org/integration/autoland/rev/329645ff1e23
Relabel various other stuff as being OTHER. r=njn
https://hg.mozilla.org/integration/autoland/rev/e5ad2e525ea9
Add an IDLE category. r=njn
https://hg.mozilla.org/integration/autoland/rev/6fe79d1ca1bd
Annotate idle stacks in the chromium event loop. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/34e71c789903
Annotate idle stacks in the main thread event loop. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/68eabfbf3c16
Annotate idle stacks in the native event loop on macOS. r=spohl
https://hg.mozilla.org/integration/autoland/rev/d1ca8b0f2221
Annotate idle stacks in the native event loop on Windows. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/a543b94b049a
Annotate idle stacks in the native event loop on Linux. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/71fe35fd1f7e
Add idle annotations for DOM worker threads. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/7880f9dc7023
Annotate idle stacks in nsThreadPool. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/88321efb673b
Add the list of categories into the profile JSON and bump the version to 11. r=njn
https://hg.mozilla.org/integration/autoland/rev/79556798ff9f
Update the devtools performance panel for the new category list. r=gregtatum

Comment 150

[Natalia Csoregi [:nataliaCs]]

Backed out for ESlint failure on FlameGraph.js:1297.

backout: https://hg.mozilla.org/integration/autoland/rev/66f87c551f18e83f5b04f34c42d2feddbe4fd88c

push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=79556798ff9f2fdab1bf1212cd27399d6d5c599d&group_state=expanded

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=181412525&repo=autoland&lineNumber=297

[task 2018-06-01T20:33:22.292Z] New python executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python2.7
[task 2018-06-01T20:33:22.292Z] Also creating executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/init/bin/python
[task 2018-06-01T20:33:24.258Z] Installing setuptools, pip, wheel...done.
[task 2018-06-01T20:33:25.480Z] running build_ext
[task 2018-06-01T20:33:25.480Z] building 'psutil._psutil_linux' extension
[task 2018-06-01T20:33:25.480Z] creating build
[task 2018-06-01T20:33:25.480Z] creating build/temp.linux-x86_64-2.7
[task 2018-06-01T20:33:25.480Z] creating build/temp.linux-x86_64-2.7/psutil
[task 2018-06-01T20:33:25.480Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-06-01T20:33:25.481Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-06-01T20:33:25.481Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o
[task 2018-06-01T20:33:25.481Z] creating build/lib.linux-x86_64-2.7
[task 2018-06-01T20:33:25.481Z] creating build/lib.linux-x86_64-2.7/psutil
[task 2018-06-01T20:33:25.481Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so
[task 2018-06-01T20:33:25.481Z] building 'psutil._psutil_posix' extension
[task 2018-06-01T20:33:25.481Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-06-01T20:33:25.481Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-06-01T20:33:25.481Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so
[task 2018-06-01T20:33:25.481Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
[task 2018-06-01T20:33:25.481Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2018-06-01T20:33:25.481Z] 
[task 2018-06-01T20:33:25.481Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2018-06-01T20:39:47.932Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/shared/widgets/FlameGraph.js:1297:17 | 'category' is never reassigned. Use 'const' instead. (prefer-const)
[taskcluster 2018-06-01 20:39:48.291Z] === Task Finished ===
[taskcluster 2018-06-01 20:39:48.292Z] Unsuccessful task run with exit code: 1 completed in 692.073 seconds

Comment 151

[Markus Stange [:mstange]]

Comment on attachment 8981200 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247284/diff/7-8/

Comment 152

[Mozilla Autoland]

hg error in cmd: hg rebase -s c2911a626671ec41cad3de50db94996e81f739d5 -d 66f87c551f18: abort: source and destination form a cycle

Comment 153

[Markus Stange [:mstange]]

Comment on attachment 8977381 [details]
Bug 1462784 - Change the enum ProfilingStackFrame::Category from a bitfield to a regular enum.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245216/diff/5-6/

Comment 154

[Markus Stange [:mstange]]

Comment on attachment 8977382 [details]
Bug 1462784 - Merge GC and CC categories into one category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245218/diff/5-6/

Comment 155

[Markus Stange [:mstange]]

Comment on attachment 8977383 [details]
Bug 1462784 - Remove EVENTS category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245220/diff/5-6/

Comment 156

[Markus Stange [:mstange]]

Comment on attachment 8977384 [details]
Bug 1462784 - Rename the CSS category to LAYOUT.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245222/diff/5-6/

Comment 157

[Markus Stange [:mstange]]

Comment on attachment 8977385 [details]
Bug 1462784 - Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245224/diff/6-7/

Comment 158

[Markus Stange [:mstange]]

Comment on attachment 8977386 [details]
Bug 1462784 - Add a DOM category and use it for the WebIDL binding stack labels.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245226/diff/6-7/

Comment 159

[Markus Stange [:mstange]]

Comment on attachment 8977387 [details]
Bug 1462784 - Remove the STORAGE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245228/diff/6-7/

Comment 160

[Markus Stange [:mstange]]

Comment on attachment 8977388 [details]
Bug 1462784 - Relabel various other stuff as being OTHER.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245230/diff/6-7/

Comment 161

[Markus Stange [:mstange]]

Comment on attachment 8977389 [details]
Bug 1462784 - Add an IDLE category.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245232/diff/6-7/

Comment 162

[Markus Stange [:mstange]]

Comment on attachment 8977390 [details]
Bug 1462784 - Annotate idle stacks in the chromium event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245234/diff/6-7/

Comment 163

[Markus Stange [:mstange]]

Comment on attachment 8977391 [details]
Bug 1462784 - Annotate idle stacks in the main thread event loop.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245236/diff/6-7/

Comment 164

[Markus Stange [:mstange]]

Comment on attachment 8977392 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on macOS.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245238/diff/7-8/

Comment 165

[Markus Stange [:mstange]]

Comment on attachment 8977394 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Windows.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245242/diff/7-8/

Comment 166

[Markus Stange [:mstange]]

Comment on attachment 8977395 [details]
Bug 1462784 - Annotate idle stacks in the native event loop on Linux.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245244/diff/7-8/

Comment 167

[Markus Stange [:mstange]]

Comment on attachment 8980164 [details]
Bug 1462784 - Add idle annotations for DOM worker threads.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246316/diff/6-7/

Comment 168

[Markus Stange [:mstange]]

Comment on attachment 8980165 [details]
Bug 1462784 - Annotate idle stacks in nsThreadPool.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/246318/diff/7-8/

Comment 169

[Markus Stange [:mstange]]

Comment on attachment 8977393 [details]
Bug 1462784 - Add the list of categories into the profile JSON and bump the version to 11.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245240/diff/8-9/

Comment 170

[Markus Stange [:mstange]]

Comment on attachment 8981200 [details]
Bug 1462784 - Update the devtools performance panel for the new category list.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/247284/diff/8-9/

Comment 171

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fbea249b0521
Change the enum ProfilingStackFrame::Category from a bitfield to a regular enum. r=njn
https://hg.mozilla.org/integration/autoland/rev/45f1cff50efc
Merge GC and CC categories into one category. r=njn
https://hg.mozilla.org/integration/autoland/rev/6dc964ee521c
Remove EVENTS category. r=njn
https://hg.mozilla.org/integration/autoland/rev/3ba0b75a1419
Rename the CSS category to LAYOUT. r=njn
https://hg.mozilla.org/integration/autoland/rev/88a612098ee5
Annotate layout-related stacks with the LAYOUT category instead of the GRAPHICS category. r=njn
https://hg.mozilla.org/integration/autoland/rev/83d77dfa66eb
Add a DOM category and use it for the WebIDL binding stack labels. r=njn
https://hg.mozilla.org/integration/autoland/rev/5bc21aa25ea7
Remove the STORAGE category. r=njn
https://hg.mozilla.org/integration/autoland/rev/a3c9ec2c9d7a
Relabel various other stuff as being OTHER. r=njn
https://hg.mozilla.org/integration/autoland/rev/d539f9f0f7ba
Add an IDLE category. r=njn
https://hg.mozilla.org/integration/autoland/rev/8d812a39d56f
Annotate idle stacks in the chromium event loop. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/4d488c0cdc69
Annotate idle stacks in the main thread event loop. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/400afc8aaee2
Annotate idle stacks in the native event loop on macOS. r=spohl
https://hg.mozilla.org/integration/autoland/rev/7cde6d6997b6
Annotate idle stacks in the native event loop on Windows. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/e40a185033e5
Annotate idle stacks in the native event loop on Linux. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/b2adab83964f
Add idle annotations for DOM worker threads. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/21c3c5fd4fac
Annotate idle stacks in nsThreadPool. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/23e0775d9c9d
Add the list of categories into the profile JSON and bump the version to 11. r=njn
https://hg.mozilla.org/integration/autoland/rev/072ca507a4ba
Update the devtools performance panel for the new category list. r=gregtatum

Comment 172

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/fbea249b0521
https://hg.mozilla.org/mozilla-central/rev/45f1cff50efc
https://hg.mozilla.org/mozilla-central/rev/6dc964ee521c
https://hg.mozilla.org/mozilla-central/rev/3ba0b75a1419
https://hg.mozilla.org/mozilla-central/rev/88a612098ee5
https://hg.mozilla.org/mozilla-central/rev/83d77dfa66eb
https://hg.mozilla.org/mozilla-central/rev/5bc21aa25ea7
https://hg.mozilla.org/mozilla-central/rev/a3c9ec2c9d7a
https://hg.mozilla.org/mozilla-central/rev/d539f9f0f7ba
https://hg.mozilla.org/mozilla-central/rev/8d812a39d56f
https://hg.mozilla.org/mozilla-central/rev/4d488c0cdc69
https://hg.mozilla.org/mozilla-central/rev/400afc8aaee2
https://hg.mozilla.org/mozilla-central/rev/7cde6d6997b6
https://hg.mozilla.org/mozilla-central/rev/e40a185033e5
https://hg.mozilla.org/mozilla-central/rev/b2adab83964f
https://hg.mozilla.org/mozilla-central/rev/21c3c5fd4fac
https://hg.mozilla.org/mozilla-central/rev/23e0775d9c9d
https://hg.mozilla.org/mozilla-central/rev/072ca507a4ba