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

Fix performance by caching Calendar.autoupdatingCurrent #221

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

UberJason
Copy link
Contributor

Hi @MatthewYork! I recently discovered a performance problem when performing a lot of date calculations in a row, which boils down to the fact that initializing Calendar.autoupdatingCurrent is a somewhat expensive operation. This PR simplifies things by simply keeping a static copy of Calendar.autoupdatingCurrent on Date and using it everywhere. I had an operation that was taking 8 seconds drop to <1 second with this fix.

Jason Ji and others added 4 commits March 29, 2017 11:52
…, avoiding constantly reinstantiating it (which is a performance hit when performing multiple consecutive Date manipulations)
…d reading an incorrect number of hours apart.
@UberJason
Copy link
Contributor Author

Hi @MatthewYork, thought I should call attention to this again. It's a pretty simple but performance-increasing fix. I accidentally included these commits in my PR #256 (now closed) for a different issue.

… but the day component is 2 apart. (Example: April 19, 10PM and April 21, 2PM)
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.

1 participant