chore(perf): Revert commit to bring back up to 20% perf

[JaoodxD]

As mentioned here #555 (comment) 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 which caused perf drop: 395e630

[metcoder95]

Can we do benchmarks with payloads twice or three times its size?

[JaoodxD]

Do you mean to push something bigger than integers or just more items?
If the second option then it's already done.
Results could be seen here #555 (comment) for twice the size, 64 times the size and 1024 times the size of single circular buffer.

[metcoder95]

Oh, good call, sorry I missed that

[JaoodxD]

No problem.
Btw in the last few days I tried to play with circular buffer size, tried to preallocate and reuse ring buffers with a pool and so on but nothing really brings benefits, only performance degradations.
It feels like FixedQueue was created by a dark magician who put an "unbeatable" spell on it. 😅

[metcoder95]

It is pretty well optimized. Liked the tradeoff of memory for more performance 👌

[metcoder95]

Does it fails with 2048?
In the worst case, it will pre-allocate another buffer, isn't it?

[JaoodxD]

In general it doesn't fail but it should match the batch size due to test's intensions.
The test may require additional clarification with comments.
The main goal was to check following scenario:

1. We fill the queue with 3 full circular buffers amount of items.
2. Empty the middle circular buffer.
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.

My initial remove implementation didn't remove empty buffer so I wrote a test to fix it.

[metcoder95]

Nice, now is clear; maybe add that to the description of the test can help understand why are we doing that

[JaoodxD]

Although I added the description I must admit that the test is actually insufficient/fragile due to lack of internal checks.
As you rightly noticed it doesn't and it wouldn't break even if the size of internal circular buffer will ever changes.
We potentialy could try to really check the amount of buffers with something like following, but this approach will definetly be broken when we ship "real" private fields in further major release.

const queue = new FixedQueue()

let buffer = queue.tail
let count = 0

while (buffer !== null) {
  count++
  buffer = buffer.next
}

console.log(`The queue has ${count} buffers`)

[metcoder95]

Overall, I'd not recommend to alter the internals just for testing; I've always advocated for testing the outcome rather than expectations.
If we want to check how many internal circular buffers has been made, we can always check the overall outcome, we know that if two circular buffers are made, at least we should have 2048 + 1, this at the FixedTaskQueue level.

At the FixedQueue we can go deep but always using the interfaces as a bridge and not going beyond

[JaoodxD]

I completely agree that it's not the right way. I just worry about further consequences due to the lack of clarity.
Is there anything else blocking the PR from being landed?