From dfc43d4e9be67fdf25553677f469379d966ff806 Mon Sep 17 00:00:00 2001 From: Alex Potsides Date: Fri, 23 Sep 2022 14:23:16 +0100 Subject: [PATCH] fix: replace slice with subarray for increased performance (#4210) In several places we call `.slice` as a way to transform `BufferList`s to `Uint8Array`s. Due to refactors in some places we are now calling `.slice` on `Uint8Array`s which is a memory-copy operation. In other places `Uint8ArrayList`s are now returned instead of `BufferList`s on which `.slice` is also a memory-copy operation. Swap `.slice` for `.subarray` which is no-copy for `Uint8Array`s and can be no-copy for `Uint8ArrayList`s too, where there is only a single backing buffer. In places where we need to transform multiple `Uint8ArrayList`s to multiple `Uint8Array`s, yield the iterators of the `Uint8ArrayList`s as this is also a no-copy operation. --- packages/ipfs-cli/package.json | 2 +- packages/ipfs-cli/src/commands/cat.js | 10 ++++++++-- packages/ipfs-cli/src/commands/get.js | 2 -- packages/ipfs-cli/test/cat.spec.js | 16 +++++++++++++++- packages/ipfs-cli/test/get.spec.js | 7 +------ .../src/components/files/utils/hamt-constants.js | 2 +- packages/ipfs-core/src/components/files/write.js | 4 ++-- packages/ipfs-core/src/components/get.js | 13 ++----------- .../src/utils/web-socket-message-channel.js | 4 ++-- .../ipfs-http-gateway/src/resources/gateway.js | 6 +----- .../ipfs-http-response/src/utils/content-type.js | 13 ++++++++++--- 11 files changed, 43 insertions(+), 36 deletions(-) diff --git a/packages/ipfs-cli/package.json b/packages/ipfs-cli/package.json index 68b99ef655..5156fddad3 100644 --- a/packages/ipfs-cli/package.json +++ b/packages/ipfs-cli/package.json @@ -85,7 +85,6 @@ "ipfs-http-client": "^58.0.1", "ipfs-utils": "^9.0.6", "it-concat": "^2.0.0", - "it-map": "^1.0.6", "it-merge": "^1.0.3", "it-pipe": "^2.0.3", "it-split": "^1.0.0", @@ -110,6 +109,7 @@ "ipfs-repo": "^15.0.3", "it-all": "^1.0.4", "it-first": "^1.0.4", + "it-map": "^1.0.6", "it-to-buffer": "^2.0.0", "nanoid": "^4.0.0", "ncp": "^2.0.0", diff --git a/packages/ipfs-cli/src/commands/cat.js b/packages/ipfs-cli/src/commands/cat.js index befdfd5105..da4163f79c 100644 --- a/packages/ipfs-cli/src/commands/cat.js +++ b/packages/ipfs-cli/src/commands/cat.js @@ -6,6 +6,7 @@ import parseDuration from 'parse-duration' * @property {string} Argv.ipfsPath * @property {number} Argv.offset * @property {number} Argv.length + * @property {boolean} Argv.preload * @property {number} Argv.timeout */ @@ -26,14 +27,19 @@ const command = { number: true, describe: 'Maximum number of bytes to read' }, + preload: { + boolean: true, + default: true, + describe: 'Preload this object when adding' + }, timeout: { string: true, coerce: parseDuration } }, - async handler ({ ctx: { ipfs, print }, ipfsPath, offset, length, timeout }) { - for await (const buf of ipfs.cat(ipfsPath, { offset, length, timeout })) { + async handler ({ ctx: { ipfs, print }, ipfsPath, offset, length, preload, timeout }) { + for await (const buf of ipfs.cat(ipfsPath, { offset, length, preload, timeout })) { print.write(buf) } } diff --git a/packages/ipfs-cli/src/commands/get.js b/packages/ipfs-cli/src/commands/get.js index 9e344e4f40..18c219d959 100644 --- a/packages/ipfs-cli/src/commands/get.js +++ b/packages/ipfs-cli/src/commands/get.js @@ -8,7 +8,6 @@ import { stripControlCharacters } from '../utils.js' import { extract } from 'it-tar' -import map from 'it-map' /** * @typedef {object} Argv @@ -110,7 +109,6 @@ const command = { await fs.promises.mkdir(path.dirname(outputPath), { recursive: true }) await pipe( body, - (source) => map(source, buf => buf.slice()), toIterable.sink(fs.createWriteStream(outputPath)) ) } else if (header.type === 'directory') { diff --git a/packages/ipfs-cli/test/cat.spec.js b/packages/ipfs-cli/test/cat.spec.js index 66996db78e..3b096ce664 100644 --- a/packages/ipfs-cli/test/cat.spec.js +++ b/packages/ipfs-cli/test/cat.spec.js @@ -9,7 +9,8 @@ import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string' const defaultOptions = { offset: undefined, length: undefined, - timeout: undefined + timeout: undefined, + preload: true } describe('cat', () => { @@ -81,4 +82,17 @@ describe('cat', () => { const out = await cli(`cat ${cid} --timeout=1s`, { ipfs, raw: true }) expect(out).to.deep.equal(buf) }) + + it('should cat a file without preloading', async () => { + const cid = CID.parse('QmPZ9gcCEpqKTo6aq61g2nXGUhM4iCL3ewB6LDXZCtioEB') + const buf = uint8ArrayFromString('hello world') + + ipfs.cat.withArgs(cid.toString(), { + ...defaultOptions, + preload: false + }).returns([buf]) + + const out = await cli(`cat ${cid} --preload=false`, { ipfs, raw: true }) + expect(out).to.deep.equal(buf) + }) }) diff --git a/packages/ipfs-cli/test/get.spec.js b/packages/ipfs-cli/test/get.spec.js index a3f7d22200..8101c81712 100644 --- a/packages/ipfs-cli/test/get.spec.js +++ b/packages/ipfs-cli/test/get.spec.js @@ -9,7 +9,6 @@ import sinon from 'sinon' import { fromString as uint8ArrayFromString } from 'uint8arrays/from-string' import { pack } from 'it-tar' import { pipe } from 'it-pipe' -import map from 'it-map' import toBuffer from 'it-to-buffer' import { clean } from './utils/clean.js' import Pako from 'pako' @@ -27,11 +26,7 @@ const defaultOptions = { async function * tarballed (files) { yield * pipe( files, - pack(), - /** - * @param {AsyncIterable} source - */ - (source) => map(source, buf => buf.slice()) + pack() ) } diff --git a/packages/ipfs-core/src/components/files/utils/hamt-constants.js b/packages/ipfs-core/src/components/files/utils/hamt-constants.js index 9a3c4eabfb..0a8676c563 100644 --- a/packages/ipfs-core/src/components/files/utils/hamt-constants.js +++ b/packages/ipfs-core/src/components/files/utils/hamt-constants.js @@ -11,7 +11,7 @@ export async function hamtHashFn (buf) { // Murmur3 outputs 128 bit but, accidentally, IPFS Go's // implementation only uses the first 64, so we must do the same // for parity.. - .slice(0, 8) + .subarray(0, 8) // Invert buffer because that's how Go impl does it .reverse() } diff --git a/packages/ipfs-core/src/components/files/write.js b/packages/ipfs-core/src/components/files/write.js index a1eb9e9b14..9e81518b39 100644 --- a/packages/ipfs-core/src/components/files/write.js +++ b/packages/ipfs-core/src/components/files/write.js @@ -334,7 +334,7 @@ const limitAsyncStreamBytes = (stream, limit) => { emitted += buf.length if (emitted > limit) { - yield buf.slice(0, limit - emitted) + yield buf.subarray(0, limit - emitted) return } @@ -353,7 +353,7 @@ const asyncZeroes = (count, chunkSize = MFS_MAX_CHUNK_SIZE) => { async function * _asyncZeroes () { while (true) { - yield buf.slice() + yield buf } } diff --git a/packages/ipfs-core/src/components/get.js b/packages/ipfs-core/src/components/get.js index b0318dff12..27f1606ea8 100644 --- a/packages/ipfs-core/src/components/get.js +++ b/packages/ipfs-core/src/components/get.js @@ -6,7 +6,6 @@ import { CID } from 'multiformats/cid' import { pack } from 'it-tar' import { pipe } from 'it-pipe' import Pako from 'pako' -import map from 'it-map' import toBuffer from 'it-to-buffer' // https://www.gnu.org/software/gzip/manual/gzip.html @@ -57,11 +56,7 @@ export function createGet ({ repo, preload }) { }, body: file.content() }], - pack(), - /** - * @param {AsyncIterable} source - */ - (source) => map(source, buf => buf.slice()) + pack() ) } else { args.push( @@ -126,11 +121,7 @@ export function createGet ({ repo, preload }) { yield output } }, - pack(), - /** - * @param {AsyncIterable} source - */ - (source) => map(source, buf => buf.slice()) + pack() ] if (options.compress) { diff --git a/packages/ipfs-grpc-server/src/utils/web-socket-message-channel.js b/packages/ipfs-grpc-server/src/utils/web-socket-message-channel.js index fc89ac3be3..e2da3046c2 100644 --- a/packages/ipfs-grpc-server/src/utils/web-socket-message-channel.js +++ b/packages/ipfs-grpc-server/src/utils/web-socket-message-channel.js @@ -77,7 +77,7 @@ export class WebSocketMessageChannel { return } - const header = buf.slice(offset, HEADER_SIZE + offset) + const header = buf.subarray(offset, HEADER_SIZE + offset) const length = header.readUInt32BE(1) offset += HEADER_SIZE @@ -85,7 +85,7 @@ export class WebSocketMessageChannel { return } - const message = buf.slice(offset, offset + length) + const message = buf.subarray(offset, offset + length) const deserialized = this.handler.deserialize(message) this.source.push(deserialized) }) diff --git a/packages/ipfs-http-gateway/src/resources/gateway.js b/packages/ipfs-http-gateway/src/resources/gateway.js index e8f752cbe5..a34183eeb0 100644 --- a/packages/ipfs-http-gateway/src/resources/gateway.js +++ b/packages/ipfs-http-gateway/src/resources/gateway.js @@ -132,11 +132,7 @@ export const Gateway = { } const { source, contentType } = await detectContentType(ipfsPath, ipfs.cat(data.cid, catOptions)) - const responseStream = toStream.readable((async function * () { - for await (const chunk of source) { - yield chunk.slice() // Convert BufferList to Buffer - } - })()) + const responseStream = toStream.readable(source) const res = h.response(responseStream).code(rangeResponse ? 206 : 200) diff --git a/packages/ipfs-http-response/src/utils/content-type.js b/packages/ipfs-http-response/src/utils/content-type.js index 0b5f7b2814..eff8d80a83 100644 --- a/packages/ipfs-http-response/src/utils/content-type.js +++ b/packages/ipfs-http-response/src/utils/content-type.js @@ -28,11 +28,11 @@ export const detectContentType = async (path, source) => { if (done) { return { - source: map(stream, (buf) => buf.slice()) + source: map(stream, (buf) => buf.subarray()) } } - fileSignature = await fileTypeFromBuffer(value.slice()) + fileSignature = await fileTypeFromBuffer(value.subarray()) output = (async function * () { // eslint-disable-line require-await yield value @@ -62,7 +62,14 @@ export const detectContentType = async (path, source) => { } if (output != null) { - return { source: map(output, (buf) => buf.slice()), contentType } + return { + source: (async function * () { + for await (const list of output) { + yield * list + } + }()), + contentType + } } return { source, contentType }