Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

animationFrameScheduler - Execution not syncing with requestAnimationFrame #5397

Closed
Achilles1515 opened this issue Apr 19, 2020 · 4 comments
Closed

Comments

@Achilles1515
Copy link

Achilles1515 commented Apr 19, 2020

Bug Report

See this StackBlitz example, where I am testing different methods of using animationFrameScheduler to (hopefully) execute code on every requestAnimationFrame.

The following methods are as follows:

/** REAL */
requestAnimationFrame(raf);
function raf() {
  real++;
  requestAnimationFrame(raf)
}
/////////////

/** FIRST */
of(0, animationFrameScheduler)
  .pipe(repeat())
  .subscribe(() => {
    first++;
  });

/** SECOND */
interval(0, animationFrameScheduler)
  .subscribe(() => {
    second++;
  });

/** THIRD */
scheduled(of(0), animationFrameScheduler)
  .pipe(repeat())
  .subscribe(() => {
    third++;
  });

/** FOURTH */
animationFrameScheduler.schedule(function() {
  fourth++;
  this.schedule();
}, 0, 0);

When running these all at the same time, the console log outputs of the counters after 1 second are as follows:

REAL:	59
FIRST:	36
SECOND:	71
THIRD:	35
FOURTH:	70

Why do these not all match the "real" value of 59?

Next, comment out the first method. Console output:

REAL:	60
FIRST:	0
SECOND:	80
THIRD:	40
FOURTH:	80

Why do the second, third, and fourth outputs here not "match" the above run?

Next, comment out the first, second, and third methods. Console output:

REAL:	60
FIRST:	0
SECOND:	0
THIRD:	0
FOURTH:	118

(this fourth method is a copy of the animationFrameScheduler example from the official docs)

Why is this running almost twice as often as requestAnimationFrame? Probably related to #4972

In general with all these "comment out" examples, why is the number of schedules running affecting these execution counts?

Environment

  • Runtime: Chrome 80
  • RxJS version: 6.5.5
@cartant
Copy link
Collaborator

cartant commented Apr 21, 2020

The fundamental problem is that the animationFrameScheduler is not a loop - like the REAL example in your StackBlitz - rather, the scheduler queues the scheduled actions and ensures that they run on an animation-frame boundaries, so to speak.

Evidently, there is some overhead in the scheduler implementation. AFAICT, a significant amount of this overhead is due to the bug raised in #4972 is also seen here - as you've mentioned - and I have some idea about what might causing it.

However, for loops like the one in the REAL example, the (relatively) new animationFrames observable should be used. (Be aware, however, that there is an open issue to bring that observable's emitted times into line with the requestAnimationFrame API - see #5194.)

cartant added a commit to cartant/rxjs that referenced this issue Apr 21, 2020
benlesh pushed a commit that referenced this issue Apr 22, 2020
…ush (#5399)

* test: add failing animationFrameScheduler test

* test: add failing asapScheduler test

* fix: don't execute rescheduled animation frame actions

Closes #4972 and #5397

* fix: don't execute rescheduled asap actions
@Achilles1515
Copy link
Author

Thanks for the comment. This is good info to know.
I'll close this issue for now.

@cartant
Copy link
Collaborator

cartant commented Apr 30, 2020

FWIW, most (almost all?) of the overhead would have been effected by the other bug you referenced - which was fixed in 33c9c8c. But you would still be better off using animationFrames for a loop.

@Achilles1515
Copy link
Author

Achilles1515 commented Apr 30, 2020

But you would still be better off using animationFrames for a loop.

I noticed that "double execution" bug in the Angular CDK drag-drop project in the code handling auto-scrolling during a drag.

https://github.com/angular/components/blob/master/src/cdk/drag-drop/drop-list-ref.ts#L778

Looks like they are referencing RxJS 6.5.3 with that project (so can't use animationFrames). I was planning on putting in a PR to switch to a "regular" requestAnimationFrame loop instead.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants