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

Remove dependency on Foundation? #299

Open
adam-fowler opened this issue Aug 25, 2020 · 7 comments
Open

Remove dependency on Foundation? #299

adam-fowler opened this issue Aug 25, 2020 · 7 comments

Comments

@adam-fowler
Copy link
Member

AsyncHTTPClient is currently dependent on Foundation. Would removing this dependency be a target for AHC?

I have done a quick test to see where Foundation is being used. The main source is unsurprisingly URL. The other two are Date/DateFormatter which might be difficult to replace but are used in one very specific place (Cookie expiry dates) and Data which is used in StreamWriter.data and in Authorization.basic where a string is converted to Data to convert it to base64.

To still allow interface functions that use Foundation types they could be moved out to a AsyncHTTPClientFoundationCompat library à la swift-nio. The implementation of HTTPClient functions that take a URL could be moved there along with StreamWriter.data.

@tanner0101
Copy link
Member

I think URL will be the hardest part to migrate away from since URL parsing is non-trivial. Vapor 4 has a URI type built on top of Node's C url parser. You can see that here:

For date formatting, Vapor 3 had some code that generated RFC 1123 strings without foundation:

Regarding base64, that should be easy enough to implement in Swift or find a small C lib for. Vapor 4 for example pulls in a small C lib for base32:


As for whether or not this should be done. I have no idea. What benefits would this give us?

@adam-fowler
Copy link
Member Author

The main benefit this gives us is a much improved Lambda cold start up time. If you include Foundation in a Swift Lambda the cold start can be up to 300ms longer.

Regarding base64, I know @fabianfett has a swift implementation of base64

@pokryfka
Copy link

I had a quick look at that last week as well, the goal being to make dependency on Foundation optional while not breaking existing API.

I moved code using Foundation types to AsyncHTTPClientFoundationCompat and created a naive URI parser in Swift https://github.com/pokryfka/swift-uri (I did not know Vapor has one).

        // exports AsyncHTTPClientCore and AsyncHTTPClientFoundationCompat
        .library(name: "AsyncHTTPClient", targets: ["AsyncHTTPClient"]),
        // core functionality
        .library(name: "AsyncHTTPClientCore", targets: ["AsyncHTTPClientCore"]),
        // adds convenience methods with Foundation types (Data, URL)
        .library(name: "AsyncHTTPClientFoundationCompat", targets: ["AsyncHTTPClientFoundationCompat"]),

I think URL will be the hardest part to migrate away from

Note that most (all?) public methods use String type and URL is used only for parsing, example:

        public init(url: String, method: HTTPMethod = .GET, headers: HTTPHeaders = HTTPHeaders(), body: Body? = nil) throws {
            guard let url = URL(string: url) else {
                throw HTTPClientError.invalidURL
            }

although URL type is in used in Request, most of parsed URL components are also provided: scheme, host, (internal) uri (= path+query):

extension HTTPClient {
    public struct Request {
        /// Remote URL.
        public let url: URL
        /// Remote HTTP scheme, resolved from `URL`.
        public let scheme: String
        /// Remote host, resolved from `URL`.
        public let host: String
        /// Socket path, resolved from `URL`.
        let socketPath: String
        /// URI composed of the path and query, resolved from `URL`.
        let uri: String

        // ...
    }
}

@Lukasa
Copy link
Collaborator

Lukasa commented Aug 29, 2020

We need to be moving to a URL parser that’s more useful than Foundation’s. That will likely mean something that follows the WHATWG URL specification.

@fabianfett
Copy link
Member

Maybe some of us can come together and build a URL struct based on the WHATWG URL spec, Cory mentioned. For completeness the spec can be found here: https://url.spec.whatwg.org

Vapor 4 for example pulls in a small C lib for base32

We could also go all in and use swift variants for this, a base64 implementation is ready and @jenslauterbach is also working on a base32 implementation, which shall live in the same repo.

@karwa
Copy link
Contributor

karwa commented Feb 28, 2021

Hey,

I’ve been working on a new URL type for a little while now. Results are good - I believe we’re fully compliant with the standard (save for IDNA, which for now we just return nil at - it’s a big project just by itself, but we can’t just ignore it because it’s important for operations like equality to work properly). Performance is competitive with Foundation’s URL on my Mac (and equal at -Ounchecked, suggesting maybe the compiler could help us out more than it currently is by eliminating overflow checks and such). Weirdly, the new type is 10x faster than Foundation on my Raspberry pi 4 running Ubuntu 64-bit, but maybe the community toolchain I’m using isn’t optimised correctly or something. I think we also do quite a lot more work than Foundation’s parser - including parsing and canonicalising IP addresses and simplifying paths, as the standard requires. There are more obvious optimisation opportunities, but I still need to create a comprehensive set of benchmarks to quantify them.

I’m currently working on the public API, which already has some nice features like in-place mutation of components. There are also IP address types, percent-encoding APIs, Host and Origin types, etc. Once I have that part closer to where I’d like it, I’m going to announce it on the forums. Obviously I’d be delighted if this was adopted by AHC one day, so I’d be interested in any early feedback you have.

the URL library is here: https://github.com/karwa/swift-url

I have a prototype AHC port here: https://github.com/karwa/async-http-client

There’s also a cool developer tool here, which allows you to compare live the results of this URL type, the JavaScript reference implementation (running in a webview), and Foundation’s URL: https://github.com/karwa/swift-url-tools

After removing uses of Foundation URL, the only other dependency AHC has on it is for base64-encoding cookies, which I believe @fabianfett already has a better-performing alternative for.

@tomerd
Copy link
Contributor

tomerd commented Mar 3, 2021

Making the core API + internals not depend on Foundation is a good idea IMO, as it lets us use AHC in situation where Foundations is not available. That said, I think we should add a "bridging" module that take Foundation types (mainly URL) for situations where Foundation is available so composition is easier. We do the same ion NIO and Lambda.

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

No branches or pull requests

7 participants