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

Finding property with minimal string distance is slow #1183

Closed
schuay opened this issue Aug 13, 2018 · 2 comments
Closed

Finding property with minimal string distance is slow #1183

schuay opened this issue Aug 13, 2018 · 2 comments

Comments

@schuay
Copy link

schuay commented Aug 13, 2018

Found while investigating the web-tooling-benchmark:

We spend almost 1/3 of all time in the chai web-tooling-benchmark on sorting properties. The custom compare function is excessively expensive:

chai/chai.js

Line 9098 in 7db5214

return stringDistance(property, a) - stringDistance(property, b);

For this use-case, sorting isn't necessary at all since we're only interested in the property with minimal string distance. Instead, it could be determined in linear time in a single iteration.

@meeber
Copy link
Contributor

meeber commented Aug 13, 2018

@schuay Looks related to the discussion in #1098. We always welcome performance improvements, but it's worth stressing that this isn't a critical codepath; it's only hit when someone writes a test with invalid Chai syntax (e.g., misspelling "true" in expect(myVar).to.be.ture).

@schuay
Copy link
Author

schuay commented Aug 13, 2018

Thanks for the pointer, indeed #1098 looks relevant. I think we can close this as a duplicate.

it's worth stressing that this isn't a critical codepath

Right, that's what it looked like. I wonder why the test suite / the web-tooling-benchmark stresses this path so much. As it stands, web-tooling-benchmark/chai is basically an Array-sort-with-a-crazy-expensive-comparison-function benchmark. cc @bmeurer

@meeber meeber closed this as completed Sep 8, 2018
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

No branches or pull requests

2 participants