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

Optimize Levenshtein implementation #427

Merged
merged 2 commits into from
Jun 5, 2015
Merged

Conversation

megawac
Copy link
Contributor

@megawac megawac commented Jun 3, 2015

This implementation is directly from https://github.com/hiddentao/fast-levenshtein

Before this change:

[21:17:45] levenshtein x 25,172 ops/sec ±0.70% (98 runs sampled)

After:

[21:09:18] levenshtein x 141,238 ops/sec ±0.95% (97 runs sampled)

Relevant jsperf: http://jsperf.com/levenshtein-distance/22 (I experimented with some of the others but this one had by far the best performance in node)

var curCol = nextCol;

// substution
nextCol = prevRow[j] + ( (str1.charAt(i) === str2.charAt(j)) ? 0 : 1 );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ternary is optional here; coercing to a boolean will work as well

@megawac
Copy link
Contributor Author

megawac commented Jun 4, 2015

/cc @stoeffel any chance of getting this in and releasing a 3.0.4?

@stoeffel
Copy link
Collaborator

stoeffel commented Jun 4, 2015

@megawac sure I will have a look at this tomorrow and publish a new release.

@stoeffel
Copy link
Collaborator

stoeffel commented Jun 4, 2015

how about we link to hiddentao (in the readme) since this is directly from his implementation?

@stoeffel
Copy link
Collaborator

stoeffel commented Jun 5, 2015

otherwise LGTM. I will check it once again this evening and then hopefully have time to pump the version.

stoeffel added a commit that referenced this pull request Jun 5, 2015
Optimize Levenshtein implementation
@stoeffel stoeffel merged commit d8d37f0 into esamattis:master Jun 5, 2015
@stoeffel
Copy link
Collaborator

stoeffel commented Jun 5, 2015

@megawac I published 3.1.0

@megawac
Copy link
Contributor Author

megawac commented Jun 5, 2015

Sweetness thanks!

@megawac megawac deleted the levenshtein branch June 5, 2015 16:24
@stoeffel
Copy link
Collaborator

stoeffel commented Jun 5, 2015

Just had to publish again since I accidentally pushed the coverage folder to npm.

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.

2 participants