Closed Bug 1487564 Opened Last year Closed 6 months ago

Delayed response initiating APZ scroll with WebRender

Categories

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

defect

Tracking

()

RESOLVED WORKSFORME

People

(Reporter: bholley, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 obsolete file)

I've noticed a subtle difference on Quantum reference devices when initiating scrolls with Fx vs FxWR. Complexity of the page doesn't matter, I'm testing on [1] (just because it's linked to from example.com, which isn't itself big enough to scroll).

When I initiate a scroll by doing a large (but brief) pulse-ish swipe on the trackpad, FxWR has a noticeable delay before the scroll gets started, whereas Fx seems to start instantly. Once it gets underway, the scroll itself feels smooth in either case, including the situation where I keep my fingers on the trackpad and apply a more complicated scroll pattern.

I took profiles of both cases:
Fx: https://perfht.ml/2Nuw9xc
FxWR: https://perfht.ml/2NvRmqp

One thing that jumps out at me is the duration of the composite markers on the Renderer thread. There are some short ones, and then a segment where they all seem almost exactly frame-length, with most of the CPU time under WaitForPreviousPresentQuery. In stock Fx all the composites are short.

I'm not sure what that means, whether it's a problem, and whether it's related to the visual glitch I see. It does seem plausible that the transition from short to long composites could correspond with the end of the delay interval, but I'm not sure how to prove that.

This is on a Quantum reference device. It doesn't happen on my Mac, which may be OS-related, or related to the fact that it's a faster device.

[1] https://www.iana.org/domains/reserved
Blocking for the duration of a vsync interval suggests that we're probably waiting on queued work, rather than the work taking exactly that duration of time.

Maybe we're submitting too many frames and it's waiting on them? My reading of the DXGI swap chain docs (and the FLIP_DISCARD mode we're using) suggest that this isn't supposed to happen though.

Does setting gfx.webrender.dcomp-win.enabled=false (needs restart) change anything here?

I can't reproduce this on my machine.
(In reply to Matt Woodrow (:mattwoodrow) from comment #1)
> Does setting gfx.webrender.dcomp-win.enabled=false (needs restart) change
> anything here?

Yes: When I flip it, the compositor timers look more like what I expect: https://perfht.ml/2wz2f3I

I'm short on time so I can't quite evaluate whether the perceptual effect is there or not. Will try to do that.
By the way, if gfx.webrender.dcomp-win.enabled is false, it hit Bug 1453991(performance problem).
(In reply to Bobby Holley (:bholley) from comment #2)
> I'm short on time so I can't quite evaluate whether the perceptual effect is
> there or not. Will try to do that.

I can confirm that disabling dcomp fixes perceptual lag reported in comment 0. I think we still lack certainty that the lag is caused by the issue in comment 1, but it seems increasingly likely.
Can you please check to see if the try build in bug 1486958 helps?
(In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> Can you please check to see if the try build in bug 1486958 helps?

Tried with [1], no apparent effect (dcomp enabled, per default).

[1] https://queue.taskcluster.net/v1/task/RJJHlt9QRqCyBPNq14_Njw/runs/0/artifacts/public/build/target.zip
(In reply to Bobby Holley (:bholley) from comment #6)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> > Can you please check to see if the try build in bug 1486958 helps?
> 
> Tried with [1], no apparent effect (dcomp enabled, per default).

To be clear, that means I still see the bug as filed here.
(In reply to Bobby Holley (:bholley) from comment #6)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> > Can you please check to see if the try build in bug 1486958 helps?
> 
> Tried with [1], no apparent effect (dcomp enabled, per default).
> 
> [1]
> https://queue.taskcluster.net/v1/task/RJJHlt9QRqCyBPNq14_Njw/runs/0/
> artifacts/public/build/target.zip

That bug is meant to be making the dcomp pref default to false, and enabling a flip model swap chain for the non dcomp path.

Can you please test with dcomp disabled to see if that works as expected?
(In reply to Matt Woodrow (:mattwoodrow) from comment #8)
> (In reply to Bobby Holley (:bholley) from comment #6)
> > (In reply to Matt Woodrow (:mattwoodrow) from comment #5)
> > > Can you please check to see if the try build in bug 1486958 helps?
> > 
> > Tried with [1], no apparent effect (dcomp enabled, per default).
> > 
> > [1]
> > https://queue.taskcluster.net/v1/task/RJJHlt9QRqCyBPNq14_Njw/runs/0/
> > artifacts/public/build/target.zip
> 
> That bug is meant to be making the dcomp pref default to false, and enabling
> a flip model swap chain for the non dcomp path.
> 
> Can you please test with dcomp disabled to see if that works as expected?

Yeah sorry - pref was at default, and I didn't realize the default had changed with your patches. The measurements in comment 6 were with the default (dcomp off).
Interesting, that suggests that the problem is with flipping, and not DComp.

Does gfx.direct3d11.use-double-buffering=true make it happen with non-WR?
(In reply to Matt Woodrow (:mattwoodrow) from comment #10)
> Interesting, that suggests that the problem is with flipping, and not DComp.
> 
> Does gfx.direct3d11.use-double-buffering=true make it happen with non-WR?

It does not.
Priority: -- → P2
Priority: P2 → P3
Priority: P3 → P2
Assignee: nobody → sotaro.ikeda.g
:bholley, do you still see the problem?
Flags: needinfo?(bobbyholley)
I do, at least on the quantum reference laptop. Let me know if you need me to boot into windows on my nvidia desktop and check there.
Flags: needinfo?(bobbyholley)
Thanks for the confirmation!
(In reply to Bobby Holley (:bholley) from comment #11)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #10)
> > Interesting, that suggests that the problem is with flipping, and not DComp.
> > 
> > Does gfx.direct3d11.use-double-buffering=true make it happen with non-WR?
> 
> It does not.

non-WR uses less GPU, it might cause the difference between with WR and with non-WR.
One possible fix is to wait previous frame complete after posting NotifyDidRender. The wait of previous frame delays next transaction on content side. If NotifyDidRender is posted before waiting previous frame complete, content side could start next painting earlier.
:bholley, can you check if attachment 9017772 [details] [diff] [review] address the problem for you?
Flags: needinfo?(bobbyholley)
(In reply to Sotaro Ikeda [:sotaro] from comment #19)
> :bholley, can you check if attachment 9017772 [details] [diff] [review]
> address the problem for you?

The try build from comment 18 does not fix either the delay or the profiler characteristics noted in comment 0.

I think it's probably not efficient to try to fix this by guessing, so we should get you the actual device. That said, I can't reproduce this on my nvidia desktop, so it may be specific to Intel. If we don't have evidence that this affects nvidia, this shouldn't block the MVP.
Blocks: stage-wr-next
No longer blocks: stage-wr-trains
Flags: needinfo?(bobbyholley)
Thanks for checking. Yea, attachment 9017772 [details] [diff] [review] seem not efficient.
Attachment #9017772 - Attachment is obsolete: true
Depends on: 1500017
With bug 1500017, the long composite markers from comment 0 are gone. Nice work Sotaro!

Unfortunately, I can still reproduce the scroll delay. Oddly, I can only reproduce it with the profiler disabled. I wonder if it's an issue of getting swapped off a core, and the profiler thread wakeups keep the application sufficiently warm?

In any case, this doesn't block the MVP, so we should stop worrying about it for now and come back to it later.
(In reply to Bobby Holley (:bholley) from comment #22)
> Unfortunately, I can still reproduce the scroll delay. Oddly, I can only
> reproduce it with the profiler disabled.

This might be caused by the high-resolution timer mode that the profiler requests when it's running:
https://searchfox.org/mozilla-central/rev/b096dcf0ea226af628fe03f7e7acb56a25853533/tools/profiler/core/platform-win32.cpp#214

Bug 1495208 comment 12 has reported a similar effect on non-WR.
(In reply to Markus Stange [:mstange] from comment #23)
> (In reply to Bobby Holley (:bholley) from comment #22)
> > Unfortunately, I can still reproduce the scroll delay. Oddly, I can only
> > reproduce it with the profiler disabled.
> 
> This might be caused by the high-resolution timer mode that the profiler
> requests when it's running:
> https://searchfox.org/mozilla-central/rev/
> b096dcf0ea226af628fe03f7e7acb56a25853533/tools/profiler/core/platform-win32.
> cpp#214
> 
> Bug 1495208 comment 12 has reported a similar effect on non-WR.

The behavior is the same with a profiler resolution of 100ms, so the situation as-described there wouldn't be the cause, but it might be related somehow.
I rethink about attachment 9017772 [details] [diff] [review] and concluded that it is a valid solution. Then created Bug 1506091.

The WaitForPreviousPresentQuery() is not necessary for the NotifyDidRender. The NotifyDidRender is called for current webrender rendering, that is posted by "mSwapChain->Present(0, 0)". The WaitForPreviousPresentQuery() does wait for gpu tasks of 2 previous webrender rendering. It is necessary for Textures recycling of AsyncImagePipelineManager and for avoiding GPU queue is filled with too much tasks.
Depends on: 1506091
Assignee: sotaro.ikeda.g → nobody
Depends on: 1525183

I just retested this with bug 1525183, and things seem a lot better. Sotaro, would you expect that bug to fix this?

(In reply to Bobby Holley (:bholley) from comment #26)

I just retested this with bug 1525183, and things seem a lot better. Sotaro, would you expect that bug to fix this?

yea, I think that bug 1525183 made the scrolling better. Further, since Bug 1528865 fix, scrolling becomes more smooth on intel pc.

Depends on: 1528865

Ok, should we close this bug then?

Flags: needinfo?(sotaro.ikeda.g)

Yes, thanks!

Status: NEW → RESOLVED
Closed: 6 months ago
Flags: needinfo?(sotaro.ikeda.g)
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.