-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Implement performance ticks for framerate. #1420
Conversation
this.collect(); | ||
console.info('visible') | ||
} else { | ||
this.win.cancelAnimationFrame(this.requestedFrame_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (this.win.cancelAnimationFrame) {...}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this.requestedFrame_
can also be null
here. Not sure if that would be a problem here.
Looks good. I think just couple of reset conditions need fixing. |
* to avoid having to request and cancel all the time. | ||
* @private {boolean} | ||
*/ | ||
this.frameScheduled_ = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we just use this.requestedFrame_ != null
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep
This is now ready for review. |
LGTM on my side. Leaving final LGTM to @erwinmombay |
this.reset_(); | ||
this.collect(); | ||
} else { | ||
this.reset_(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove the else conditional. Always #reset_
, and call #collect
if isActive_
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed :)
@cramforce LGTM, pending @jridgewell recommendation |
Calculates the rate by counting animation frames over a timespan. We only measure when we rendered an element or the user scrolled, and then for 5 seconds. This is basically a heuristic for "something interesting might be happening". Otherwise on mostly empty pages the framerate is always 60fps.
Implement performance ticks for framerate.
Calculates the rate by counting animation frames over a timespan.
We only measure when we rendered an element or the user scrolled, and then for 5 seconds. This is basically a heuristic for "something interesting might be happening". Otherwise on mostly empty pages the framerate is always 60fps.