Transfer webview resource buffers

[mjbvz]

Transfer the underlying buffer of a webview resource load to avoid an extra data copy. This improves performance. The perf improvement is small for normal requests, but becomes more signficant the larger the resource becomes

To support this, I needed to expose a new option that forces VSBuffer to use UInt8Array instead of a Node buffer. In my testing, trying to transfer a node Buffer ends up breaking our general ipc buffers and breaks the entire app. Not sure if there's a better way to do this

[mjbvz]

Exposing this low level flag feels wrong, but as I noted in the issue comment I'm not sure if there is a better way to do this

[alexdima]

Here's my understanding of what is going on:

• in VSBuffer.alloc, we use Buffer.allocUnsafe
• Buffer.allocUnsafe is a lot faster because it uses the nodejs Buffer pool.
• it looks like all ArrayBuffers that are used in the Buffer pool are marked as untransferrable.

For this particular optimization, instead of adding forceUseUint8Array and having some VSBuffer instances use nodejs' Buffer and some use Uint8Array, I suggest to have an argument forTransferable or a change that is localized to webviewElement.ts which would force using a nodejs Buffer outside of the Buffer pool. From what I understand from the nodejs documentation, Buffer.concat, Buffer.from and Buffer.allocUnsafe might all end up returning a Buffer from the Buffer pool.

Here's a quick, untested attempt of a change that could be localized to webviewElement.ts. (I wasn't sure what the exact steps would be to reproduce the crash you mentioned):

function concatWithoutNodePool(buffers: VSBuffer[]): VSBuffer {
	const totalLength = buffers.reduce((prev, curr) => prev + curr.byteLength, 0);
	const ret = Buffer.alloc(totalLength);
	let offset = 0;
	for (let i = 0, len = buffers.length; i < len; i++) {
		const element = buffers[i];
		ret.set(element.buffer, offset);
		offset += element.byteLength;
	}
	return VSBuffer.wrap(ret);
}

const buffer = VSBuffer.wrap(await streams.consumeStream<Buffer>(stream, chunks => concatWithoutNodePool(chunks)));

I will document these findings on main in the JSDoc for these functions. Bottom line, I personally find it better to have all VSBuffer either all use nodejs Buffers or all use Uint8Array and then a way to explicitly ask for nodejs Buffers outside of the pool or separate methods alloc/slowAlloc, concat/slowConcat.

cc @deepak1556

[LifeIsStrange]

@alexdima Reading your comment reminded me of the amazing library
https://github.com/Bnaya/objectbuffer
Which uses this state of the art, very efficient allocator:
https://github.com/thi-ng/umbrella/tree/develop/packages/malloc#about

See performance:

In this micro benchmark, allocating 4KB blocks (1024 floats) is ~20x faster than calling new Float32Array(1024).

Therefore I wonder how this web compatible allocator compare to the current VSbuffer.alloc/allocUnsafe.
Using it might improve performance or memory use for VScode workloads so it would be nice to benchmarck it :)
VScode might also consider using objectbuffer for improved ergonomics regarding arraybuffers/web workers.

[mjbvz]

Thanks @alexdima! I've re-implemented this all in the webview code