From edcca599a878f63065f4bc38503b518523ac06ca Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Fri, 3 Jan 2025 12:57:16 +0100 Subject: [PATCH 01/12] Improve performance of some GraphNode's methods, and don't recalculate aabb a few times while rendering --- src/scene/graph-node.js | 237 ++++++++++++++++++-------- src/scene/mesh-instance.js | 11 +- src/scene/renderer/renderer.js | 5 +- src/scene/renderer/shadow-renderer.js | 2 +- 4 files changed, 175 insertions(+), 80 deletions(-) diff --git a/src/scene/graph-node.js b/src/scene/graph-node.js index 49eff9b6195..47134a70344 100644 --- a/src/scene/graph-node.js +++ b/src/scene/graph-node.js @@ -43,31 +43,6 @@ function createTest(attr, value) { }; } -/** - * Helper function to recurse findOne without calling createTest constantly. - * - * @param {GraphNode} node - Current node. - * @param {FindNodeCallback} test - Test function. - * @returns {GraphNode|null} A graph node that matches the search criteria. Returns null if no - * node is found. - */ -function findNode(node, test) { - if (test(node)) { - return node; - } - - const children = node._children; - const len = children.length; - for (let i = 0; i < len; ++i) { - const result = findNode(children[i], test); - if (result) { - return result; - } - } - - return null; -} - /** * Callback used by {@link GraphNode#find} and {@link GraphNode#findOne} to search through a graph * node and all of its descendants. @@ -98,6 +73,12 @@ function findNode(node, test) { * a powerful set of features that are leveraged by the `Entity` class. */ class GraphNode extends EventHandler { + /** + * @type {GraphNode[]} + * @ignore + */ + static _tmpGraphNodeStack = []; + /** * The non-unique name of a graph node. Defaults to 'Untitled'. * @@ -448,12 +429,30 @@ class GraphNode extends EventHandler { * @protected */ _notifyHierarchyStateChanged(node, enabled) { - node._onHierarchyStateChanged(enabled); - - const c = node._children; - for (let i = 0, len = c.length; i < len; i++) { - if (c[i]._enabled) { - this._notifyHierarchyStateChanged(c[i], enabled); + /** + * NOTE: GraphNode._tmpGraphNodeStack is not used here because _notifyHierarchyStateChanged can be called + * while other _notifyHierarchyStateChanged is still running because of _onHierarchyStateChanged + * + * @type {GraphNode[]} + */ + const stack = [node]; + + while (stack.length) { + const current = stack.pop(); + current._onHierarchyStateChanged(enabled); + + const c = current._children; + + for (let i = 0; i < c.length; i++) { + const child = c[i]; + + if (enabled) { + if (child._enabled) { + stack.push(child); + } + } else { + stack.push(child); + } } } } @@ -594,11 +593,22 @@ class GraphNode extends EventHandler { const results = []; const test = createTest(attr, value); - this.forEach((node) => { + const stack = GraphNode._tmpGraphNodeStack; + stack.push(this); + + while (stack.length) { + const node = stack.pop(); + if (test(node)) { results.push(node); } - }); + + const children = node._children; + + for (let i = 0; i < children.length; i++) { + stack.push(children[i]); + } + } return results; } @@ -629,7 +639,22 @@ class GraphNode extends EventHandler { */ findOne(attr, value) { const test = createTest(attr, value); - return findNode(this, test); + + const stack = GraphNode._tmpGraphNodeStack; + stack.push(this); + + while (stack.length) { + const current = stack.pop(); + if (test(current)) + return current; + + const children = current._children; + for (let i = 0; i < children.length; i++) { + stack.push(children[i]); + } + } + + return null; } /** @@ -653,21 +678,27 @@ class GraphNode extends EventHandler { * // Return all assets that tagged by (`carnivore` AND `mammal`) OR (`carnivore` AND `reptile`) * const meatEatingMammalsAndReptiles = node.findByTag(["carnivore", "mammal"], ["carnivore", "reptile"]); */ - findByTag() { - const query = arguments; + findByTag(...query) { const results = []; + const stack = GraphNode._tmpGraphNodeStack; + + const children = this._children; + for (let i = 0; i < children.length; i++) { + stack.push(children[i]); + } - const queryNode = (node, checkNode) => { - if (checkNode && node.tags.has(...query)) { + while (stack.length) { + const node = stack.pop(); + + if (node.tags.has(...query)) { results.push(node); } - for (let i = 0; i < node._children.length; i++) { - queryNode(node._children[i], true); + const children = node._children; + for (let i = 0; i < children.length; i++) { + stack.push(children[i]); } - }; - - queryNode(this, false); + } return results; } @@ -715,23 +746,36 @@ class GraphNode extends EventHandler { /** * Executes a provided function once on this graph node and all of its descendants. + * The method executes the provided function for each node in the graph in a breadth-first manner. * * @param {ForEachNodeCallback} callback - The function to execute on the graph node and each * descendant. - * @param {object} [thisArg] - Optional value to use as this when executing callback function. + * @param {object} [thisArg] - Optional value to use as this when executing the callback function. * @example - * // Log the path and name of each node in descendant tree starting with "parent" + * // Log the path and name of each node in the descendant tree starting with "parent" * parent.forEach(function (node) { * console.log(node.path + "/" + node.name); * }); */ forEach(callback, thisArg) { - callback.call(thisArg, this); + /** + * NOTE: GraphNode._tmpGraphNodeStack is not used here because forEach can be called within the callback + * + * @type {GraphNode[]} + */ + const queue = [this]; - const children = this._children; - const len = children.length; - for (let i = 0; i < len; ++i) { - children[i].forEach(callback, thisArg); + while (queue.length > 0) { + // shift() is used for breadth-first approach, use pop() for depth-first + const current = queue.shift(); + + callback.call(thisArg, current); + + const children = current._children; + + for (let i = 0; i < children.length; i++) { + queue.push(children[i]); + } } } @@ -1115,18 +1159,28 @@ class GraphNode extends EventHandler { /** @private */ _dirtifyWorldInternal() { - if (!this._dirtyWorld) { - this._frozen = false; - this._dirtyWorld = true; - for (let i = 0; i < this._children.length; i++) { - if (!this._children[i]._dirtyWorld) { - this._children[i]._dirtifyWorldInternal(); + const stack = GraphNode._tmpGraphNodeStack; + stack.push(this); + + while (stack.length > 0) { + const node = stack.pop(); + + if (!node._dirtyWorld) { + node._frozen = false; + node._dirtyWorld = true; + } + + node._dirtyNormal = true; + node._worldScaleSign = 0; + node._aabbVer++; + + const children = node._children; + for (let i = 0; i < children.length; i++) { + if (!children[i]._dirtyWorld) { + stack.push(children[i]); } } } - this._dirtyNormal = true; - this._worldScaleSign = 0; // world matrix is dirty, mark this flag dirty too - this._aabbVer++; } /** @@ -1345,8 +1399,28 @@ class GraphNode extends EventHandler { */ _fireOnHierarchy(name, nameHierarchy, parent) { this.fire(name, parent); - for (let i = 0; i < this._children.length; i++) { - this._children[i]._fireOnHierarchy(nameHierarchy, nameHierarchy, parent); + + /** + * NOTE: GraphNode._tmpGraphNodeStack is not used here because _fireOnHierarchy can be called + * while other _fireOnHierarchy is still running + * + * @type {GraphNode[]} + */ + const stack = [this]; + + const child = this._children; + + for (let i = 0; i < child.length; i++) { + stack.push(child[i]); + } + + while (stack.length) { + const curr = stack.pop(); + curr.fire(nameHierarchy, parent); + + for (let j = 0; j < curr._children.length; j++) { + stack.push(curr._children[j]); + } } } @@ -1390,15 +1464,21 @@ class GraphNode extends EventHandler { } /** - * Recurse the hierarchy and update the graph depth at each node. + * Iterates through the hierarchy and update the graph depth at each node. * * @private */ _updateGraphDepth() { - this._graphDepth = this._parent ? this._parent._graphDepth + 1 : 0; + const stack = GraphNode._tmpGraphNodeStack; + stack.push(this); - for (let i = 0, len = this._children.length; i < len; i++) { - this._children[i]._updateGraphDepth(); + while (stack.length) { + const n = stack.pop(); + n._graphDepth = n._parent ? n._parent._graphDepth + 1 : 0; + + for (let i = 0; i < n._children.length; i++) { + stack.push(n._children[i]); + } } } @@ -1501,22 +1581,29 @@ class GraphNode extends EventHandler { * @ignore */ syncHierarchy() { - if (!this._enabled) { + if (!this._enabled || this._frozen) { return; } - if (this._frozen) { - return; - } this._frozen = true; - if (this._dirtyLocal || this._dirtyWorld) { - this._sync(); - } + const stack = GraphNode._tmpGraphNodeStack; + stack.push(this); - const children = this._children; - for (let i = 0, len = children.length; i < len; i++) { - children[i].syncHierarchy(); + while (stack.length > 0) { + const node = stack.pop(); + if (!node._enabled || node._frozen) { + continue; + } + + if (node._dirtyLocal || node._dirtyWorld) { + node._sync(); + } + + const children = node._children; + for (let i = children.length - 1; i >= 0; i--) { + stack.push(children[i]); + } } } diff --git a/src/scene/mesh-instance.js b/src/scene/mesh-instance.js index 4b8fe23f1eb..4a102b64238 100644 --- a/src/scene/mesh-instance.js +++ b/src/scene/mesh-instance.js @@ -277,6 +277,8 @@ class MeshInstance { */ pick = true; + _aabbUpdateIndex = NaN; + /** * The stencil parameters for front faces or null if no stencil is enabled. * @@ -945,7 +947,7 @@ class MeshInstance { * @returns {boolean} - True if the mesh instance is visible by the camera, false otherwise. * @ignore */ - _isVisible(camera) { + _isVisible(camera, aabbUpdateIndex) { if (this.visible) { @@ -954,8 +956,11 @@ class MeshInstance { return this.isVisibleFunc(camera); } - _tempSphere.center = this.aabb.center; // this line evaluates aabb - _tempSphere.radius = this._aabb.halfExtents.length(); + const aabb = this._aabbUpdateIndex === aabbUpdateIndex && this._aabb || this.aabb; + this._aabbUpdateIndex = aabbUpdateIndex; + + _tempSphere.center = aabb.center; // this line evaluates aabb + _tempSphere.radius = aabb.halfExtents.length(); return camera.frustum.containsSphere(_tempSphere) > 0; } diff --git a/src/scene/renderer/renderer.js b/src/scene/renderer/renderer.js index 945f9a44234..24f3a70d0d0 100644 --- a/src/scene/renderer/renderer.js +++ b/src/scene/renderer/renderer.js @@ -158,6 +158,8 @@ class Renderer { blueNoise = new BlueNoise(123); + _aabbUpdateIndex = 0; + /** * Create a new instance. * @@ -916,7 +918,7 @@ class Renderer { const drawCall = drawCalls[i]; if (drawCall.visible) { - const visible = !doCull || !drawCall.cull || drawCall._isVisible(camera); + const visible = !doCull || !drawCall.cull || drawCall._isVisible(camera, this._aabbUpdateIndex); if (visible) { drawCall.visibleThisFrame = true; @@ -1127,6 +1129,7 @@ class Renderer { */ cullComposition(comp) { + this._aabbUpdateIndex++; // #if _PROFILER const cullTime = now(); // #endif diff --git a/src/scene/renderer/shadow-renderer.js b/src/scene/renderer/shadow-renderer.js index b61adea89d5..dcfb68e94d0 100644 --- a/src/scene/renderer/shadow-renderer.js +++ b/src/scene/renderer/shadow-renderer.js @@ -142,7 +142,7 @@ class ShadowRenderer { const meshInstance = meshInstances[i]; if (meshInstance.castShadow) { - if (!meshInstance.cull || meshInstance._isVisible(camera)) { + if (!meshInstance.cull || meshInstance._isVisible(camera, this.renderer._aabbUpdateIndex)) { meshInstance.visibleThisFrame = true; visible.push(meshInstance); } From c85208b8ec10c41874d06df85a848dd5251b839a Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Fri, 3 Jan 2025 13:24:01 +0100 Subject: [PATCH 02/12] Fix linter comment --- src/scene/graph-node.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/scene/graph-node.js b/src/scene/graph-node.js index 47134a70344..17682585b6e 100644 --- a/src/scene/graph-node.js +++ b/src/scene/graph-node.js @@ -645,8 +645,9 @@ class GraphNode extends EventHandler { while (stack.length) { const current = stack.pop(); - if (test(current)) + if (test(current)) { return current; + } const children = current._children; for (let i = 0; i < children.length; i++) { From 00dce36b167392cc825746be37f4d5db351de634 Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Fri, 3 Jan 2025 14:33:30 +0100 Subject: [PATCH 03/12] Minor fix --- src/scene/graph-node.js | 8 +++++--- src/scene/mesh-instance.js | 6 ++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/scene/graph-node.js b/src/scene/graph-node.js index 17682585b6e..2412da08e62 100644 --- a/src/scene/graph-node.js +++ b/src/scene/graph-node.js @@ -1586,22 +1586,24 @@ class GraphNode extends EventHandler { return; } - this._frozen = true; - const stack = GraphNode._tmpGraphNodeStack; stack.push(this); - while (stack.length > 0) { + while (stack.length) { const node = stack.pop(); + if (!node._enabled || node._frozen) { continue; } + node._frozen = true; + if (node._dirtyLocal || node._dirtyWorld) { node._sync(); } const children = node._children; + for (let i = children.length - 1; i >= 0; i--) { stack.push(children[i]); } diff --git a/src/scene/mesh-instance.js b/src/scene/mesh-instance.js index 4a102b64238..49063288581 100644 --- a/src/scene/mesh-instance.js +++ b/src/scene/mesh-instance.js @@ -956,10 +956,12 @@ class MeshInstance { return this.isVisibleFunc(camera); } - const aabb = this._aabbUpdateIndex === aabbUpdateIndex && this._aabb || this.aabb; + const aabb = this._aabbUpdateIndex === aabbUpdateIndex + ? this._aabb + : this.aabb; // this line evaluates aabb this._aabbUpdateIndex = aabbUpdateIndex; - _tempSphere.center = aabb.center; // this line evaluates aabb + _tempSphere.center = aabb.center; _tempSphere.radius = aabb.halfExtents.length(); return camera.frustum.containsSphere(_tempSphere) > 0; From d96f15efa051a4b67ed781eae003425e0fc62103 Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Fri, 3 Jan 2025 14:44:48 +0100 Subject: [PATCH 04/12] Linter comment --- src/scene/mesh-instance.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/scene/mesh-instance.js b/src/scene/mesh-instance.js index 49063288581..2e8517d0750 100644 --- a/src/scene/mesh-instance.js +++ b/src/scene/mesh-instance.js @@ -956,9 +956,7 @@ class MeshInstance { return this.isVisibleFunc(camera); } - const aabb = this._aabbUpdateIndex === aabbUpdateIndex - ? this._aabb - : this.aabb; // this line evaluates aabb + const aabb = this._aabbUpdateIndex === aabbUpdateIndex ? this._aabb: this.aabb; // this line evaluates aabb this._aabbUpdateIndex = aabbUpdateIndex; _tempSphere.center = aabb.center; From 484dba66f056750f6e5a810a13670ce17f802d33 Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Fri, 3 Jan 2025 14:47:03 +0100 Subject: [PATCH 05/12] Linter comment --- src/scene/mesh-instance.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scene/mesh-instance.js b/src/scene/mesh-instance.js index 2e8517d0750..85f5caaef36 100644 --- a/src/scene/mesh-instance.js +++ b/src/scene/mesh-instance.js @@ -956,7 +956,7 @@ class MeshInstance { return this.isVisibleFunc(camera); } - const aabb = this._aabbUpdateIndex === aabbUpdateIndex ? this._aabb: this.aabb; // this line evaluates aabb + const aabb = this._aabbUpdateIndex === aabbUpdateIndex ? this._aabb : this.aabb; // this line evaluates aabb this._aabbUpdateIndex = aabbUpdateIndex; _tempSphere.center = aabb.center; From 7c18f99092ad681595e8d7291d8ad549f7c767a2 Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Fri, 3 Jan 2025 18:12:00 +0100 Subject: [PATCH 06/12] Minor fix: https://github.com/playcanvas/engine/pull/7245#discussion_r1901799780 --- src/framework/lightmapper/lightmapper.js | 2 +- src/scene/graph-node.js | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/framework/lightmapper/lightmapper.js b/src/framework/lightmapper/lightmapper.js index 4b3147b3d44..1d72d9ad1ca 100644 --- a/src/framework/lightmapper/lightmapper.js +++ b/src/framework/lightmapper/lightmapper.js @@ -811,7 +811,7 @@ class Lightmapper { const meshInstances = bakeNode.meshInstances; for (let i = 0; i < meshInstances.length; i++) { - if (meshInstances[i]._isVisible(shadowCam)) { + if (meshInstances[i]._isVisible(shadowCam, this.renderer._aabbUpdateIndex)) { nodeVisible = true; break; } diff --git a/src/scene/graph-node.js b/src/scene/graph-node.js index 2412da08e62..679b77161ec 100644 --- a/src/scene/graph-node.js +++ b/src/scene/graph-node.js @@ -77,7 +77,7 @@ class GraphNode extends EventHandler { * @type {GraphNode[]} * @ignore */ - static _tmpGraphNodeStack = []; + static _stack = []; /** * The non-unique name of a graph node. Defaults to 'Untitled'. @@ -430,7 +430,7 @@ class GraphNode extends EventHandler { */ _notifyHierarchyStateChanged(node, enabled) { /** - * NOTE: GraphNode._tmpGraphNodeStack is not used here because _notifyHierarchyStateChanged can be called + * NOTE: GraphNode._stack is not used here because _notifyHierarchyStateChanged can be called * while other _notifyHierarchyStateChanged is still running because of _onHierarchyStateChanged * * @type {GraphNode[]} @@ -593,7 +593,7 @@ class GraphNode extends EventHandler { const results = []; const test = createTest(attr, value); - const stack = GraphNode._tmpGraphNodeStack; + const stack = GraphNode._stack; stack.push(this); while (stack.length) { @@ -640,7 +640,7 @@ class GraphNode extends EventHandler { findOne(attr, value) { const test = createTest(attr, value); - const stack = GraphNode._tmpGraphNodeStack; + const stack = GraphNode._stack; stack.push(this); while (stack.length) { @@ -681,7 +681,7 @@ class GraphNode extends EventHandler { */ findByTag(...query) { const results = []; - const stack = GraphNode._tmpGraphNodeStack; + const stack = GraphNode._stack; const children = this._children; for (let i = 0; i < children.length; i++) { @@ -760,7 +760,7 @@ class GraphNode extends EventHandler { */ forEach(callback, thisArg) { /** - * NOTE: GraphNode._tmpGraphNodeStack is not used here because forEach can be called within the callback + * NOTE: GraphNode._stack is not used here because forEach can be called within the callback * * @type {GraphNode[]} */ @@ -1160,7 +1160,7 @@ class GraphNode extends EventHandler { /** @private */ _dirtifyWorldInternal() { - const stack = GraphNode._tmpGraphNodeStack; + const stack = GraphNode._stack; stack.push(this); while (stack.length > 0) { @@ -1402,7 +1402,7 @@ class GraphNode extends EventHandler { this.fire(name, parent); /** - * NOTE: GraphNode._tmpGraphNodeStack is not used here because _fireOnHierarchy can be called + * NOTE: GraphNode._stack is not used here because _fireOnHierarchy can be called * while other _fireOnHierarchy is still running * * @type {GraphNode[]} @@ -1470,7 +1470,7 @@ class GraphNode extends EventHandler { * @private */ _updateGraphDepth() { - const stack = GraphNode._tmpGraphNodeStack; + const stack = GraphNode._stack; stack.push(this); while (stack.length) { @@ -1586,7 +1586,7 @@ class GraphNode extends EventHandler { return; } - const stack = GraphNode._tmpGraphNodeStack; + const stack = GraphNode._stack; stack.push(this); while (stack.length) { From 6beff4689dc4c2f3010098ac14a3e60afe68fb12 Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Fri, 3 Jan 2025 18:12:42 +0100 Subject: [PATCH 07/12] Minor fix: https://github.com/playcanvas/engine/pull/7245#discussion_r1901801966 --- src/scene/graph-node.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scene/graph-node.js b/src/scene/graph-node.js index 679b77161ec..4c008995849 100644 --- a/src/scene/graph-node.js +++ b/src/scene/graph-node.js @@ -75,7 +75,7 @@ function createTest(attr, value) { class GraphNode extends EventHandler { /** * @type {GraphNode[]} - * @ignore + * @private */ static _stack = []; From 352be44775303e1f0a20347ebec8ff65b65dc193 Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Sun, 5 Jan 2025 14:23:57 +0100 Subject: [PATCH 08/12] Add the Queue class, add the free method to the ObjectPool class, add tests for them, use the classes in the forEach method of GraphNode --- src/core/object-pool.js | 70 +++++++++++++++++----- src/core/queue.js | 102 +++++++++++++++++++++++++++++++++ src/scene/graph-node.js | 47 +++++++++------ test/core/object-pool.test.mjs | 49 ++++++++++++++++ test/core/queue.test.mjs | 80 ++++++++++++++++++++++++++ 5 files changed, 319 insertions(+), 29 deletions(-) create mode 100644 src/core/queue.js create mode 100644 test/core/object-pool.test.mjs create mode 100644 test/core/queue.test.mjs diff --git a/src/core/object-pool.js b/src/core/object-pool.js index 46e089c7022..8a00a80e5c5 100644 --- a/src/core/object-pool.js +++ b/src/core/object-pool.js @@ -29,6 +29,15 @@ class ObjectPool { */ _count = 0; + /** + * A map from object references to their index in `_pool`. This is used to determine + * whether an object is actually allocated from this pool (and at which index). + * + * @type {WeakMap, number>} + * @private + */ + _objToIndexMap = new WeakMap(); + /** * @param {T} constructorFunc - The constructor function for the * objects in the pool. @@ -40,18 +49,6 @@ class ObjectPool { this._resize(size); } - /** - * @param {number} size - The number of object instances to allocate. - * @private - */ - _resize(size) { - if (size > this._pool.length) { - for (let i = this._pool.length; i < size; i++) { - this._pool[i] = new this._constructor(); - } - } - } - /** * Returns an object instance from the pool. If no instances are available, the pool will be * doubled in size and a new instance will be returned. @@ -60,11 +57,43 @@ class ObjectPool { */ allocate() { if (this._count >= this._pool.length) { - this._resize(this._pool.length * 2); + this._resize(Math.max(1, this._pool.length * 2)); } return this._pool[this._count++]; } + /** + * Attempts to free the given object back into the pool. This only works if the object + * was previously allocated (i.e. it exists in `_objToIndexMap`) and is still in use + * (i.e., its index is below `_count`). + * + * @param {InstanceType} obj + * @returns {boolean} Whether freeing succeeded. + */ + free(obj) { + const index = this._objToIndexMap.get(obj); + if (index === undefined) { + return false; + } + + if (index >= this._count) { + return false; + } + + // Swap this object with the last allocated object, then decrement `_count` + const lastIndex = this._count - 1; + const lastObj = this._pool[lastIndex]; + + this._pool[index] = lastObj; + this._pool[lastIndex] = obj; + + this._objToIndexMap.set(lastObj, index); + this._objToIndexMap.set(obj, lastIndex); + + this._count -= 1; + return true; + } + /** * All object instances in the pool will be available again. The pool itself will not be * resized. @@ -72,6 +101,21 @@ class ObjectPool { freeAll() { this._count = 0; } + + /** + * @param {number} size - The number of object instances to allocate. + * @private + */ + _resize(size) { + if (size > this._pool.length) { + for (let i = this._pool.length; i < size; i++) { + const obj = new this._constructor(); + this._pool[i] = obj; + + this._objToIndexMap.set(obj, i); + } + } + } } export { ObjectPool }; diff --git a/src/core/queue.js b/src/core/queue.js new file mode 100644 index 00000000000..586eaab7fee --- /dev/null +++ b/src/core/queue.js @@ -0,0 +1,102 @@ + +/** + * A circular queue that automatically extends its capacity when full. + * This implementation uses a fixed-size array to store elements and + * supports efficient enqueue and dequeue operations. + * It is recommended to use `initialCapacity` that is close to **real-world** usage. + */ +class Queue { + constructor(initialCapacity = 8) { + this._storage = new Array(initialCapacity); + + this._capacity = initialCapacity; + + this._head = 0; + + this._size = 0; + } + + /** + * Enqueue (push) a value to the back of the queue. + * Automatically extends capacity if the queue is full. + * @param {*} value The value to enqueue. + */ + enqueue(value) { + if (this._size === this._capacity) { + this.resize(this._capacity * 2); + } + + const tailIndex = (this._head + this._size) % this._capacity; + this._storage[tailIndex] = value; + this._size++; + } + + /** + * Dequeue (pop) a value from the front of the queue. + * Returns undefined if the queue is empty. + * @returns {*} + */ + dequeue() { + if (this.isEmpty()) { + return undefined; + } + + const value = this._storage[this._head]; + this._storage[this._head] = undefined; + this._head = (this._head + 1) % this._capacity; + this._size--; + + return value; + } + + /** + * Return the front element without removing it. + * @returns {*} + */ + peek() { + if (this.isEmpty()) { + return undefined; + } + + return this._storage[this._head]; + } + + /** + * The current number of elements in the queue. + * @returns {number} + */ + size() { + return this._size; + } + + /** + * Check if the queue is empty. + * @returns {boolean} + */ + isEmpty() { + return this._size === 0; + } + + /** + * Internal method to resize the underlying storage. + * @param {number} size The new capacity for the queue. + */ + resize(size) { + if (size <= this._size) { + return; + } + + const newStorage = new Array(size); + + for (let i = 0; i < this._size; i++) { + const index = (this._head + i) % this._capacity; + newStorage[i] = this._storage[index]; + } + + this._head = 0; + this._capacity = size; + this._storage = newStorage; + } +} + +export { Queue }; diff --git a/src/scene/graph-node.js b/src/scene/graph-node.js index 4c008995849..39e5296d7fc 100644 --- a/src/scene/graph-node.js +++ b/src/scene/graph-node.js @@ -7,6 +7,9 @@ import { Mat4 } from '../core/math/mat4.js'; import { Quat } from '../core/math/quat.js'; import { Vec3 } from '../core/math/vec3.js'; +import { Queue } from '../core/queue.js'; +import { ObjectPool } from '../core/object-pool.js'; + const scaleCompensatePosTransform = new Mat4(); const scaleCompensatePos = new Vec3(); const scaleCompensateRot = new Quat(); @@ -79,6 +82,22 @@ class GraphNode extends EventHandler { */ static _stack = []; + /** + * It is a pool of graph node queues that are used to reduce memory allocation overhead + * + * @type {ObjectPool} + * @private + */ + static _queuePool = new ObjectPool(Queue, 1); + + /** + * Maximum number of nodes that can be stored in a graph node queue. + * + * @type {number} + * @private + */ + static _maxQueueSize = 512; + /** * The non-unique name of a graph node. Defaults to 'Untitled'. * @@ -759,25 +778,21 @@ class GraphNode extends EventHandler { * }); */ forEach(callback, thisArg) { - /** - * NOTE: GraphNode._stack is not used here because forEach can be called within the callback - * - * @type {GraphNode[]} - */ - const queue = [this]; - - while (queue.length > 0) { - // shift() is used for breadth-first approach, use pop() for depth-first - const current = queue.shift(); - - callback.call(thisArg, current); - - const children = current._children; - + const queue = GraphNode._queuePool.allocate(); + queue.resize(GraphNode._maxQueueSize); + queue.enqueue(this); + + while (queue.size()) { + const current = queue.dequeue() + callback(current); + const children = current.children; for (let i = 0; i < children.length; i++) { - queue.push(children[i]); + queue.enqueue(children[i]); } } + + GraphNode._maxQueueSize = Math.max(GraphNode._maxQueueSize, queue.size()); + GraphNode._queuePool.free(queue); } /** diff --git a/test/core/object-pool.test.mjs b/test/core/object-pool.test.mjs new file mode 100644 index 00000000000..0dbe8bd578c --- /dev/null +++ b/test/core/object-pool.test.mjs @@ -0,0 +1,49 @@ +import { expect } from 'chai'; + +import { ObjectPool } from '../../src/core/object-pool.js'; + +class SampleObject { + constructor() { + this.value = 0; + } +} + +describe('ObjectPool', function () { + let pool; + + beforeEach(function () { + pool = new ObjectPool(SampleObject, 2); + }); + + it('should allocate an object from the pool', function () { + const obj = pool.allocate(); + expect(obj).to.be.an.instanceof(SampleObject); + }); + + it('should resize the pool when allocation exceeds size', function () { + const obj1 = pool.allocate(); + const obj2 = pool.allocate(); + const obj3 = pool.allocate(); + expect(obj3).to.be.an.instanceof(SampleObject); + }); + + it('should free an allocated object back to the pool', function () { + const obj = pool.allocate(); + const success = pool.free(obj); + expect(success).to.be.true; + }); + + it('should not free an object not allocated from the pool', function () { + const obj = new SampleObject(); + const success = pool.free(obj); + expect(success).to.be.false; + }); + + it('should free all allocated objects', function () { + pool.allocate(); + pool.allocate(); + pool.freeAll(); + const obj = pool.allocate(); + expect(obj).to.be.an.instanceof(SampleObject); + }); +}); diff --git a/test/core/queue.test.mjs b/test/core/queue.test.mjs new file mode 100644 index 00000000000..3b6e48b8919 --- /dev/null +++ b/test/core/queue.test.mjs @@ -0,0 +1,80 @@ +import { expect } from 'chai'; + +import { Queue } from '../../src/core/queue.js'; + +describe('Queue', function () { + let queue; + + beforeEach(function () { + queue = new Queue(2); + }); + + it('should initialize as empty', function () { + expect(queue.isEmpty()).to.be.true; + expect(queue.size()).to.equal(0); + }); + + it('should enqueue elements into the queue', function () { + queue.enqueue(1); + expect(queue.isEmpty()).to.be.false; + expect(queue.size()).to.equal(1); + expect(queue.peek()).to.equal(1); + }); + + it('should dequeue elements from the queue', function () { + queue.enqueue(1); + queue.enqueue(2); + const dequeued = queue.dequeue(); + expect(dequeued).to.equal(1); + expect(queue.size()).to.equal(1); + expect(queue.peek()).to.equal(2); + }); + + it('should return undefined when dequeuing from an empty queue', function () { + const dequeued = queue.dequeue(); + expect(dequeued).to.be.undefined; + expect(queue.isEmpty()).to.be.true; + }); + + it('should peek at the front element without dequeuing it', function () { + queue.enqueue(1); + const front = queue.peek(); + expect(front).to.equal(1); + expect(queue.size()).to.equal(1); + }); + + it('should resize when capacity is exceeded', function () { + queue.enqueue(1); + queue.enqueue(2); + queue.enqueue(3); // This should trigger a resize + expect(queue.size()).to.equal(3); + expect(queue.peek()).to.equal(1); + }); + + it('should maintain order after resizing', function () { + queue.enqueue(1); + queue.enqueue(2); + queue.enqueue(3); + expect(queue.dequeue()).to.equal(1); + expect(queue.dequeue()).to.equal(2); + expect(queue.dequeue()).to.equal(3); + }); + + it('should correctly report if the queue is empty', function () { + expect(queue.isEmpty()).to.be.true; + queue.enqueue(1); + expect(queue.isEmpty()).to.be.false; + queue.dequeue(); + expect(queue.isEmpty()).to.be.true; + }); + + it('should correctly report the size of the queue', function () { + expect(queue.size()).to.equal(0); + queue.enqueue(1); + expect(queue.size()).to.equal(1); + queue.enqueue(2); + expect(queue.size()).to.equal(2); + queue.dequeue(); + expect(queue.size()).to.equal(1); + }); +}); From 9e12552fe289966edbfd422fe77f7a1cf9d78793 Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Sun, 5 Jan 2025 14:33:44 +0100 Subject: [PATCH 09/12] Fix linter comment --- src/core/object-pool.js | 2 +- src/core/queue.js | 19 +++++++------------ src/scene/graph-node.js | 2 +- test/core/object-pool.test.mjs | 8 ++++---- 4 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/core/object-pool.js b/src/core/object-pool.js index 8a00a80e5c5..39f82e37583 100644 --- a/src/core/object-pool.js +++ b/src/core/object-pool.js @@ -67,7 +67,7 @@ class ObjectPool { * was previously allocated (i.e. it exists in `_objToIndexMap`) and is still in use * (i.e., its index is below `_count`). * - * @param {InstanceType} obj + * @param {InstanceType} obj - The object instance to be freed back into the pool. * @returns {boolean} Whether freeing succeeded. */ free(obj) { diff --git a/src/core/queue.js b/src/core/queue.js index 586eaab7fee..a08b0f5c889 100644 --- a/src/core/queue.js +++ b/src/core/queue.js @@ -1,4 +1,3 @@ - /** * A circular queue that automatically extends its capacity when full. * This implementation uses a fixed-size array to store elements and @@ -19,7 +18,7 @@ class Queue { /** * Enqueue (push) a value to the back of the queue. * Automatically extends capacity if the queue is full. - * @param {*} value The value to enqueue. + * @param {*} value - The value to enqueue. */ enqueue(value) { if (this._size === this._capacity) { @@ -32,9 +31,8 @@ class Queue { } /** - * Dequeue (pop) a value from the front of the queue. - * Returns undefined if the queue is empty. - * @returns {*} + * @returns {*} The value at the front of the queue after dequeuing, + * or undefined if the queue is empty. */ dequeue() { if (this.isEmpty()) { @@ -50,8 +48,7 @@ class Queue { } /** - * Return the front element without removing it. - * @returns {*} + * @returns {*} The value at the front of the queue without dequeuing it. */ peek() { if (this.isEmpty()) { @@ -62,16 +59,14 @@ class Queue { } /** - * The current number of elements in the queue. - * @returns {number} + * @returns {number} The current number of elements in the queue. */ size() { return this._size; } /** - * Check if the queue is empty. - * @returns {boolean} + * @returns {boolean} True if the queue is empty, false otherwise. */ isEmpty() { return this._size === 0; @@ -79,7 +74,7 @@ class Queue { /** * Internal method to resize the underlying storage. - * @param {number} size The new capacity for the queue. + * @param {number} size - The new capacity for the queue. */ resize(size) { if (size <= this._size) { diff --git a/src/scene/graph-node.js b/src/scene/graph-node.js index 39e5296d7fc..6e14780d31d 100644 --- a/src/scene/graph-node.js +++ b/src/scene/graph-node.js @@ -783,7 +783,7 @@ class GraphNode extends EventHandler { queue.enqueue(this); while (queue.size()) { - const current = queue.dequeue() + const current = queue.dequeue(); callback(current); const children = current.children; for (let i = 0; i < children.length; i++) { diff --git a/test/core/object-pool.test.mjs b/test/core/object-pool.test.mjs index 0dbe8bd578c..bff82164c73 100644 --- a/test/core/object-pool.test.mjs +++ b/test/core/object-pool.test.mjs @@ -21,10 +21,10 @@ describe('ObjectPool', function () { }); it('should resize the pool when allocation exceeds size', function () { - const obj1 = pool.allocate(); - const obj2 = pool.allocate(); - const obj3 = pool.allocate(); - expect(obj3).to.be.an.instanceof(SampleObject); + pool.allocate(); + pool.allocate(); + const obj = pool.allocate(); + expect(obj).to.be.an.instanceof(SampleObject); }); it('should free an allocated object back to the pool', function () { From 776dfa2f8bc63a97ec1592bca43e1087c1c8c34c Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Mon, 6 Jan 2025 04:37:51 +0100 Subject: [PATCH 10/12] Update Queue. Use offsets --- src/core/object-pool.js | 3 +- src/core/queue.js | 120 ++++++++++++------- src/scene/graph-node.js | 202 +++++++++++++++----------------- test/core/queue.test.mjs | 243 ++++++++++++++++++++++++++++++--------- 4 files changed, 359 insertions(+), 209 deletions(-) diff --git a/src/core/object-pool.js b/src/core/object-pool.js index 39f82e37583..09a38938e3c 100644 --- a/src/core/object-pool.js +++ b/src/core/object-pool.js @@ -64,8 +64,7 @@ class ObjectPool { /** * Attempts to free the given object back into the pool. This only works if the object - * was previously allocated (i.e. it exists in `_objToIndexMap`) and is still in use - * (i.e., its index is below `_count`). + * was previously allocated and is still in use. * * @param {InstanceType} obj - The object instance to be freed back into the pool. * @returns {boolean} Whether freeing succeeded. diff --git a/src/core/queue.js b/src/core/queue.js index a08b0f5c889..cd00fa94766 100644 --- a/src/core/queue.js +++ b/src/core/queue.js @@ -3,36 +3,97 @@ * This implementation uses a fixed-size array to store elements and * supports efficient enqueue and dequeue operations. * It is recommended to use `initialCapacity` that is close to **real-world** usage. + * @template T */ class Queue { + /** + * Create a new queue. + * @param {number} [initialCapacity=8] - The initial capacity of the queue. + */ constructor(initialCapacity = 8) { + /** + * Underlying storage for the queue. + * @type {Array} + * @private + */ this._storage = new Array(initialCapacity); - this._capacity = initialCapacity; - + /** + * The head (front) index. + * @type {number} + * @private + */ this._head = 0; - this._size = 0; + /** + * The current number of elements in the queue. + * @type {number} + * @private + */ + this._length = 0; + } + + /** + * The current number of elements in the queue. + * @type {number} + * @readonly + */ + get length() { + return this._length; + } + + /** + * The capacity of the queue. + * @type {number} + * @readonly + */ + get capacity() { + return this._storage.length; + } + + /** + * Change the capacity of the underlying storage. + * Does not shrink capacity if new capacity is less than or equal to the current length. + * @param {number} capacity - The new capacity for the queue. + */ + set capacity(capacity) { + if (capacity <= this._length) { + return; + } + + const oldCapacity = this._storage.length; + this._storage.length = capacity; + + // Handle wrap-around scenario by moving elements. + if (this._head + this._length > oldCapacity) { + const endLength = oldCapacity - this._head; + for (let i = 0; i < endLength; i++) { + this._storage[capacity - endLength + i] = this._storage[this._head + i]; + } + this._head = capacity - endLength; + } } /** * Enqueue (push) a value to the back of the queue. * Automatically extends capacity if the queue is full. - * @param {*} value - The value to enqueue. + * @param {T} value - The value to enqueue. + * @returns {number} The new length of the queue. */ enqueue(value) { - if (this._size === this._capacity) { - this.resize(this._capacity * 2); + if (this._length === this._storage.length) { + this.capacity = this._storage.length * 2; } - const tailIndex = (this._head + this._size) % this._capacity; + const tailIndex = (this._head + this._length) % this._storage.length; this._storage[tailIndex] = value; - this._size++; + this._length++; + return this._length; } /** - * @returns {*} The value at the front of the queue after dequeuing, - * or undefined if the queue is empty. + * Dequeue (pop) a value from the front of the queue. + * @returns {T|undefined} The dequeued value, or `undefined` if the queue is empty. */ dequeue() { if (this.isEmpty()) { @@ -41,56 +102,29 @@ class Queue { const value = this._storage[this._head]; this._storage[this._head] = undefined; - this._head = (this._head + 1) % this._capacity; - this._size--; + this._head = (this._head + 1) % this._storage.length; + this._length--; return value; } /** - * @returns {*} The value at the front of the queue without dequeuing it. + * Returns the value at the front of the queue without removing it. + * @returns {T|undefined} The front value, or `undefined` if the queue is empty. */ peek() { if (this.isEmpty()) { return undefined; } - return this._storage[this._head]; } /** - * @returns {number} The current number of elements in the queue. - */ - size() { - return this._size; - } - - /** + * Determines whether the queue is empty. * @returns {boolean} True if the queue is empty, false otherwise. */ isEmpty() { - return this._size === 0; - } - - /** - * Internal method to resize the underlying storage. - * @param {number} size - The new capacity for the queue. - */ - resize(size) { - if (size <= this._size) { - return; - } - - const newStorage = new Array(size); - - for (let i = 0; i < this._size; i++) { - const index = (this._head + i) % this._capacity; - newStorage[i] = this._storage[index]; - } - - this._head = 0; - this._capacity = size; - this._storage = newStorage; + return this._length === 0; } } diff --git a/src/scene/graph-node.js b/src/scene/graph-node.js index 6e14780d31d..aaeea9802dd 100644 --- a/src/scene/graph-node.js +++ b/src/scene/graph-node.js @@ -77,15 +77,18 @@ function createTest(attr, value) { */ class GraphNode extends EventHandler { /** - * @type {GraphNode[]} + * It is a pool of graph node stacks that are used to reduce memory allocation overhead + * Note: we usually don't need more than 1 stack at a time, but we allocate 2 just in case. + * + * @type {ObjectPool>} * @private */ - static _stack = []; + static _stackPool = new ObjectPool(Array, 2); /** * It is a pool of graph node queues that are used to reduce memory allocation overhead * - * @type {ObjectPool} + * @type {ObjectPool>} * @private */ static _queuePool = new ObjectPool(Queue, 1); @@ -96,7 +99,7 @@ class GraphNode extends EventHandler { * @type {number} * @private */ - static _maxQueueSize = 512; + static _maxQueueCapacity = 512; /** * The non-unique name of a graph node. Defaults to 'Untitled'. @@ -448,30 +451,12 @@ class GraphNode extends EventHandler { * @protected */ _notifyHierarchyStateChanged(node, enabled) { - /** - * NOTE: GraphNode._stack is not used here because _notifyHierarchyStateChanged can be called - * while other _notifyHierarchyStateChanged is still running because of _onHierarchyStateChanged - * - * @type {GraphNode[]} - */ - const stack = [node]; - - while (stack.length) { - const current = stack.pop(); - current._onHierarchyStateChanged(enabled); - - const c = current._children; - - for (let i = 0; i < c.length; i++) { - const child = c[i]; - - if (enabled) { - if (child._enabled) { - stack.push(child); - } - } else { - stack.push(child); - } + node._onHierarchyStateChanged(enabled); + + const c = node._children; + for (let i = 0, len = c.length; i < len; i++) { + if (c[i]._enabled) { + this._notifyHierarchyStateChanged(c[i], enabled); } } } @@ -551,7 +536,6 @@ class GraphNode extends EventHandler { return this; } - /** * Destroy the graph node and all of its descendants. First, the graph node is removed from the * hierarchy. This is then repeated recursively for all descendants of the graph node. @@ -612,22 +596,11 @@ class GraphNode extends EventHandler { const results = []; const test = createTest(attr, value); - const stack = GraphNode._stack; - stack.push(this); - - while (stack.length) { - const node = stack.pop(); - + this.forEach(node => { if (test(node)) { results.push(node); } - - const children = node._children; - - for (let i = 0; i < children.length; i++) { - stack.push(children[i]); - } - } + }); return results; } @@ -659,21 +632,28 @@ class GraphNode extends EventHandler { findOne(attr, value) { const test = createTest(attr, value); - const stack = GraphNode._stack; - stack.push(this); + const stack = GraphNode._stackPool.allocate(); + let size = 0; + stack[size++] = this; + + while (size > 0) { + const node = stack[--size]; - while (stack.length) { - const current = stack.pop(); - if (test(current)) { - return current; + if (test(node)) { + GraphNode._stackPool.free(stack); + + return node; } - const children = current._children; - for (let i = 0; i < children.length; i++) { - stack.push(children[i]); + const children = node._children; + const length = children.length; + for (let i = length - 1; i >= 0; --i) { + stack[size++] = children[i]; } } + GraphNode._stackPool.free(stack); + return null; } @@ -700,26 +680,26 @@ class GraphNode extends EventHandler { */ findByTag(...query) { const results = []; - const stack = GraphNode._stack; + const stack = GraphNode._stackPool.allocate(); + Array.prototype.push.apply(stack, this._children); + let size = stack.length; - const children = this._children; - for (let i = 0; i < children.length; i++) { - stack.push(children[i]); - } - - while (stack.length) { - const node = stack.pop(); + while (size > 0) { + const node = stack[--size]; if (node.tags.has(...query)) { results.push(node); } const children = node._children; - for (let i = 0; i < children.length; i++) { - stack.push(children[i]); + const length = children.length; + for (let i = length - 1; i >= 0; --i) { + stack[size++] = children[i]; } } + GraphNode._stackPool.free(stack); + return results; } @@ -778,20 +758,23 @@ class GraphNode extends EventHandler { * }); */ forEach(callback, thisArg) { + // This is BFS via a queue, so not a stack usage const queue = GraphNode._queuePool.allocate(); - queue.resize(GraphNode._maxQueueSize); + queue.capacity = GraphNode._maxQueueCapacity; queue.enqueue(this); - while (queue.size()) { - const current = queue.dequeue(); - callback(current); - const children = current.children; + while (queue.length) { + const node = queue.dequeue(); + + callback.call(thisArg, node); + + const children = node.children; for (let i = 0; i < children.length; i++) { queue.enqueue(children[i]); } } - GraphNode._maxQueueSize = Math.max(GraphNode._maxQueueSize, queue.size()); + GraphNode._maxQueueCapacity = Math.max(GraphNode._maxQueueCapacity, queue.capacity); GraphNode._queuePool.free(queue); } @@ -811,7 +794,6 @@ class GraphNode extends EventHandler { if (parent === node) { return true; } - parent = parent._parent; } return false; @@ -1001,11 +983,9 @@ class GraphNode extends EventHandler { * @ignore */ get worldScaleSign() { - if (this._worldScaleSign === 0) { this._worldScaleSign = this.getWorldTransform().scaleSign; } - return this._worldScaleSign; } @@ -1175,11 +1155,12 @@ class GraphNode extends EventHandler { /** @private */ _dirtifyWorldInternal() { - const stack = GraphNode._stack; - stack.push(this); + const stack = GraphNode._stackPool.allocate(); + let size = 0; + stack[size++] = this; - while (stack.length > 0) { - const node = stack.pop(); + while (size > 0) { + const node = stack[--size]; if (!node._dirtyWorld) { node._frozen = false; @@ -1187,16 +1168,18 @@ class GraphNode extends EventHandler { } node._dirtyNormal = true; - node._worldScaleSign = 0; + node._worldScaleSign = 0; // world matrix is dirty, mark this flag dirty too node._aabbVer++; const children = node._children; for (let i = 0; i < children.length; i++) { if (!children[i]._dirtyWorld) { - stack.push(children[i]); + stack[size++] = children[i]; } } } + + GraphNode._stackPool.free(stack); } /** @@ -1358,7 +1341,6 @@ class GraphNode extends EventHandler { * @ignore */ addChildAndSaveTransform(node) { - const wPos = node.getPosition(); const wRot = node.getRotation(); @@ -1383,7 +1365,6 @@ class GraphNode extends EventHandler { * this.entity.insertChild(e, 1); */ insertChild(node, index) { - this._prepareInsertChild(node); this._children.splice(index, 0, node); this._onInsertChild(node); @@ -1396,7 +1377,6 @@ class GraphNode extends EventHandler { * @private */ _prepareInsertChild(node) { - // remove it from the existing parent node.remove(); @@ -1416,28 +1396,22 @@ class GraphNode extends EventHandler { _fireOnHierarchy(name, nameHierarchy, parent) { this.fire(name, parent); - /** - * NOTE: GraphNode._stack is not used here because _fireOnHierarchy can be called - * while other _fireOnHierarchy is still running - * - * @type {GraphNode[]} - */ - const stack = [this]; + const stack = GraphNode._stackPool.allocate(); + let size = 1; - const child = this._children; + while (size > 0) { + const node = stack[--size]; - for (let i = 0; i < child.length; i++) { - stack.push(child[i]); - } + node.fire(nameHierarchy, parent); - while (stack.length) { - const curr = stack.pop(); - curr.fire(nameHierarchy, parent); - - for (let j = 0; j < curr._children.length; j++) { - stack.push(curr._children[j]); + const children = node._children; + const length = children.length; + for (let i = length - 1; i >= 0; --i) { + stack[size++] = children[i]; } } + + GraphNode._stackPool.free(stack); } /** @@ -1485,17 +1459,22 @@ class GraphNode extends EventHandler { * @private */ _updateGraphDepth() { - const stack = GraphNode._stack; - stack.push(this); + const stack = GraphNode._stackPool.allocate(); + let size = 0; + stack[size++] = this; - while (stack.length) { - const n = stack.pop(); - n._graphDepth = n._parent ? n._parent._graphDepth + 1 : 0; + while (size > 0) { + const node = stack[--size]; + node._graphDepth = node._parent ? node._parent._graphDepth + 1 : 0; - for (let i = 0; i < n._children.length; i++) { - stack.push(n._children[i]); + const children = node._children; + const length = children.length; + for (let i = length - 1; i >= 0; --i) { + stack[size++] = children[i]; } } + + GraphNode._stackPool.free(stack); } /** @@ -1601,11 +1580,12 @@ class GraphNode extends EventHandler { return; } - const stack = GraphNode._stack; - stack.push(this); + const stack = GraphNode._stackPool.allocate(); + let size = 0; + stack[size++] = this; - while (stack.length) { - const node = stack.pop(); + while (size > 0) { + const node = stack[--size]; if (!node._enabled || node._frozen) { continue; @@ -1618,11 +1598,13 @@ class GraphNode extends EventHandler { } const children = node._children; - - for (let i = children.length - 1; i >= 0; i--) { - stack.push(children[i]); + const length = children.length; + for (let i = length - 1; i >= 0; --i) { + stack[size++] = children[i]; } } + + GraphNode._stackPool.free(stack); } /** diff --git a/test/core/queue.test.mjs b/test/core/queue.test.mjs index 3b6e48b8919..dc30ccd9135 100644 --- a/test/core/queue.test.mjs +++ b/test/core/queue.test.mjs @@ -9,72 +9,207 @@ describe('Queue', function () { queue = new Queue(2); }); - it('should initialize as empty', function () { - expect(queue.isEmpty()).to.be.true; - expect(queue.size()).to.equal(0); - }); + describe('#constructor', function () { + it('should initialize as empty', function () { + expect(queue.isEmpty()).to.be.true; + expect(queue.length).to.equal(0); + }); - it('should enqueue elements into the queue', function () { - queue.enqueue(1); - expect(queue.isEmpty()).to.be.false; - expect(queue.size()).to.equal(1); - expect(queue.peek()).to.equal(1); - }); + it('should return the initial capacity from the constructor', function () { + expect(queue.capacity).to.equal(2); - it('should dequeue elements from the queue', function () { - queue.enqueue(1); - queue.enqueue(2); - const dequeued = queue.dequeue(); - expect(dequeued).to.equal(1); - expect(queue.size()).to.equal(1); - expect(queue.peek()).to.equal(2); + const customQueue = new Queue(5); + expect(customQueue.capacity).to.equal(5); + }); }); - it('should return undefined when dequeuing from an empty queue', function () { - const dequeued = queue.dequeue(); - expect(dequeued).to.be.undefined; - expect(queue.isEmpty()).to.be.true; - }); + describe('#enqueue()/#dequeue()', function () { + it('should enqueue elements into the queue', function () { + queue.enqueue(1); + expect(queue.isEmpty()).to.be.false; + expect(queue.length).to.equal(1); + expect(queue.peek()).to.equal(1); + }); + + it('should dequeue elements from the queue', function () { + queue.enqueue(1); + queue.enqueue(2); + const dequeued = queue.dequeue(); + expect(dequeued).to.equal(1); + expect(queue.length).to.equal(1); + expect(queue.peek()).to.equal(2); + }); + + it('should return undefined when dequeuing from an empty queue', function () { + const dequeued = queue.dequeue(); + expect(dequeued).to.be.undefined; + expect(queue.isEmpty()).to.be.true; + }); + + it('should resize when capacity is exceeded', function () { + queue.enqueue(1); + queue.enqueue(2); + queue.enqueue(3); // This should trigger a resize + expect(queue.length).to.equal(3); + expect(queue.peek()).to.equal(1); + }); + + it('should maintain order after resizing', function () { + queue.enqueue(1); + queue.enqueue(2); + queue.enqueue(3); + expect(queue.dequeue()).to.equal(1); + expect(queue.dequeue()).to.equal(2); + expect(queue.dequeue()).to.equal(3); + }); + + it('should correctly report if the queue is empty', function () { + expect(queue.isEmpty()).to.be.true; + queue.enqueue(1); + expect(queue.isEmpty()).to.be.false; + queue.dequeue(); + expect(queue.isEmpty()).to.be.true; + }); - it('should peek at the front element without dequeuing it', function () { - queue.enqueue(1); - const front = queue.peek(); - expect(front).to.equal(1); - expect(queue.size()).to.equal(1); + it('should correctly report the size of the queue', function () { + expect(queue.length).to.equal(0); + queue.enqueue(1); + expect(queue.length).to.equal(1); + queue.enqueue(2); + expect(queue.length).to.equal(2); + queue.dequeue(); + expect(queue.length).to.equal(1); + }); + + it('should handle multiple resizes', function () { + for (let i = 1; i <= 5; i++) { + queue.enqueue(i); + } + + expect(queue.capacity).to.be.at.least(4); + expect(queue.length).to.equal(5); + + expect(queue.dequeue()).to.equal(1); + expect(queue.dequeue()).to.equal(2); + expect(queue.dequeue()).to.equal(3); + expect(queue.dequeue()).to.equal(4); + expect(queue.dequeue()).to.equal(5); + }); + + it('should allow enqueuing after manually setting a larger capacity', function () { + expect(queue.capacity).to.equal(2); + queue.capacity = 10; + expect(queue.capacity).to.equal(10); + + for (let i = 0; i < 10; i++) { + queue.enqueue(i); + } + expect(queue.length).to.equal(10); + expect(queue.capacity).to.equal(10); + }); }); - it('should resize when capacity is exceeded', function () { - queue.enqueue(1); - queue.enqueue(2); - queue.enqueue(3); // This should trigger a resize - expect(queue.size()).to.equal(3); - expect(queue.peek()).to.equal(1); + describe('#capacity', function () { + it('should not reduce capacity if new capacity is smaller or equal to current length', function () { + queue.enqueue(1); + queue.enqueue(2); + expect(queue.length).to.equal(2); + + queue.capacity = 1; + expect(queue.capacity).to.equal(2); + + expect(queue.dequeue()).to.equal(1); + expect(queue.dequeue()).to.equal(2); + expect(queue.isEmpty()).to.be.true; + }); + + it('should set a larger capacity if new capacity is bigger than current size', function () { + expect(queue.capacity).to.equal(2); + queue.enqueue(1); + + queue.capacity = 5; + expect(queue.capacity).to.equal(5); + expect(queue.length).to.equal(1); + expect(queue.dequeue()).to.equal(1); + }); + + it('should handle wrap-around when manually setting capacity', function () { + queue.enqueue(1); + queue.enqueue(2); + + const first = queue.dequeue(); + expect(first).to.equal(1); + + queue.enqueue(3); + + queue.capacity = 4; + expect(queue.capacity).to.equal(4); + + expect(queue.dequeue()).to.equal(2); + expect(queue.dequeue()).to.equal(3); + expect(queue.isEmpty()).to.be.true; + }); }); - it('should maintain order after resizing', function () { - queue.enqueue(1); - queue.enqueue(2); - queue.enqueue(3); - expect(queue.dequeue()).to.equal(1); - expect(queue.dequeue()).to.equal(2); - expect(queue.dequeue()).to.equal(3); + describe('#length', function () { + it('should accurately reflect the number of items in the queue', function () { + expect(queue.length).to.equal(0); + + queue.enqueue(10); + queue.enqueue(20); + expect(queue.length).to.equal(2); + + queue.dequeue(); + expect(queue.length).to.equal(1); + + queue.enqueue(30); + expect(queue.length).to.equal(2); + + queue.dequeue(); + queue.dequeue(); + expect(queue.length).to.equal(0); + expect(queue.isEmpty()).to.be.true; + }); }); - it('should correctly report if the queue is empty', function () { - expect(queue.isEmpty()).to.be.true; - queue.enqueue(1); - expect(queue.isEmpty()).to.be.false; - queue.dequeue(); - expect(queue.isEmpty()).to.be.true; + describe('#isEmpty()', function () { + it('should return true when queue is newly created', function () { + expect(queue.isEmpty()).to.be.true; + }); + + it('should return false when at least one item is enqueued', function () { + queue.enqueue('test'); + expect(queue.isEmpty()).to.be.false; + }); + + it('should return true after enqueuing and then dequeuing the same number of items', function () { + queue.enqueue('a'); + queue.enqueue('b'); + queue.dequeue(); + queue.dequeue(); + expect(queue.isEmpty()).to.be.true; + }); }); - it('should correctly report the size of the queue', function () { - expect(queue.size()).to.equal(0); - queue.enqueue(1); - expect(queue.size()).to.equal(1); - queue.enqueue(2); - expect(queue.size()).to.equal(2); - queue.dequeue(); - expect(queue.size()).to.equal(1); + describe('#peek()', function () { + it('should return undefined if the queue is empty', function () { + expect(queue.peek()).to.be.undefined; + }); + + it('should peek at the front element without dequeuing it', function () { + queue.enqueue(1); + const front = queue.peek(); + expect(front).to.equal(1); + expect(queue.length).to.equal(1); + }); + + it('should return the front element after multiple enqueues/dequeues', function () { + queue.enqueue(1); + queue.enqueue(2); + queue.enqueue(3); + queue.dequeue(); + + expect(queue.peek()).to.equal(2); + }); }); }); From c38b981f6fadeed05f4aa6e4843f429ed5e3fc45 Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Mon, 6 Jan 2025 04:55:32 +0100 Subject: [PATCH 11/12] Minor fix --- src/core/queue.js | 20 ++++++++++---------- src/scene/graph-node.js | 9 ++++++--- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/core/queue.js b/src/core/queue.js index cd00fa94766..be9aa4436f5 100644 --- a/src/core/queue.js +++ b/src/core/queue.js @@ -8,7 +8,7 @@ class Queue { /** * Create a new queue. - * @param {number} [initialCapacity=8] - The initial capacity of the queue. + * @param {number} [initialCapacity] - The initial capacity of the queue. */ constructor(initialCapacity = 8) { /** @@ -42,15 +42,6 @@ class Queue { return this._length; } - /** - * The capacity of the queue. - * @type {number} - * @readonly - */ - get capacity() { - return this._storage.length; - } - /** * Change the capacity of the underlying storage. * Does not shrink capacity if new capacity is less than or equal to the current length. @@ -74,6 +65,15 @@ class Queue { } } + /** + * The capacity of the queue. + * @type {number} + * @readonly + */ + get capacity() { + return this._storage.length; + } + /** * Enqueue (push) a value to the back of the queue. * Automatically extends capacity if the queue is full. diff --git a/src/scene/graph-node.js b/src/scene/graph-node.js index aaeea9802dd..8b621d18b22 100644 --- a/src/scene/graph-node.js +++ b/src/scene/graph-node.js @@ -596,7 +596,7 @@ class GraphNode extends EventHandler { const results = []; const test = createTest(attr, value); - this.forEach(node => { + this.forEach((node) => { if (test(node)) { results.push(node); } @@ -681,8 +681,11 @@ class GraphNode extends EventHandler { findByTag(...query) { const results = []; const stack = GraphNode._stackPool.allocate(); - Array.prototype.push.apply(stack, this._children); - let size = stack.length; + + let size = 0; + for (let i = 0; i < this._children.length; ++i) { + stack[size++] = this._children[i]; + } while (size > 0) { const node = stack[--size]; From 7f66759b58e8236217c7d30e48a72f295e8d9aa6 Mon Sep 17 00:00:00 2001 From: Kirill Osipov Date: Mon, 6 Jan 2025 05:32:20 +0100 Subject: [PATCH 12/12] Minor fix --- src/scene/graph-node.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/scene/graph-node.js b/src/scene/graph-node.js index 8b621d18b22..4247d24acd0 100644 --- a/src/scene/graph-node.js +++ b/src/scene/graph-node.js @@ -647,7 +647,7 @@ class GraphNode extends EventHandler { const children = node._children; const length = children.length; - for (let i = length - 1; i >= 0; --i) { + for (let i = 0; i < length; ++i) { stack[size++] = children[i]; } } @@ -696,7 +696,7 @@ class GraphNode extends EventHandler { const children = node._children; const length = children.length; - for (let i = length - 1; i >= 0; --i) { + for (let i = 0; i < length; ++i) { stack[size++] = children[i]; } } @@ -1409,7 +1409,7 @@ class GraphNode extends EventHandler { const children = node._children; const length = children.length; - for (let i = length - 1; i >= 0; --i) { + for (let i = 0; i < length; ++i) { stack[size++] = children[i]; } } @@ -1472,7 +1472,7 @@ class GraphNode extends EventHandler { const children = node._children; const length = children.length; - for (let i = length - 1; i >= 0; --i) { + for (let i = 0; i < length; ++i) { stack[size++] = children[i]; } } @@ -1602,7 +1602,7 @@ class GraphNode extends EventHandler { const children = node._children; const length = children.length; - for (let i = length - 1; i >= 0; --i) { + for (let i = 0; i < length; ++i) { stack[size++] = children[i]; } }