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

Add opt to use existing loc obj in weekdays/months #877

Merged
merged 2 commits into from
May 8, 2021

Conversation

dan-overton
Copy link
Contributor

Potential fix for #775 .

This allows an existing loc object to be passed into Info months and weekdays functions, to be used instead of creating a new one. The use of these in DateTime has been modified to use this option.

In a loop of 100000 uses of DateTime.weekdayLong this appears to be roughly twice as fast as creating a Locale each time.

@icambron
Copy link
Member

icambron commented Mar 2, 2021

Good stuff. Can you add something to the benchmarks code?

@dan-overton
Copy link
Contributor Author

Sure, will do

@dan-overton
Copy link
Contributor Author

I've added some benchmarks. The benchmarks needed to be at the Info level to compare running with / without an existing locale object. That led to me refactoring the benchmarks themselves to run multiple files. Let me know what you think - I'm happy to roll it back a bit if there's a simpler way that works.

@icambron
Copy link
Member

icambron commented May 8, 2021

Seems fine. Thanks!

@icambron icambron merged commit 3d74e60 into moment:master May 8, 2021
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.

None yet

2 participants