search

servo / rust-url

URL parser for Rust
https://docs.rs/url/
Apache License 2.0
1.27k stars 317 forks source link

perf: change ports type to `Option<NonZeroU16>` #931

Closed DonIsaac closed 1 month ago

DonIsaac commented 1 month ago

Proposal implementation for #930

I've left the public API unchanged; all public port getter/setters still deal with u16s.

valenting commented 1 month ago

Doesn't this make it impossible to parse http://example.org:0/ ? That's still a valid URL according to the URL standard.

DonIsaac commented 1 month ago

Yes, it is a valid port. It appears to be used for some use cases in the wild. I'm wondering if/how we could get this memory layout improvement without impacting users that need port 0. Maybe a feature flag?

valenting commented 1 month ago

I don't think this is possible for port, but I encourage you to explore options for

    query_start: Option<u32>,    // Before '?', unlike Position::QueryStart
    fragment_start: Option<u32>, // Before '#', unlike Position::FragmentStart

If I'm not mistaken, it should be impossible for them to start at 0.