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

std.http.Client.fetch sends headers unnecessarily before the remaining payload, causing performance issues in some peculiar environments #22259

Open
jmullaney opened this issue Dec 17, 2024 · 0 comments
Labels
bug Observed behavior contradicts documented or intended behavior

Comments

@jmullaney
Copy link

Zig Version

0.14.0-dev.2487+af89bb05d

Steps to Reproduce and Observed Behavior

While debugging a performance problem in a peculiar environment, I noticed that fetch flushes the headers to the connection stream before sending the rest of the payload. Depending on the size of the http headers and payload relative to the size of data that will fit in the underlying TCP/IP packets, the request will be spread across more packets than is necessary.

The relevant part of the "fetch" code in std.http.Client.zig looks like this (~line 1758):

    ...
    try req.send();

    if (options.payload) |payload| try req.writeAll(payload);
    ...

and send is like this:

    pub fn send(req: *Request) SendError!void {
        ... (write headers to connection stream)
        try connection.flush();
    }

The "flush" call would cause any buffered data to be sent out in an IP packet, which will be unnecessarily small.

I know this seems like a trivial thing, but some environments are apparently very sensitive to this.

(The "peculiar environment" I'm working with is AWS Lambda. You make HTTP requests to interact with the AWS Lambda runtime. When the headers are flushed when posting the first handler response, AWS takes about 50 ms extra to accept the response. It normally takes < 1 ms. Apparently it really doesn't like the request header split from the request body. Of course, that's an AWS problem not a zig problem, but it still makes sense for fetch() to be efficient with how it sends requests.)

Expected Behavior

fetch() doesn't unnecessarily flush headers before sending the payload.

Additionally, it may make sense for the lower-level API to support this case. E.g, "send()" might have an option to flush the connection, or maybe there could be a "send_noflush()" function to support that case. Not sure the best way to expose the option, but I think there should be an option somewhere.

(As it is now, I can't use the standard library's http module, which seems like a shame.)

@jmullaney jmullaney added the bug Observed behavior contradicts documented or intended behavior label Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

No branches or pull requests

1 participant