Skip to content

Commit

Permalink
reduce buffer copies (#867)
Browse files Browse the repository at this point in the history
  • Loading branch information
kuhe authored Mar 12, 2024
1 parent 9275e12 commit 511206e
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 35 deletions.
6 changes: 6 additions & 0 deletions .changeset/strange-cars-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@smithy/util-body-length-node": patch
"@smithy/node-http-handler": patch
---

reduce buffer copies
61 changes: 33 additions & 28 deletions packages/node-http-handler/src/node-http-handler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import { NodeHttpHandler } from "./node-http-handler";
import { ReadFromBuffers } from "./readable.mock";
import {
createContinueResponseFunction,
createMirrorResponseFunction,
createMockHttpServer,
createMockHttpsServer,
createResponseFunction,
getResponseBody,
} from "./server.mock";

describe("NodeHttpHandler", () => {
Expand Down Expand Up @@ -203,36 +205,39 @@ describe("NodeHttpHandler", () => {
expect(response.body).toBeDefined();
});

it("can send requests with bodies", async () => {
const body = Buffer.from("test");
const mockResponse = {
statusCode: 200,
headers: {},
};
mockHttpServer.addListener("request", createResponseFunction(mockResponse));
const spy = jest.spyOn(http, "request").mockImplementationOnce(() => {
const calls = spy.mock.calls;
const currentIndex = calls.length - 1;
return http.request(calls[currentIndex][0], calls[currentIndex][1]);
});

const nodeHttpHandler = new NodeHttpHandler();
const { response } = await nodeHttpHandler.handle(
new HttpRequest({
hostname: "localhost",
method: "PUT",
port: (mockHttpServer.address() as AddressInfo).port,
protocol: "http:",
path: "/",
[
{ name: "buffer", body: Buffer.from("Buffering🚀") },
{ name: "uint8Array", body: Uint8Array.from(Buffer.from("uint8Array 🚀")) },
{ name: "string", body: Buffer.from("string-test 🚀") },
{ name: "uint8Array subarray", body: Uint8Array.from(Buffer.from("test")).subarray(1, 3) },
{ name: "buffer subarray", body: Buffer.from("test").subarray(1, 3) },
].forEach(({ body, name }) => {
it(`can send requests with bodies ${name}`, async () => {
const mockResponse = {
statusCode: 200,
headers: {},
body,
}),
{}
);
};
mockHttpServer.addListener("request", createMirrorResponseFunction(mockResponse));
const nodeHttpHandler = new NodeHttpHandler();
const { response } = await nodeHttpHandler.handle(
new HttpRequest({
hostname: "localhost",
method: "PUT",
port: (mockHttpServer.address() as AddressInfo).port,
protocol: "http:",
path: "/",
headers: {},
body,
}),
{}
);

expect(response.statusCode).toEqual(mockResponse.statusCode);
expect(response.headers).toBeDefined();
expect(response.headers).toMatchObject(mockResponse.headers);
expect(response.statusCode).toEqual(mockResponse.statusCode);
expect(response.headers).toBeDefined();
expect(response.headers).toMatchObject(mockResponse.headers);
const responseBody = await getResponseBody(response);
expect(responseBody).toEqual(Buffer.from(body).toString());
});
});

it("can handle expect 100-continue", async () => {
Expand Down
39 changes: 37 additions & 2 deletions packages/node-http-handler/src/server.mock.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { HeaderBag, HttpResponse } from "@smithy/types";
import { HeaderBag, HttpResponse, NodeJsRuntimeBlobTypes } from "@smithy/types";
import { readFileSync } from "fs";
import { createServer as createHttpServer, IncomingMessage, Server as HttpServer, ServerResponse } from "http";
import { createServer as createHttp2Server, Http2Server } from "http2";
Expand All @@ -14,7 +14,7 @@ const setResponseHeaders = (response: ServerResponse, headers: HeaderBag) => {
}
};

const setResponseBody = (response: ServerResponse, body: Readable | string) => {
const setResponseBody = (response: ServerResponse, body: string | NodeJsRuntimeBlobTypes) => {
if (body instanceof Readable) {
body.pipe(response);
} else {
Expand Down Expand Up @@ -67,3 +67,38 @@ export const createMockHttp2Server = (): Http2Server => {
const server = createHttp2Server();
return server;
};

export const createMirrorResponseFunction = (httpResp: HttpResponse) => (
request: IncomingMessage,
response: ServerResponse
) => {
const bufs: Buffer[] = [];
request.on("data", (chunk) => {
bufs.push(chunk);
});
request.on("end", () => {
response.statusCode = httpResp.statusCode;
setResponseHeaders(response, httpResp.headers);
setResponseBody(response, Buffer.concat(bufs));
});
request.on("error", (err) => {
response.statusCode = 500;
setResponseHeaders(response, httpResp.headers);
setResponseBody(response, err.message);
});
};

export const getResponseBody = (response: HttpResponse) => {
return new Promise<string>((resolve, reject) => {
const bufs: Buffer[] = [];
response.body.on("data", function (d: Buffer) {
bufs.push(d);
});
response.body.on("end", function () {
resolve(Buffer.concat(bufs).toString());
});
response.body.on("error", (err: Error) => {
reject(err);
});
});
};
29 changes: 25 additions & 4 deletions packages/node-http-handler/src/write-request-body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,30 @@ function writeBody(
if (body instanceof Readable) {
// pipe automatically handles end
body.pipe(httpRequest);
} else if (body) {
httpRequest.end(Buffer.from(body as Parameters<typeof Buffer.from>[0]));
} else {
httpRequest.end();
return;
}

if (body) {
if (Buffer.isBuffer(body) || typeof body === "string") {
httpRequest.end(body);
return;
}

const uint8 = body as Uint8Array;
if (
typeof uint8 === "object" &&
uint8.buffer &&
typeof uint8.byteOffset === "number" &&
typeof uint8.byteLength === "number"
) {
// this avoids copying the array.
httpRequest.end(Buffer.from(uint8.buffer, uint8.byteOffset, uint8.byteLength));
return;
}

httpRequest.end(Buffer.from(body as ArrayBuffer));
return;
}

httpRequest.end();
}
2 changes: 1 addition & 1 deletion packages/util-body-length-node/src/calculateBodyLength.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export const calculateBodyLength = (body: any): number | undefined => {
return 0;
}
if (typeof body === "string") {
return Buffer.from(body).length;
return Buffer.byteLength(body);
} else if (typeof body.byteLength === "number") {
// handles Uint8Array, ArrayBuffer, Buffer, and ArrayBufferView
return body.byteLength;
Expand Down

0 comments on commit 511206e

Please sign in to comment.