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

Flat name #1190

Closed
wants to merge 10 commits into from
Closed

Flat name #1190

wants to merge 10 commits into from

Conversation

bluejekyll
Copy link
Member

An attempt at making Name have zero-overhead options.

this currently show a performance degradation over the existing implementation.

Until I can figure out if there's a way around those, I'll be leaving this as a draft for experimentation.

@bluejekyll
Copy link
Member Author

With some of the new const fn stabilizations, we may be able to move some DNS name validation into a const method...

@saethlin
Copy link
Contributor

I am very interested in making Name parsing faster. Right now in our profiles it looks like an allocator stress test. Which is livable but you know, I'd prefer if it wasn't.

this currently show a performance degradation over the existing implementation.

What evidence is this based on? The cargo bench benchmarks on the latest release are quite minimal, and don't exercise the Name parsing code under anything resembling a real workload. There's also a comment in code that Names with 3 labels are expected to be very common; I don't have good stats on hand but I'd be shocked if that's the median number of labels we're seeing in DNS traffic. I'm more than happy to take this one step at a time and beef up benchmarking.

@bluejekyll
Copy link
Member Author

I am very interested in making Name parsing faster. Right now in our profiles it looks like an allocator stress test.

If you have a strong interest here, I'd love for the help and definitely appreciate the focus on an improvements in this area.

What evidence is this based on? The cargo bench benchmarks on the latest release are quite minimal, and don't exercise the Name parsing code under anything resembling a real workload.

I bet there are many ways to improve this.

I stopped working on this PR a while back, but have definitely wanted to pick it up again. If this is something you'd like to work on, I'd absolutely help with that as much as I can. The existing PR may be outdated, and not ideal for a lot of reasons. I think one of the challenges in this area is that there are in many cases additional search paths that need to be applied during resolution, which makes zero-overhead solutions challenging. On top of that, the need to support utf8 conversions to puny-code again creates a potential allocation issue (I'm still upset that DNS isn't just utf8 on the wire).

Anyway, the project can definitely use a lot of attention in this area, and I'm always happy to help explain things where the docs might not be detailed enough, or past decisions were not clear.

@bluejekyll
Copy link
Member Author

Closing this in favor of #1388

@bluejekyll bluejekyll closed this Mar 6, 2021
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.

None yet

2 participants