From 165f48d90f34466e940e346e6950ab0216191429 Mon Sep 17 00:00:00 2001 From: James Prevett Date: Tue, 14 Jan 2025 18:40:42 -0600 Subject: [PATCH] Added `growBuffer` Changed from `ArrayBuffer.slice` to `ArrayBuffer.subarray` (performance) Fixed `LazyFile.write` and `.writeSync` using an incorrect position --- eslint.shared.js | 1 + src/backends/store/fs.ts | 31 ++++++++++++++++++------------- src/file.ts | 37 ++++++++++--------------------------- src/utils.ts | 39 +++++++++++++++++++++++++++++++++++++++ src/vfs/async.ts | 2 +- src/vfs/promises.ts | 4 ++-- tests/fs/write.test.ts | 17 ++++++----------- 7 files changed, 77 insertions(+), 54 deletions(-) diff --git a/eslint.shared.js b/eslint.shared.js index 8328ebbf..3f0fb1d2 100644 --- a/eslint.shared.js +++ b/eslint.shared.js @@ -45,6 +45,7 @@ export default [ '@typescript-eslint/no-unsafe-call': 'off', '@typescript-eslint/restrict-plus-operands': 'off', '@typescript-eslint/no-base-to-string': 'off', + '@typescript-eslint/no-unused-expressions': 'warn', }, }, { diff --git a/src/backends/store/fs.ts b/src/backends/store/fs.ts index eeeb2620..dee48eea 100644 --- a/src/backends/store/fs.ts +++ b/src/backends/store/fs.ts @@ -1,11 +1,11 @@ import { randomInt, serialize } from 'utilium'; import { Errno, ErrnoError } from '../../error.js'; import type { File } from '../../file.js'; -import { PreloadFile } from '../../file.js'; +import { LazyFile, PreloadFile } from '../../file.js'; import type { CreationOptions, FileSystemMetadata, PureCreationOptions } from '../../filesystem.js'; import { FileSystem } from '../../filesystem.js'; import type { FileType, Stats } from '../../stats.js'; -import { _throw, canary, decodeDirListing, encodeDirListing, encodeUTF8 } from '../../utils.js'; +import { _throw, canary, decodeDirListing, encodeDirListing, encodeUTF8, growBuffer } from '../../utils.js'; import { S_IFDIR, S_IFREG, S_ISGID, S_ISUID, size_max } from '../../vfs/constants.js'; import { basename, dirname, join, parse, resolve } from '../../vfs/path.js'; import { Index } from './file_index.js'; @@ -267,28 +267,28 @@ export class StoreFS extends FileSystem { public async createFile(path: string, flag: string, mode: number, options: CreationOptions): Promise { const node = await this.commitNew(path, S_IFREG, { mode, ...options }, new Uint8Array(), 'createFile'); - return new PreloadFile(this, path, flag, node.toStats(), new Uint8Array()); + return new LazyFile(this, path, flag, node.toStats()); } public createFileSync(path: string, flag: string, mode: number, options: CreationOptions): File { const node = this.commitNewSync(path, S_IFREG, { mode, ...options }, new Uint8Array(), 'createFile'); - return new PreloadFile(this, path, flag, node.toStats(), new Uint8Array()); + return new LazyFile(this, path, flag, node.toStats()); } public async openFile(path: string, flag: string): Promise { await using tx = this.store.transaction(); const node = await this.findInode(tx, path, 'openFile'); - const data = (await tx.get(node.data)) ?? _throw(ErrnoError.With('ENODATA', path, 'openFile')); + //const data = (await tx.get(node.data)) ?? _throw(ErrnoError.With('ENODATA', path, 'openFile')); - return new PreloadFile(this, path, flag, node.toStats(), data); + return new LazyFile(this, path, flag, node.toStats()); } public openFileSync(path: string, flag: string): File { using tx = this.store.transaction(); const node = this.findInodeSync(tx, path, 'openFile'); - const data = tx.getSync(node.data) ?? _throw(ErrnoError.With('ENOENT', path, 'openFile')); + //const data = tx.getSync(node.data) ?? _throw(ErrnoError.With('ENODATA', path, 'openFile')); - return new PreloadFile(this, path, flag, node.toStats(), data); + return new LazyFile(this, path, flag, node.toStats()); } public async unlink(path: string): Promise { @@ -420,10 +420,15 @@ export class StoreFS extends FileSystem { const inode = await this.findInode(tx, path, 'write'); - const buffer = await tx.get(inode.data); + const buffer = growBuffer(await tx.get(inode.data), offset + data.byteLength); buffer.set(data, offset); - await this.sync(path, buffer, inode); + inode.update({ mtimeMs: Date.now(), size: buffer.byteLength }); + + await tx.set(inode.ino, serialize(inode)); + await tx.set(inode.data, buffer); + + await tx.commit(); } public writeSync(path: string, data: Uint8Array, offset: number): void { @@ -431,11 +436,11 @@ export class StoreFS extends FileSystem { const inode = this.findInodeSync(tx, path, 'write'); - inode.update({ mtimeMs: Date.now() }); - - const buffer = tx.getSync(inode.data); + const buffer = growBuffer(tx.getSync(inode.data), offset + data.byteLength); buffer.set(data, offset); + inode.update({ mtimeMs: Date.now(), size: buffer.byteLength }); + tx.setSync(inode.ino, serialize(inode)); tx.setSync(inode.data, buffer); diff --git a/src/file.ts b/src/file.ts index 1af53d62..ee423abf 100644 --- a/src/file.ts +++ b/src/file.ts @@ -2,6 +2,7 @@ import { Errno, ErrnoError } from './error.js'; import type { FileSystem } from './filesystem.js'; import './polyfills.js'; import { _chown, Stats, type StatsLike } from './stats.js'; +import { growBuffer } from './utils.js'; import { config } from './vfs/config.js'; import * as c from './vfs/constants.js'; @@ -377,7 +378,7 @@ export class PreloadFile extends File { } this.stats.size = length; // Truncate. - this._buffer = length ? this._buffer.slice(0, length) : new Uint8Array(); + this._buffer = length ? this._buffer.subarray(0, length) : new Uint8Array(); } public async truncate(length: number): Promise { @@ -399,26 +400,10 @@ export class PreloadFile extends File { this.dirty = true; const end = position + length; - const slice = buffer.slice(offset, offset + length); + const slice = buffer.subarray(offset, offset + length); - if (end > this.stats.size) { - this.stats.size = end; - if (end > this._buffer.byteLength) { - const { buffer } = this._buffer; - if ('resizable' in buffer && buffer.resizable && buffer.maxByteLength <= end) { - buffer.resize(end); - } else if ('growable' in buffer && buffer.growable && buffer.maxByteLength <= end) { - buffer.grow(end); - } else if (config.unsafeBufferReplace) { - this._buffer = slice; - } else { - // Extend the buffer! - const newBuffer = new Uint8Array(new ArrayBuffer(end, this.fs.metadata().noResizableBuffers ? {} : { maxByteLength })); - newBuffer.set(this._buffer); - this._buffer = newBuffer; - } - } - } + this._buffer = growBuffer(this._buffer, end); + if (end > this.stats.size) this.stats.size = end; this._buffer.set(slice, position); this.stats.mtimeMs = Date.now(); @@ -479,7 +464,7 @@ export class PreloadFile extends File { // No copy/read. Return immediately for better performance return bytesRead; } - const slice = this._buffer.slice(position, end); + const slice = this._buffer.subarray(position, end); new Uint8Array(buffer.buffer, buffer.byteOffset, buffer.byteLength).set(slice, offset); return bytesRead; } @@ -717,11 +702,9 @@ export class LazyFile extends File { this.dirty = true; const end = position + length; - const slice = buffer.slice(offset, offset + length); + const slice = buffer.subarray(offset, offset + length); - if (end > this.stats.size) { - this.stats.size = end; - } + if (end > this.stats.size) this.stats.size = end; this.stats.mtimeMs = Date.now(); this._position = position + slice.byteLength; @@ -738,7 +721,7 @@ export class LazyFile extends File { */ public async write(buffer: Uint8Array, offset: number = 0, length: number = buffer.byteLength - offset, position: number = this.position): Promise { const slice = this.prepareWrite(buffer, offset, length, position); - await this.fs.write(this.path, slice, offset); + await this.fs.write(this.path, slice, position); if (config.syncImmediately) await this.sync(); return slice.byteLength; } @@ -754,7 +737,7 @@ export class LazyFile extends File { */ public writeSync(buffer: Uint8Array, offset: number = 0, length: number = buffer.byteLength - offset, position: number = this.position): number { const slice = this.prepareWrite(buffer, offset, length, position); - this.fs.writeSync(this.path, slice, offset); + this.fs.writeSync(this.path, slice, position); if (config.syncImmediately) this.syncSync(); return slice.byteLength; } diff --git a/src/utils.ts b/src/utils.ts index 168aad8f..fff0d3b5 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -198,3 +198,42 @@ export function canary(path?: string, syscall?: string) { export function _throw(e: unknown): never { throw e; } + +interface ArrayBufferViewConstructor { + readonly prototype: ArrayBufferView; + new (length: number): ArrayBufferView; + new (array: ArrayLike): ArrayBufferView; + new (buffer: TArrayBuffer, byteOffset?: number, length?: number): ArrayBufferView; + new (array: ArrayLike | ArrayBuffer): ArrayBufferView; +} + +/** + * Grows a buffer if it isn't large enough + * @returns The original buffer if resized successfully, or a newly created buffer + * @internal Not for external use! + */ +export function growBuffer(buffer: T, newByteLength: number): T { + if (buffer.byteLength >= newByteLength) return buffer; + + if (ArrayBuffer.isView(buffer)) { + const newBuffer = growBuffer(buffer.buffer, newByteLength); + return new (buffer.constructor as ArrayBufferViewConstructor)(newBuffer, buffer.byteOffset, newByteLength) as T; + } + + const isShared = buffer instanceof SharedArrayBuffer; + + // Note: If true, the buffer must be resizable/growable because of the first check. + if (buffer.maxByteLength > newByteLength) { + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + isShared ? buffer.grow(newByteLength) : buffer.resize(newByteLength); + return buffer; + } + + if (!isShared) { + return buffer.transfer(newByteLength) as T; + } + + const newBuffer = new SharedArrayBuffer(newByteLength) as T & SharedArrayBuffer; + new Uint8Array(newBuffer).set(new Uint8Array(buffer)); + return newBuffer; +} diff --git a/src/vfs/async.ts b/src/vfs/async.ts index a59cf77f..30c5496f 100644 --- a/src/vfs/async.ts +++ b/src/vfs/async.ts @@ -696,7 +696,7 @@ export function createReadStream(this: V_Context, path: fs.PathLike, options?: B try { handle ||= await promises.open.call(context, path, 'r', options?.mode); const result = await handle.read(new Uint8Array(size), 0, size, handle.file.position); - stream.push(!result.bytesRead ? null : result.buffer.slice(0, result.bytesRead)); + stream.push(!result.bytesRead ? null : result.buffer.subarray(0, result.bytesRead)); handle.file.position += result.bytesRead; if (!result.bytesRead) { await handle.close(); diff --git a/src/vfs/promises.ts b/src/vfs/promises.ts index e4fef5d0..7a62d9be 100644 --- a/src/vfs/promises.ts +++ b/src/vfs/promises.ts @@ -212,7 +212,7 @@ export class FileHandle implements promises.FileHandle { controller.close(); return; } - controller.enqueue(result.buffer.slice(0, result.bytesRead)); + controller.enqueue(result.buffer.subarray(0, result.bytesRead)); position += result.bytesRead; if (++i >= maxChunks) { throw new ErrnoError(Errno.EFBIG, 'Too many iterations on readable stream', this.file.path, 'FileHandle.readableWebStream'); @@ -371,7 +371,7 @@ export class FileHandle implements promises.FileHandle { read: async (size: number) => { try { const result = await this.read(new Uint8Array(size), 0, size, this.file.position); - stream.push(!result.bytesRead ? null : result.buffer.slice(0, result.bytesRead)); // Push data or null for EOF + stream.push(!result.bytesRead ? null : result.buffer.subarray(0, result.bytesRead)); // Push data or null for EOF this.file.position += result.bytesRead; } catch (error) { stream.destroy(error as Error); diff --git a/tests/fs/write.test.ts b/tests/fs/write.test.ts index 38093c69..aa7b92c0 100644 --- a/tests/fs/write.test.ts +++ b/tests/fs/write.test.ts @@ -1,10 +1,9 @@ import assert from 'node:assert'; import { suite, test } from 'node:test'; import { fs } from '../common.js'; - +const fn = 'write.txt'; suite('write', () => { test('write file with specified content', async () => { - const fn = 'write.txt'; const expected = 'ümlaut.'; const handle = await fs.promises.open(fn, 'w', 0o644); @@ -20,10 +19,9 @@ suite('write', () => { }); test('write a buffer to a file', async () => { - const filename = 'write.txt'; const expected = Buffer.from('hello'); - const handle = await fs.promises.open(filename, 'w', 0o644); + const handle = await fs.promises.open(fn, 'w', 0o644); const written = await handle.write(expected, 0, expected.length, null); @@ -31,15 +29,12 @@ suite('write', () => { await handle.close(); - assert((await fs.promises.readFile(filename)).equals(expected)); + assert((await fs.promises.readFile(fn)).equals(expected)); - await fs.promises.unlink(filename); + await fs.promises.unlink(fn); }); -}); -suite('writeSync', () => { - test('write file with specified content', () => { - const fn = 'write.txt'; + test('writeSync file with specified content', () => { const fd = fs.openSync(fn, 'w'); let written = fs.writeSync(fd, ''); @@ -53,6 +48,6 @@ suite('writeSync', () => { fs.closeSync(fd); - assert(fs.readFileSync(fn, 'utf8') === 'foobár'); + assert.strictEqual(fs.readFileSync(fn, 'utf8'), 'foobár'); }); });