1779763 - Too much time spent in ulock_wait() on 10.15

Status: RESOLVED WORKSFORME

(Core :: Memory Allocator, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

https://share.firefox.dev/3c58vbJ

We're spending 59% of the time on the renderer in ulock_wait()

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1670885

Comment 2

[Julien Wajsberg [:julienw]]

hey Jeff, to be able to properly triage this bug, we need some more information:

• Does that reproduce easily?
• On which websites?
• On which types of workloads? Our understanding is that it's affecting animations, is that it?
  Thanks!

Comment 3

[Jeff Muizelaar [:jrmuizel]]

I don't know how easy it is to get into this state. I don't think I'm seeing it now. It didn't seem to be website specific and affected everything.

Comment 4

[Donal Meehan [:dmeehan]]

Setting 103 to Won't Fix
Anything else that could be done to investigate or are there any pref metrics/telemetry around this?

Comment 5

[Gabriele Svelto [:gsvelto]]

Is this something that's happening only on 10.15? If it isn't affecting macOS 11+ on x86-64 then we could keep using the old spinlocks on 10.15. Also did you measure a significant performance difference? The reason I'm asking is that with the old locks you wouldn't see a contended lock actually being contended, because the implementation would keep yielding to another thread if the lock was taken, which could land on another thread which also didn't own the lock and yield and so on. So the time waiting for the lock to be released would be spread over a bunch of threads continuously waking up and yielding. That is, it's possible that we already had contention here but it wasn't visible.

Comment 6

[Julien Wajsberg [:julienw]]

(In reply to Donal Meehan [:dmeehan] from comment #4)

Setting 103 to Won't Fix
Anything else that could be done to investigate or are there any pref metrics/telemetry around this?

I'm setting the performance priority for this bug using Jeff's answer above and the performance calculator, but otherwise leaving this up to Gabriele to decide.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1670885

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:glandium, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Maybe gab knows more (in pto currently)

Comment 10

[Gabriele Svelto [:gsvelto]]

I've left some info in comment 5, I'd be happy to look at the profile with a reproducible test case. In general if we're spending too much time in memory allocation locks we might have to further profile and optimize our memory allocator (or use a new one, but that's a tall order in terms of complexity).

Comment 11

[Mike Hommey [:glandium]]

I'm going to assume the recentish improvements around locking made things better here. If not, please reopen with a new profile or file a new bug.