From 95a89885e9068a6687aa70e03d5a4c45e2b26872 Mon Sep 17 00:00:00 2001 From: JaoodxD Date: Wed, 22 May 2024 18:14:37 +0300 Subject: [PATCH 1/2] Revert commit to bring back up to 20% perf In one of the commits capacity optimization were made. It saved 1 unit of storage per circular buffer but actually takes up to 20% of performance. Link to commit: https://github.com/piscinajs/piscina/pull/555/commits/395e630e2f9791fdc04fbc05735694ff251c06a5 --- src/fixed-queue.ts | 9 ++------- test/fixed-queue.ts | 6 +++--- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/fixed-queue.ts b/src/fixed-queue.ts index 2bc2a4af..30053b0c 100644 --- a/src/fixed-queue.ts +++ b/src/fixed-queue.ts @@ -63,28 +63,25 @@ class FixedCircularBuffer { top: number list: Array next: FixedCircularBuffer | null - _size: number constructor () { this.bottom = 0; this.top = 0; this.list = new Array(kSize); this.next = null; - this._size = 0; } isEmpty () { - return this.top === this.bottom && this._size === 0; + return this.top === this.bottom; } isFull () { - return this.top === this.bottom && this._size === kSize; + return ((this.top + 1) & kMask) === this.bottom; } push (data:Task) { this.list[this.top] = data; this.top = (this.top + 1) & kMask; - this._size++; } shift () { @@ -92,7 +89,6 @@ class FixedCircularBuffer { if (nextItem === undefined) { return null; } this.list[this.bottom] = undefined; this.bottom = (this.bottom + 1) & kMask; - this._size--; return nextItem; } @@ -114,7 +110,6 @@ class FixedCircularBuffer { curr = next; } this.top = (this.top - 1) & kMask; - this._size--; } } diff --git a/test/fixed-queue.ts b/test/fixed-queue.ts index 085e0132..52877e1a 100644 --- a/test/fixed-queue.ts +++ b/test/fixed-queue.ts @@ -65,7 +65,7 @@ test('remove not queued task should not lead to errors', async ({ equal }) => { test('removing elements from intermediate CircularBuffer should not lead to issues', async ({ equal, same }) => { const queue = new FixedQueue(); - const batchSize = 2048; + const batchSize = 2047; const firstBatch = Array.from({ length: batchSize }, () => new QueueTask()); const secondBatch = Array.from({ length: batchSize }, () => new QueueTask()); @@ -96,7 +96,7 @@ test('removing elements from intermediate CircularBuffer should not lead to issu test('removing elements from first CircularBuffer should not lead to issues', async ({ equal, same }) => { const queue = new FixedQueue(); - const batchSize = 2048; + const batchSize = 2047; const firstBatch = Array.from({ length: batchSize }, () => new QueueTask()); const secondBatch = Array.from({ length: batchSize }, () => new QueueTask()); @@ -127,7 +127,7 @@ test('removing elements from first CircularBuffer should not lead to issues', as test('removing elements from last CircularBuffer should not lead to issues', async ({ equal, same }) => { const queue = new FixedQueue(); - const batchSize = 2048; + const batchSize = 2047; const firstBatch = Array.from({ length: batchSize }, () => new QueueTask()); const secondBatch = Array.from({ length: batchSize }, () => new QueueTask()); From 9422ee8ada1e4893c2c4cab00fc14705569392c3 Mon Sep 17 00:00:00 2001 From: JaoodxD Date: Fri, 24 May 2024 11:20:30 +0300 Subject: [PATCH 2/2] Add description for tests --- test/fixed-queue.ts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/fixed-queue.ts b/test/fixed-queue.ts index 52877e1a..782ffb30 100644 --- a/test/fixed-queue.ts +++ b/test/fixed-queue.ts @@ -63,8 +63,18 @@ test('remove not queued task should not lead to errors', async ({ equal }) => { }); test('removing elements from intermediate CircularBuffer should not lead to issues', async ({ equal, same }) => { + /* + The test intends to check following scenario: + 1) We fill the queue with 3 full circular buffers amount of items. + 2) Empty the middle circular buffer with remove(). + 3) This should lead to the removal of the middle buffer from the queue: + - Before emptying: tail buffer -> middle buffer -> head buffer. + - After emptying: tail buffer -> head buffer. + */ + const queue = new FixedQueue(); + // size of single circular buffer const batchSize = 2047; const firstBatch = Array.from({ length: batchSize }, () => new QueueTask()); @@ -94,8 +104,17 @@ test('removing elements from intermediate CircularBuffer should not lead to issu }); test('removing elements from first CircularBuffer should not lead to issues', async ({ equal, same }) => { + /* + The test intends to check following scenario: + 1) We fill the queue with 3 full circular buffers amount of items. + 2) Empty the first circular buffer with remove(). + 3) This should lead to the removal of the tail buffer from the queue: + - Before emptying: tail buffer -> middle buffer -> head buffer. + - After emptying: tail buffer (previously middle) -> head buffer. + */ const queue = new FixedQueue(); + // size of single circular buffer const batchSize = 2047; const firstBatch = Array.from({ length: batchSize }, () => new QueueTask()); @@ -125,8 +144,17 @@ test('removing elements from first CircularBuffer should not lead to issues', as }); test('removing elements from last CircularBuffer should not lead to issues', async ({ equal, same }) => { + /* + The test intends to check following scenario: + 1) We fill the queue with 3 full circular buffers amount of items. + 2) Empty the last circular buffer with remove(). + 3) This should lead to the removal of the head buffer from the queue: + - Before emptying: tail buffer -> middle buffer -> head buffer. + - After emptying: tail buffer -> head buffer (previously middle). + */ const queue = new FixedQueue(); + // size of single circular buffer const batchSize = 2047; const firstBatch = Array.from({ length: batchSize }, () => new QueueTask());