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

idna maintenance and performance improvements #616

Closed
wants to merge 5 commits into from
Closed

idna maintenance and performance improvements #616

wants to merge 5 commits into from

Conversation

djc
Copy link
Contributor

@djc djc commented Jul 8, 2020

I started looking at the performance profile of to_unicode() and here I am, 12 hours later:

  • Updated IDNA tests to latest version
  • Updated IDNA mapping table to Unicode 13.0
  • Added a Codec API that enables some of the allocation costs over multiple to_unicode()/to_ascii() calls

I wrote some benchmarks for to_unicode(), before:

test to_unicode_ascii        ... bench:       1,117 ns/iter (+/- 27)
test to_unicode_merged_label ... bench:       4,415 ns/iter (+/- 135)
test to_unicode_puny_label   ... bench:       2,273 ns/iter (+/- 78)

and after:

test to_unicode_ascii        ... bench:          79 ns/iter (+/- 4)
test to_unicode_merged_label ... bench:       2,034 ns/iter (+/- 58)
test to_unicode_puny_label   ... bench:       1,506 ns/iter (+/- 52)

These changes have taken care not to change any of the existing APIs.

Additionally, I saw the old thread about asking for maintenance help. At this point I feel like I'm in a pretty good position to help out with idna maintenance (and could potentially help with the other crates as well if that's useful). Let me know if that'd be useful/how that could take shape.

I've tried to keep commits atomic and easy to review (some of them benefit from reviewing while hiding whitespace changes). Let me know if there are any questions.

@djc
Copy link
Contributor Author

djc commented Jul 9, 2020

(Updated original description with benchmark results.)

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably 0a92c73) made this pull request unmergeable. Please resolve the merge conflicts.

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably aa08186) made this pull request unmergeable. Please resolve the merge conflicts.

@djc
Copy link
Contributor Author

djc commented Aug 25, 2020

I've landed a bunch of this and opened a new PR with the remaining changes in #635.

@djc djc closed this Aug 25, 2020
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