Angular framework performance test spends 10% in nursery GC

NEW
Unassigned

Status

()

Core
JavaScript: GC
2 years ago
3 months ago

People

(Reporter: till, Unassigned)

Tracking

(Blocks: 1 bug, {perf})

Trunk
Points:
---

Firefox Tracking Flags

(firefox50 affected)

Details

(Whiteboard: [qf:p3])

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
Running the Angular part of [1], I see a lot of time spent under js::Nursery::Collect - roughly 10%, reliably on each run. That seems like an awfully long time for nursery collections. Looking at what triggers the GCs, it seems like event dispatching is involved.

[1] https://chrisharrington.github.io/demos/performance/

Comment 1

2 years ago
bug 1237058 might be related.
Event dispatch (or setTimeout or animation frame callbacks or Promise callbacks) tend to trigger everything in browser.

Comment 2

2 years ago
I see some minorGCs but nothing there hints event dispatching is causing it.
And profiles do look quite different to bug 1237058 after all.

till, if you have profiles hinting that event dispatching might be causing some issues here, want to upload the profiles somewhere?
(Reporter)

Comment 3

2 years ago
Created attachment 8763378 [details]
angular-profile.json

> till, if you have profiles hinting that event dispatching might be causing
> some issues here, want to upload the profiles somewhere?

The reason I thought events might be related is because in a profile using the built-in profiler, an inverted call tree shows js::Nursery::collect as having EventDispatcher::Dispatch as its most-frequent caller. See the attached profile.

I'm not sure I'm reading the profile right, so I could very well be wrong about the connection.

Comment 4

2 years ago
I'm not familiar using builtin profiler, but if I interpret that right, it is just that there is an event listener for click event, and the listener then just triggers the test to run, so naturally that is where all the minorGCs happen, underneath click event dispatch.

(I use Zoom on linux for profiling, but naturally it doesn't understand JITed JS code more than time is spent there)

Btw, is the profile done using Nightly? If so, with async stacks enabled? They are enabled by default on non-release builds, like Nightly. pref name is javascript.options.asyncstack
Async stacks slow down everything a lot. We really should disable them by default.
(Reporter)

Comment 5

2 years ago
Created attachment 8763387 [details]
angular-profile-no-async-stacks.json

(In reply to Olli Pettay [:smaug] (high review load, please consider other reviewers) from comment #4)
> I'm not familiar using builtin profiler, but if I interpret that right, it
> is just that there is an event listener for click event, and the listener
> then just triggers the test to run, so naturally that is where all the
> minorGCs happen, underneath click event dispatch.

That makes sense.

> Btw, is the profile done using Nightly? If so, with async stacks enabled?
> They are enabled by default on non-release builds, like Nightly. pref name
> is javascript.options.asyncstack

I did test with and without; for this particular test it didn't make too much of a difference. See the new profile.

> Async stacks slow down everything a lot. We really should disable them by
> default.

I agree: they're definitely too slow, and it doubtlessly costs us developer mindshare to have them enabled in builds that webdevs (hopefully) use.
Created attachment 8766073 [details]
screenshot of profile

Here's a screenshot so others don't need to import the profile themselves.

Till's reading of the profile matches my own: event dispatch seems to be pretty hard on the nursery here.

Regarding async stacks: it would be nice to speed them up and it would also be nice to only use them when devtools are open.

Comment 7

2 years ago
(In reply to Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] from comment #6)
> Till's reading of the profile matches my own: event dispatch seems to be
> pretty hard on the nursery here.
Not event dispatch, but stuff under it ;)
Click event just happens to trigger the testcase.
(In reply to Olli Pettay [:smaug] (high review load, please consider other reviewers) from comment #7)
> (In reply to Nick Fitzgerald [:fitzgen] [⏰PDT; UTC-7] from comment #6)
> > Till's reading of the profile matches my own: event dispatch seems to be
> > pretty hard on the nursery here.
> Not event dispatch, but stuff under it ;)
> Click event just happens to trigger the testcase.

Do we have a heuristic where we empty the nursery before/after dispatching an event?

Otherwise, if the stack really does look like this:

  js::Nursery::collect
  ...
  JS event handler function
  EventDispatcher::Dispatch
  ...

Then I would expect to see the JS event handler function's frame to be in between js::Nursery::collect and EventDispatcher::Dispatch.
Whiteboard: [qf:p3]
You need to log in before you can comment on or make changes to this bug.