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

Avoid allocating functions and use at most one timer per interval in throttleLeadingAndTrailing #5476

Merged

Conversation

mrxz
Copy link
Contributor

@mrxz mrxz commented Feb 27, 2024

Description:
The implementation of throttleLeadingAndTrailing was very inefficient when many updates took place in short succession. Virtually all invocations would result in clearing a timer, allocating a new function and creating a new timer. This impacted both the performance and memory usage of setAttribute.

Using the performance/set-attribute and performance/animation-set-attribute benchmarks showed a clear performance benefit (when congested) cutting the duration in half (~9.16µs/op -> ~4.04µs/op) and reduced memory consumption:
image

Instead of allocating a new function for each timer, one is created in advance. In case calls take place within the minimumInterval only one timer is created and re-used until it expires ensuring at most one timer per interval (if needed).

Changes proposed:

  • Pre-allocate timeout callback function
  • Avoid clearing and recreating timeout and instead create and re-use one timeout per interval (if needed)

@mrxz mrxz force-pushed the optimize-throttle-leading-and-trailing branch from aae9b6c to 3449ca7 Compare February 27, 2024 12:52
src/utils/index.js Outdated Show resolved Hide resolved
}, minimumInterval - sinceLastTime);
// Inside minimum interval, create timer if needed.
if (typeof deferTimer === 'undefined') {
deferTimer = setTimeout(timerExpired, minimumInterval - sinceLastTime);
Copy link
Member

@dmarcos dmarcos Feb 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can do a single line

deferTimer = deferTimer || setTimeout(timerExpired, minimumInterval - sinceLastTime);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@mrxz mrxz force-pushed the optimize-throttle-leading-and-trailing branch from 3449ca7 to 4c45497 Compare February 27, 2024 22:25
@dmarcos
Copy link
Member

dmarcos commented Mar 18, 2024

Thanks!

@dmarcos dmarcos merged commit fe238e7 into aframevr:master Mar 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants