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

Bidi class optimization #68

Closed
wants to merge 1 commit into from
Closed

Bidi class optimization #68

wants to merge 1 commit into from

Conversation

edkimmel
Copy link

@edkimmel edkimmel commented Sep 1, 2015

bidi_class_for as it stands now is very slow. A linear search and the multiple parseInt calls per character are to blame.

I ended up fixing the generated file directly for my own application. I've created a pull request with that file to show the changes that would ultimately have to happen when generating the javascript files.

The bidi_classes object is now a sorted array of ranges, and the bidi_class_for method performs a binary search to find the correct range.

…he current bidi_class_for function that improves performance drastically. With this change, I am able to use this library in a realtime map application.
@KL-7
Copy link
Contributor

KL-7 commented Sep 1, 2015

Thanks for figuring this one out, @edkimmel!

As you noted yourself, this file is in fact generated. The original CoffeeScript code can be found over here with the data generated in Ruby over here. Do you think you could apply your changes to the original implementation so we can regenerate all JS files from it?

@ghost
Copy link

ghost commented Oct 18, 2015

Thanks, yes, the file is generated. @KL-7 Thanks for CoffeeScript code.

mikemorris added a commit to mapbox/mapbox-gl-js that referenced this pull request Dec 16, 2015
@camertron
Copy link
Collaborator

@edkimmel @montanamendy Any progress getting this added to the Coffeescript?

@CLAassistant
Copy link

CLAassistant commented Jul 29, 2019

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Edward Kimmel seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@edkimmel edkimmel closed this May 23, 2022
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