refactor: Remove special fast path for hickory resolver#4022
Conversation
The downside is that we have 1 more box for each lookup_ipv6 or lookup_ipv4, but I doubt that it matters. The upside is that hickory will be easier to feature gate later. And it's less code.
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/4022/docs/iroh/ Last updated: 2026-03-13T11:43:12Z |
|
When doing #3473 I had this first too, but @dignifiedquire had perf objections so we added the fast path. Not sure myself if its worth it. But featureflagging the enum variant shouldn't also be too hard. So, fine with me, but let's wait for feedback from dig? |
Yeah, feature flagging the enum variant is of course possible. But I somewhat dread that feature flagging too many enum variants will turn the codebase into an effectively readonly maze of feature flags. And I would be seriously surprised if the boxed future makes a difference. I am the first person to be annoyed about boxed futures, but really only in super hot paths, like the middle of a hash function or a 64 byte write in iroh-blobs. For an entire DNS lookup I would bet that you can't even see the diff in a flamegraph, even if the lookup hits a cache. |
|
it’s fine, we‘ll fix perf if it becomes an issue |
Description
Remove special fast path for hickory resolver
The downside is that we have 1 more box for each lookup_ipv6 or lookup_ipv4, but I doubt that it matters. These are network ops, maybe cache hits, but in any case much more heavyweight than a boxed future.
The upside is that hickory will be easier to feature gate later, which we need for embedded. And it's less code.
Rationale for this: hickory is really nasty on embedded. It requires giant (for embedded) stack sizes and also is a big dep. We need it for address_discovery_dns, but there is an alternative for that: just use pkarr for both put and get.
If we don't have a default dep on hickory we can implement a simple dns resolver that just uses getaddrinfo. It won't work for TXT records, but we don't need them for the core functionality of doing just plain A or AAAA requests to figure out relay ip addrs.
Breaking Changes
None
Notes & open questions
Change checklist
quic-rpciroh-gossipiroh-blobsdumbpipesendme