Introduce a replacement for base::name
#422
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The goal of the 'new_name' module is to provide a simpler and more efficient implementation of the basic domain name types. Rather than being generic over the underlying byte sequence, 'Name' and 'RelName' are just unsized byte slices, leaving their allocation up to the user. Their methods are entirely based in slice manipulations, rather than through generic label iteration. They should be much more performant, but we are lacking benchmarks.
I was worried I'd have to iterate over the labels, but actually, it's pretty straightforward. I hope the compiler can vectorize it nicely.
Unlike the existing 'NameBuilder', this type uses a fixed-size buffer to write the name into. This results in simpler code and it should be more efficient. It provides simple methods to extract domain names by borrowing from the buffer instead of allocating. This is a rewrite of <#394>.
It was returning the value of 'total_len()'. Also fixed a clippy warning.
I also fixed a few bugs in the existing name methods.
|
@partim, @Philip-NLnetLabs: I'd like to hear your opinion on |
An incomplete and untested Punycode decoder has also been added.
A basic quadratic-time output builder has been implemented. At the moment, no further validation of U-labels is performed; we need a way to represent U-labels (even owned ones) and to validate and encode them from there.
I wasn't comfortable with the 'Owned' paradigm, particularly due to the generic buffer parameter. It's easier to work with an explicit buffer type, and in some cases it may even have useful additional methods.
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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
base::new_namemodule seeks to provide a simpler and more ergonomic interface for working with domain names. Its core types,Name,RelName, etc., are not generic over an underlying octets type: they are simple unsized byte slices, which can be stored in any container. This greatly simplifies their API and makes their methods more amenable to optimization.The idea is to gradually replace the use of
base::namewithbase::new_nameacross the crate, and eventually to removebase::nameand letbase::new_nametake its place. Given the large number of modules in the crate, this will likely take a while.While most of the functionality of
base::namehas been replicated, some things have been explicitly omitted. TheToNameandToRelativeNametraits have been dropped, so that most domain name functionality is supported onName/RelNameetc. but not onChainorParsedName. Since domain names are so small (usually less than 64 bytes, always less than 256 bytes),ParsedNameobjects can be copied out into regularNames before being used. By relying on direct byte slice operations, basic methods like domain name comparison have been sped up (from quadratic to linear time, in fact).