perf(virtual-scroll): use auditTime instead of sampleTime

[nename0]

The CdkVirtualScrollViewport uses sampleTime to collect multiple scroll events into one until the next animation frame.
However the sampleTime also samples even if there is no new upstream values. Which in this case creates an basically useless requestAnimationFrame callback.
A better choice would be the auditTime operator, which only schedules the requestAnimationFrame when there is actually a new upstream value.
In our case it has the exact same behavior, as from looking at the performance profiler in chrome I found out this: first the scroll event fires, then pending requestAnimationFrame callbacks get called, regardless if they where scheduled in the current or previous frame and then the paint occurs.

[mmalerba]

I'll have to take a thorough look at this, I seem to remember intentionally picking sampleTime after comparing them for some reason

[julianobrasil]

@mmalerba, I think you're referring to this #10179 (comment)

[nename0]

Interestingly throttleTime has a config parameter to define, if it emits before (leading) the delay, after (trailing) or both. When you set it to only trailing its equivalent to sampleTime and it might actually be a bit faster as it does not create a timer Observable each time. It did not choose this to keep things simple.

[nename0]

I was wrong. throttleTime with trailing differs from auditTime:

1. if if there is only one value in a delay it will not emit (possibly a bug throttle and throttleTime broken for single values when trailing is true. ReactiveX/rxjs#2859)
   So in our case auditTime is the best option
2. it will emit on complete

So in out case the best choice is auditTime.

[mmalerba]

@julianobrasil Thanks, that's what I was thinking of.

@nename0 I agree that auditTime does seem like it would be more efficient here. Let me patch this PR and check the demos, will LGTM if they all seem to work right

[ngbot]

I see that you just added the pr: merge ready label, but the following checks are still failing:
     missing required label: "target: *"

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

[nename0]

Thanks!

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}