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

Zone._index optimization: binary search for closest "until" #720

Merged
merged 1 commit into from
May 21, 2023

Conversation

dashie
Copy link
Contributor

@dashie dashie commented Jan 28, 2019

No description provided.

@dashie dashie mentioned this pull request Jan 28, 2019
@mattjohnsonpint
Copy link
Contributor

Hi @dashie. Sorry I was tuned out for a while.

You basically hit the same problem I just stumbled upon, in #743 after updating things for the latest IANA version.

Would you be so kind to rebase this off the current release?

Thanks.

@mattjohnsonpint mattjohnsonpint self-assigned this Apr 18, 2019
@mattjohnsonpint
Copy link
Contributor

... Or rather, can you separate the code changes out so I can do a full build after merging?

Thanks.

@dashie dashie force-pushed the develop branch 2 times, most recently from 6d5b091 to fa5c4b6 Compare April 20, 2019 03:36
@dashie
Copy link
Contributor Author

dashie commented Apr 20, 2019

I did it, but there are always issues with IANA version.
Anyway I've cleaned and rebased my history (dashie@fa5c4b6) so it is simple for you to look into the changes

@ichernev
Copy link
Contributor

Ooh, binary search, nice. I hope to merge this soon.

@dashie
Copy link
Contributor Author

dashie commented Dec 9, 2022

@ichernev it is merged now? Do you need any other stuff from me?

@dashie
Copy link
Contributor Author

dashie commented May 4, 2023

@ichernev @mattjohnsonpint any news on this merge?

@mattjohnsonpint
Copy link
Contributor

@gilmoreorless

@gilmoreorless
Copy link
Member

Ah sorry I missed this one as I thought it was already being handled by others. I'm all for performance improvements so I'll take a look very soon. 😄

@gilmoreorless
Copy link
Member

Sorry for the delay on this, @dashie. I'm still catching up on the backlog of cleanup tasks for this project.

I've finally found some time to look at this properly. My rudimentary performance tests suggest that for a zone with a lot of transitions (like Europe/London), this is 2–5 times faster, depending on which timestamp is being checked. Nicely done!

@gilmoreorless gilmoreorless merged commit ccaf698 into moment:develop May 21, 2023
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.

4 participants