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

Rewrite NameBuilder #394

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Rewrite NameBuilder #394

wants to merge 7 commits into from

Conversation

bal-e
Copy link

@bal-e bal-e commented Sep 16, 2024

This is primarily a performance enhancement. The idea is to require the storage backing NameBuilder to always provide a fixed 256-byte array, rather than performing any dynamic allocations. Since NameBuilders are transient objects, allocating a few more bytes won't hurt, especially if they can be put on the stack.

I initially intended to make NameBuilder just take a reference to a [u8; 256] array somewhere in the caller's code. However, this would make it harder to store NameBuilder (because you also need to store the backing buffer somewhere, and you can quickly run into self-referential lifetime issues). Instead, I use a Buffer parameter that should implement Borrow and BorrowMut (not AsRef / AsMut as they don't have the blanket impl Borrow<T> for T).

This work will lead up to #388, making it easier to build domain names quickly. Note that I've dropped all support for Symbol here.

arya dradjica added 7 commits September 16, 2024 17:10
This is a simple implementation with a different API from the previous
code, designed to be replaced with a more efficient implementation in
the future.
Most of the domain name types have been integrated with 'NameBuilder',
abandoning previous support for 'from_chars()' or 'from_symbols()' and
instead focusing on 'FromStr'.  More complex inputs should be processed
directly using 'NameBuilder' instead.
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.

1 participant