Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upmore idna data slimming #365
Merged
+6,015
−6,700
Conversation
It's easier to merge identical mapping entries if we have a separate representation of all the ranges we're going to print out.
There's no reason to store adjacent ranges that map to identical results separately; we can store them as a single entry and save ~10% space.
StringTableSlice has an alignment of 2, given its u16 members. The byte_len field can always fit into a u8. To give it an alignment of 1 so that it can pack well into a Mapping, we need to store the byte_start member as separate u8 fields, and change Mapping's discriminant to a u8. This change makes Range take up 12 bytes on a 64-bit system instead of 16, for a significant space savings. LLVM ought to be able to optimize the load/load/shift/or recomposition of byte_start into a single u16 load on unaligned architectures like x86. Current Rust does not do that, but an extra instruction or two is worth a 25% space optimization.
|
Looks great, thanks! This does not include an version number increment, but #351 does so if you don’t mind waiting a bit I’ll publish both together to avoid merge conflicts. @bors-servo r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Jun 20, 2017
more idna data slimming We can do a better job of packing the uts46 data: * We can merge identically-mapped entries that don't have an associated string slice. This saves ~10% space. * We can make slices smaller and pack them into `Mapping` better, which saves 25% space on 64-bit platforms. I think it might save half that on 32-bit platforms, but I didn't check. Together these are good for ~42KB of space savings on a 64-bit platform. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-url/365) <!-- Reviewable:end -->
That'd be great, thank you! |
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
froydnj commentedJun 18, 2017
•
edited by larsbergstrom
We can do a better job of packing the uts46 data:
Mappingbetter, which saves 25% space on 64-bit platforms. I think it might save half that on 32-bit platforms, but I didn't check.Together these are good for ~42KB of space savings on a 64-bit platform.
This change is