1686358 - Hit testing interaction with the APZ lock can be expensive

Status: NEW

(Core :: Graphics: WebRender, defect, P3)

Description

[Markus Stange [:mstange]]

See this profile from bug 1684482: https://share.firefox.dev/3bvCPKo

A busy render backend thread still seems to have the ability to delay mouse events. Nical told me that we were supposed to do hit testing on a read-only thread-safe copy of a hit testing tree. This mechanism doesn't seem to be working.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

https://searchfox.org/mozilla-central/rev/07342ce09126c513540c1c343476e026cfa907bf/gfx/webrender_bindings/src/bindings.rs#3848 is where the hit-test happens on the threadsafe copy. I don't think that is the problem here.

From the profile I'm guessing it's stuck on this lock acquisition, which can happen if the scene builder thread has grabbed the tree lock via this call and has not yet unlocked it via the corresponding unlock. Those lock/unlock calls get triggered from here and so the unlock can be delayed by this blocking recv call. My guess is that's what's happening here - the render backend thread is busy doing things and so the scene swap can't complete quickly and that causes the backup.

Comment 2

[Markus Stange [:mstange]]

Thanks! So, what's the right thing to do here? Make it so that scene swapping doesn't involve the render backend thread?

Comment 3

[Kartikaya Gupta (email:[e-mail])]

I'm not really sure, to be honest. The thread locking there is carefully done to ensure correctness during hit testing, and I don't think it will be easy to change. If we can somehow block earlier on in forward_built_transactions (before the pre-swap hook) until the RB thread is free that would accomplish the goal here but I don't know how feasible that is.