-
Notifications
You must be signed in to change notification settings - Fork 29.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
lib: expose FixedQueue internally and fix nextTick bug
A bug was introduced together with the FixedQueue implementation for process.nextTick which meant that the queue wouldn't necessarily fully clear on each run through. Fix it and abstract the data structure into an internal module that can later be used elsewhere. PR-URL: #20468 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
- Loading branch information
1 parent
1de67c7
commit 7b8e9ca
Showing
5 changed files
with
174 additions
and
116 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| 'use strict'; | ||
|
|
||
| // Currently optimal queue size, tested on V8 6.0 - 6.6. Must be power of two. | ||
| const kSize = 2048; | ||
| const kMask = kSize - 1; | ||
|
|
||
| // The FixedQueue is implemented as a singly-linked list of fixed-size | ||
| // circular buffers. It looks something like this: | ||
| // | ||
| // head tail | ||
| // | | | ||
| // v v | ||
| // +-----------+ <-----\ +-----------+ <------\ +-----------+ | ||
| // | [null] | \----- | next | \------- | next | | ||
| // +-----------+ +-----------+ +-----------+ | ||
| // | item | <-- bottom | item | <-- bottom | [empty] | | ||
| // | item | | item | | [empty] | | ||
| // | item | | item | | [empty] | | ||
| // | item | | item | | [empty] | | ||
| // | item | | item | bottom --> | item | | ||
| // | item | | item | | item | | ||
| // | ... | | ... | | ... | | ||
| // | item | | item | | item | | ||
| // | item | | item | | item | | ||
| // | [empty] | <-- top | item | | item | | ||
| // | [empty] | | item | | item | | ||
| // | [empty] | | [empty] | <-- top top --> | [empty] | | ||
| // +-----------+ +-----------+ +-----------+ | ||
| // | ||
| // Or, if there is only one circular buffer, it looks something | ||
| // like either of these: | ||
| // | ||
| // head tail head tail | ||
| // | | | | | ||
| // v v v v | ||
| // +-----------+ +-----------+ | ||
| // | [null] | | [null] | | ||
| // +-----------+ +-----------+ | ||
| // | [empty] | | item | | ||
| // | [empty] | | item | | ||
| // | item | <-- bottom top --> | [empty] | | ||
| // | item | | [empty] | | ||
| // | [empty] | <-- top bottom --> | item | | ||
| // | [empty] | | item | | ||
| // +-----------+ +-----------+ | ||
| // | ||
| // Adding a value means moving `top` forward by one, removing means | ||
| // moving `bottom` forward by one. After reaching the end, the queue | ||
| // wraps around. | ||
| // | ||
| // When `top === bottom` the current queue is empty and when | ||
| // `top + 1 === bottom` it's full. This wastes a single space of storage | ||
| // but allows much quicker checks. | ||
|
|
||
| const FixedCircularBuffer = class FixedCircularBuffer { | ||
| constructor() { | ||
| this.bottom = 0; | ||
| this.top = 0; | ||
| this.list = new Array(kSize); | ||
| this.next = null; | ||
| } | ||
|
|
||
| isEmpty() { | ||
| return this.top === this.bottom; | ||
| } | ||
|
|
||
| isFull() { | ||
| return ((this.top + 1) & kMask) === this.bottom; | ||
| } | ||
|
|
||
| push(data) { | ||
| this.list[this.top] = data; | ||
| this.top = (this.top + 1) & kMask; | ||
| } | ||
|
|
||
| shift() { | ||
| const nextItem = this.list[this.bottom]; | ||
| if (nextItem === undefined) | ||
| return null; | ||
| this.list[this.bottom] = undefined; | ||
| this.bottom = (this.bottom + 1) & kMask; | ||
| return nextItem; | ||
| } | ||
| }; | ||
|
|
||
| module.exports = class FixedQueue { | ||
| constructor() { | ||
| this.head = this.tail = new FixedCircularBuffer(); | ||
| } | ||
|
|
||
| isEmpty() { | ||
| return this.head.isEmpty(); | ||
| } | ||
|
|
||
| push(data) { | ||
| if (this.head.isFull()) { | ||
| // Head is full: Creates a new queue, sets the old queue's `.next` to it, | ||
| // and sets it as the new main queue. | ||
| this.head = this.head.next = new FixedCircularBuffer(); | ||
| } | ||
| this.head.push(data); | ||
| } | ||
|
|
||
| shift() { | ||
| const { tail } = this; | ||
| const next = tail.shift(); | ||
| if (tail.isEmpty() && tail.next !== null) { | ||
| // If there is another queue, it forms the new tail. | ||
| this.tail = tail.next; | ||
| } | ||
| return next; | ||
| } | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| // Flags: --expose-internals | ||
| 'use strict'; | ||
|
|
||
| require('../common'); | ||
|
|
||
| const assert = require('assert'); | ||
| const FixedQueue = require('internal/fixed_queue'); | ||
|
|
||
| { | ||
| const queue = new FixedQueue(); | ||
| assert.strictEqual(queue.head, queue.tail); | ||
| assert(queue.isEmpty()); | ||
| queue.push('a'); | ||
| assert(!queue.isEmpty()); | ||
| assert.strictEqual(queue.shift(), 'a'); | ||
| assert.strictEqual(queue.shift(), null); | ||
| } | ||
|
|
||
| { | ||
| const queue = new FixedQueue(); | ||
| for (let i = 0; i < 2047; i++) | ||
| queue.push('a'); | ||
| assert(queue.head.isFull()); | ||
| queue.push('a'); | ||
| assert(!queue.head.isFull()); | ||
|
|
||
| assert.notStrictEqual(queue.head, queue.tail); | ||
| for (let i = 0; i < 2047; i++) | ||
| assert.strictEqual(queue.shift(), 'a'); | ||
| assert.strictEqual(queue.head, queue.tail); | ||
| assert(!queue.isEmpty()); | ||
| assert.strictEqual(queue.shift(), 'a'); | ||
| assert(queue.isEmpty()); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| 'use strict'; | ||
|
|
||
| const common = require('../common'); | ||
|
|
||
| // This tests a highly specific regression tied to the FixedQueue size, which | ||
| // was introduced in Node.js 9.7.0: https://github.com/nodejs/node/pull/18617 | ||
| // More specifically, a nextTick list could potentially end up not fully | ||
| // clearing in one run through if exactly 2048 ticks were added after | ||
| // microtasks were executed within the nextTick loop. | ||
|
|
||
| process.nextTick(() => { | ||
| Promise.resolve(1).then(() => { | ||
| for (let i = 0; i < 2047; i++) | ||
| process.nextTick(common.mustCall()); | ||
| const immediate = setImmediate(common.mustNotCall()); | ||
| process.nextTick(common.mustCall(() => clearImmediate(immediate))); | ||
| }); | ||
| }); |