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

Cache currentTime and buffered from Flash #3705

Merged
merged 1 commit into from
Nov 4, 2016
Merged

Conversation

dmlap
Copy link
Member

@dmlap dmlap commented Oct 23, 2016

Description

Calling into the SWF too often is expensive. Current time and buffered don't actually change that often but it's very common to call them a couple times in the handling of a single event. Cache their return values for 100ms so the performance penalty of going through ExternalInterface is limited.

Specific Changes proposed

Cache return values from currentTime and buffered for 100ms in the Flash tech.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
  • Reviewed by Two Core Contributors

Calling into the SWF too often is expensive. Current time and buffered don't actually change that often but it's very common to call them a couple times in the handling of a single event. Cache their return values for 100ms so the performance penalty of going through ExternalInterface is limited.
return result.cache_;
};

result.lastCheckTime_ = -Infinity;
Copy link
Member

Choose a reason for hiding this comment

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

When you want to be absolutely certain! 😆

* @return {Function} a new function that returns cached results if
* invoked multiple times within the cache duration
*/
export default function timeExpiringCache(getter, cacheDuration) {
Copy link
Member

Choose a reason for hiding this comment

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

Since both uses of this function pass 100 here, I wonder if having a default (i.e. cacheDuration = 100) makes sense?

@gkatsev gkatsev added this to the 5.13 milestone Oct 26, 2016
@gkatsev gkatsev merged commit 45ffa81 into videojs:master Nov 4, 2016
@gkatsev gkatsev deleted the flash-cache branch November 4, 2016 22:03
gkatsev added a commit that referenced this pull request Nov 14, 2016
gkatsev added a commit that referenced this pull request Nov 14, 2016
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.

3 participants