Skip to content

Commit e2fb9d5

Browse files
kuhekuhe
authored
feat(lib-storage): improve performance by reducing buffer copies (#5078)
* feat(lib-storage): improve performance by reducing buffer copies * test(lib-storage): add e2e tests
1 parent 93f81c4 commit e2fb9d5

12 files changed

+313
-222
lines changed

‎lib/lib-storage/jest.config.e2e.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module.exports = {
2+
preset: "ts-jest",
3+
testMatch: ["**/*.e2e.spec.ts"],
4+
bail: true,
5+
};

‎lib/lib-storage/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@
1414
"build:types:downlevel": "downlevel-dts dist-types dist-types/ts3.4",
1515
"clean": "rimraf ./dist-* && rimraf *.tsbuildinfo",
1616
"extract:docs": "api-extractor run --local",
17-
"test": "jest"
17+
"test": "jest",
18+
"test:e2e": "jest -c jest.config.e2e.js"
1819
},
1920
"engines": {
2021
"node": ">=14.0.0"

‎lib/lib-storage/src/Upload.spec.ts

Lines changed: 122 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -312,135 +312,136 @@ describe(Upload.name, () => {
312312
expect(result.Location).toEqual("https://example-bucket.example-host.com/folder/example-key");
313313
});
314314

315-
it("should upload using multi-part when parts are larger than part size", async () => {
316-
// create a string that's larger than 5MB.
317-
const partSize = 1024 * 1024 * 5;
318-
const largeBuffer = Buffer.from("#".repeat(partSize + 10));
319-
const firstBuffer = largeBuffer.subarray(0, partSize);
320-
const secondBuffer = largeBuffer.subarray(partSize);
321-
const actionParams = { ...params, Body: largeBuffer };
322-
const upload = new Upload({
323-
params: actionParams,
324-
client: new S3({}),
325-
});
326-
await upload.done();
327-
expect(sendMock).toHaveBeenCalledTimes(4);
328-
// create multipartMock is called correctly.
329-
expect(createMultipartMock).toHaveBeenCalledTimes(1);
330-
expect(createMultipartMock).toHaveBeenCalledWith({
331-
...actionParams,
332-
Body: undefined,
333-
});
334-
// upload parts is called correctly.
335-
expect(uploadPartMock).toHaveBeenCalledTimes(2);
336-
expect(uploadPartMock).toHaveBeenNthCalledWith(1, {
337-
...actionParams,
338-
// @ts-ignore extended custom matcher
339-
Body: expect.toHaveSameHashAsBuffer(firstBuffer),
340-
PartNumber: 1,
341-
UploadId: "mockuploadId",
342-
});
343-
expect(uploadPartMock).toHaveBeenNthCalledWith(2, {
344-
...actionParams,
345-
// @ts-ignore extended custom matcher
346-
Body: expect.toHaveSameHashAsBuffer(secondBuffer),
347-
PartNumber: 2,
348-
UploadId: "mockuploadId",
349-
});
350-
// complete multipart upload is called correctly.
351-
expect(completeMultipartMock).toHaveBeenCalledTimes(1);
352-
expect(completeMultipartMock).toHaveBeenLastCalledWith({
353-
...actionParams,
354-
Body: undefined,
355-
UploadId: "mockuploadId",
356-
MultipartUpload: {
357-
Parts: [
358-
{
359-
ETag: "mock-upload-Etag",
360-
PartNumber: 1,
361-
},
362-
{
363-
ETag: "mock-upload-Etag-2",
364-
PartNumber: 2,
365-
},
366-
],
367-
},
368-
});
315+
[
316+
{ type: "buffer", largeBuffer: Buffer.from("#".repeat(DEFAULT_PART_SIZE + 10)) },
317+
{ type: "Uint8array", largeBuffer: Uint8Array.from(Buffer.from("#".repeat(DEFAULT_PART_SIZE + 10))) },
318+
].forEach(({ type, largeBuffer }) => {
319+
it(`should upload using multi-part when parts are larger than part size ${type}`, async () => {
320+
const firstBuffer = largeBuffer.subarray(0, DEFAULT_PART_SIZE);
321+
const secondBuffer = largeBuffer.subarray(DEFAULT_PART_SIZE);
322+
const actionParams = { ...params, Body: largeBuffer };
323+
const upload = new Upload({
324+
params: actionParams,
325+
client: new S3({}),
326+
});
327+
await upload.done();
328+
expect(sendMock).toHaveBeenCalledTimes(4);
329+
// create multipartMock is called correctly.
330+
expect(createMultipartMock).toHaveBeenCalledTimes(1);
331+
expect(createMultipartMock).toHaveBeenCalledWith({
332+
...actionParams,
333+
Body: undefined,
334+
});
335+
// upload parts is called correctly.
336+
expect(uploadPartMock).toHaveBeenCalledTimes(2);
337+
expect(uploadPartMock).toHaveBeenNthCalledWith(1, {
338+
...actionParams,
339+
// @ts-ignore extended custom matcher
340+
Body: expect.toHaveSameHashAsBuffer(firstBuffer),
341+
PartNumber: 1,
342+
UploadId: "mockuploadId",
343+
});
344+
expect(uploadPartMock).toHaveBeenNthCalledWith(2, {
345+
...actionParams,
346+
// @ts-ignore extended custom matcher
347+
Body: expect.toHaveSameHashAsBuffer(secondBuffer),
348+
PartNumber: 2,
349+
UploadId: "mockuploadId",
350+
});
351+
// complete multipart upload is called correctly.
352+
expect(completeMultipartMock).toHaveBeenCalledTimes(1);
353+
expect(completeMultipartMock).toHaveBeenLastCalledWith({
354+
...actionParams,
355+
Body: undefined,
356+
UploadId: "mockuploadId",
357+
MultipartUpload: {
358+
Parts: [
359+
{
360+
ETag: "mock-upload-Etag",
361+
PartNumber: 1,
362+
},
363+
{
364+
ETag: "mock-upload-Etag-2",
365+
PartNumber: 2,
366+
},
367+
],
368+
},
369+
});
369370

370-
// no tags were passed.
371-
expect(putObjectTaggingMock).toHaveBeenCalledTimes(0);
372-
// put was not called
373-
expect(putObjectMock).toHaveBeenCalledTimes(0);
374-
});
371+
// no tags were passed.
372+
expect(putObjectTaggingMock).toHaveBeenCalledTimes(0);
373+
// put was not called
374+
expect(putObjectMock).toHaveBeenCalledTimes(0);
375+
});
376+
377+
it("should upload using multi-part when parts are larger than part size stream", async () => {
378+
// create a string that's larger than 5MB.
379+
const firstBuffer = largeBuffer.subarray(0, DEFAULT_PART_SIZE);
380+
const secondBuffer = largeBuffer.subarray(DEFAULT_PART_SIZE);
381+
const streamBody = Readable.from(
382+
(function* () {
383+
yield largeBuffer;
384+
})()
385+
);
386+
const actionParams = { ...params, Body: streamBody };
387+
const upload = new Upload({
388+
params: actionParams,
389+
client: new S3({}),
390+
});
375391

376-
it("should upload using multi-part when parts are larger than part size stream", async () => {
377-
// create a string that's larger than 5MB.
378-
const largeBuffer = Buffer.from("#".repeat(DEFAULT_PART_SIZE + 10));
379-
const firstBuffer = largeBuffer.subarray(0, DEFAULT_PART_SIZE);
380-
const secondBuffer = largeBuffer.subarray(DEFAULT_PART_SIZE);
381-
const streamBody = Readable.from(
382-
(function* () {
383-
yield largeBuffer;
384-
})()
385-
);
386-
const actionParams = { ...params, Body: streamBody };
387-
const upload = new Upload({
388-
params: actionParams,
389-
client: new S3({}),
390-
});
392+
await upload.done();
391393

392-
await upload.done();
394+
expect(sendMock).toHaveBeenCalledTimes(4);
395+
// create multipartMock is called correctly.
396+
expect(createMultipartMock).toHaveBeenCalledTimes(1);
397+
expect(createMultipartMock).toHaveBeenCalledWith({
398+
...actionParams,
399+
Body: undefined,
400+
});
393401

394-
expect(sendMock).toHaveBeenCalledTimes(4);
395-
// create multipartMock is called correctly.
396-
expect(createMultipartMock).toHaveBeenCalledTimes(1);
397-
expect(createMultipartMock).toHaveBeenCalledWith({
398-
...actionParams,
399-
Body: undefined,
400-
});
402+
// upload parts is called correctly.
403+
expect(uploadPartMock).toHaveBeenCalledTimes(2);
404+
expect(uploadPartMock).toHaveBeenNthCalledWith(1, {
405+
...actionParams,
406+
// @ts-ignore extended custom matcher
407+
Body: expect.toHaveSameHashAsBuffer(firstBuffer),
408+
PartNumber: 1,
409+
UploadId: "mockuploadId",
410+
});
401411

402-
// upload parts is called correctly.
403-
expect(uploadPartMock).toHaveBeenCalledTimes(2);
404-
expect(uploadPartMock).toHaveBeenNthCalledWith(1, {
405-
...actionParams,
406-
// @ts-ignore extended custom matcher
407-
Body: expect.toHaveSameHashAsBuffer(firstBuffer),
408-
PartNumber: 1,
409-
UploadId: "mockuploadId",
410-
});
412+
expect(uploadPartMock).toHaveBeenNthCalledWith(2, {
413+
...actionParams,
414+
// @ts-ignore extended custom matcher
415+
Body: expect.toHaveSameHashAsBuffer(secondBuffer),
416+
PartNumber: 2,
417+
UploadId: "mockuploadId",
418+
});
411419

412-
expect(uploadPartMock).toHaveBeenNthCalledWith(2, {
413-
...actionParams,
414-
// @ts-ignore extended custom matcher
415-
Body: expect.toHaveSameHashAsBuffer(secondBuffer),
416-
PartNumber: 2,
417-
UploadId: "mockuploadId",
418-
});
420+
// complete multipart upload is called correctly.
421+
expect(completeMultipartMock).toHaveBeenCalledTimes(1);
422+
expect(completeMultipartMock).toHaveBeenLastCalledWith({
423+
...actionParams,
424+
Body: undefined,
425+
UploadId: "mockuploadId",
426+
MultipartUpload: {
427+
Parts: [
428+
{
429+
ETag: "mock-upload-Etag",
430+
PartNumber: 1,
431+
},
432+
{
433+
ETag: "mock-upload-Etag-2",
434+
PartNumber: 2,
435+
},
436+
],
437+
},
438+
});
419439

420-
// complete multipart upload is called correctly.
421-
expect(completeMultipartMock).toHaveBeenCalledTimes(1);
422-
expect(completeMultipartMock).toHaveBeenLastCalledWith({
423-
...actionParams,
424-
Body: undefined,
425-
UploadId: "mockuploadId",
426-
MultipartUpload: {
427-
Parts: [
428-
{
429-
ETag: "mock-upload-Etag",
430-
PartNumber: 1,
431-
},
432-
{
433-
ETag: "mock-upload-Etag-2",
434-
PartNumber: 2,
435-
},
436-
],
437-
},
440+
// no tags were passed.
441+
expect(putObjectTaggingMock).toHaveBeenCalledTimes(0);
442+
// put was not called
443+
expect(putObjectMock).toHaveBeenCalledTimes(0);
438444
});
439-
440-
// no tags were passed.
441-
expect(putObjectTaggingMock).toHaveBeenCalledTimes(0);
442-
// put was not called
443-
expect(putObjectMock).toHaveBeenCalledTimes(0);
444445
});
445446

446447
it("should add tags to the object if tags have been added PUT", async () => {

‎lib/lib-storage/src/bytelength.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
1+
import { Buffer } from "buffer"; // do not remove this import: Node.js buffer or buffer NPM module for browser.
2+
13
import { ClientDefaultValues } from "./runtimeConfig";
24

35
export const byteLength = (input: any) => {
46
if (input === null || input === undefined) return 0;
5-
if (typeof input === "string") input = Buffer.from(input);
7+
8+
if (typeof input === "string") {
9+
return Buffer.byteLength(input);
10+
}
11+
612
if (typeof input.byteLength === "number") {
713
return input.byteLength;
814
} else if (typeof input.length === "number") {

‎lib/lib-storage/src/chunker.ts

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,37 @@
1-
import { Buffer } from "buffer";
1+
import { Buffer } from "buffer"; // do not remove this import: Node.js buffer or buffer NPM module for browser.
22
import { Readable } from "stream";
33

4-
import { getChunkBuffer } from "./chunks/getChunkBuffer";
54
import { getChunkStream } from "./chunks/getChunkStream";
5+
import { getChunkUint8Array } from "./chunks/getChunkUint8Array";
66
import { getDataReadable } from "./chunks/getDataReadable";
77
import { getDataReadableStream } from "./chunks/getDataReadableStream";
88
import { BodyDataTypes } from "./types";
9+
import type { RawDataPart } from "./Upload";
910

10-
export const getChunk = (data: BodyDataTypes, partSize: number) => {
11-
if (data instanceof Buffer) {
12-
return getChunkBuffer(data, partSize);
13-
} else if (data instanceof Readable) {
11+
export const getChunk = (data: BodyDataTypes, partSize: number): AsyncGenerator<RawDataPart, void, undefined> => {
12+
if (data instanceof Uint8Array) {
13+
// includes Buffer (extends Uint8Array)
14+
return getChunkUint8Array(data, partSize);
15+
}
16+
17+
if (data instanceof Readable) {
1418
return getChunkStream<Readable>(data, partSize, getDataReadable);
15-
} else if (data instanceof String || typeof data === "string" || data instanceof Uint8Array) {
16-
// chunk Strings, Uint8Array.
17-
return getChunkBuffer(Buffer.from(data), partSize);
1819
}
20+
21+
if (data instanceof String || typeof data === "string") {
22+
return getChunkUint8Array(Buffer.from(data), partSize);
23+
}
24+
1925
if (typeof (data as any).stream === "function") {
2026
// approximate support for Blobs.
2127
return getChunkStream<ReadableStream>((data as any).stream(), partSize, getDataReadableStream);
22-
} else if (data instanceof ReadableStream) {
28+
}
29+
30+
if (data instanceof ReadableStream) {
2331
return getChunkStream<ReadableStream>(data, partSize, getDataReadableStream);
24-
} else {
25-
throw new Error(
26-
"Body Data is unsupported format, expected data to be one of: string | Uint8Array | Buffer | Readable | ReadableStream | Blob;."
27-
);
2832
}
33+
34+
throw new Error(
35+
"Body Data is unsupported format, expected data to be one of: string | Uint8Array | Buffer | Readable | ReadableStream | Blob;."
36+
);
2937
};

0 commit comments

Comments
 (0)