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

Use PHF for generated matches #20

Closed
wants to merge 2 commits into from

Conversation

Manishearth
Copy link
Collaborator

@Manishearth Manishearth commented Aug 14, 2018

This improves performance of language rule fetching significantly. Not as much as I'd hoped, but 5x isn't too bad.

Instead of a giant match (which involves a lot of string comparisons), we use phf-map, which at compile time generates an efficient hash function that we can use with the statically generated map for fast lookup.

# before changes
test bench_create ... bench:         604 ns/iter (+/- 73)
test bench_total  ... bench:         943 ns/iter (+/- 159)


# after changes
test bench_create ... bench:         169 ns/iter (+/- 73)
test bench_total  ... bench:         819 ns/iter (+/- 235)

@Manishearth
Copy link
Collaborator Author

r? @unclenachoduh cc @zbraniecki

@zbraniecki zbraniecki added the enhancement New feature or request label Aug 14, 2018
@zbraniecki
Copy link
Owner

This will require cargo regenerate-fixtures and cargo regenerate-fixtures-within - https://github.com/unclenachoduh/pluralrules/blob/master/make_pluralrules/.cargo/config

@zbraniecki
Copy link
Owner

Contributes to #14

@coveralls
Copy link

Pull Request Test Coverage Report for Build 106

  • 15 of 15 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.03%) to 89.884%

Totals Coverage Status
Change from base Build 105: 0.03%
Covered Lines: 542
Relevant Lines: 603

💛 - Coveralls

@unclenachoduh unclenachoduh mentioned this pull request Aug 15, 2018
Merged
@unclenachoduh
Copy link
Collaborator

Thanks for the PR @Manishearth. I updated the fixtures in #22 so this PR can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants