-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Matrix performance #12973
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Matrix performance #12973
Changes from 1 commit
460a8d6
7dd0a5a
6a759d3
7c80345
3075077
35ac681
d17ab1e
28286e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ import DeveloperError from "./DeveloperError.js"; | |
| import CesiumMath from "./Math.js"; | ||
| import Matrix3 from "./Matrix3.js"; | ||
| import RuntimeError from "./RuntimeError.js"; | ||
| import freezeMatrix from "./freezeMatrix.js"; | ||
|
|
||
| /** | ||
| * A 4x4 matrix, indexable as a column-major order array. | ||
|
|
@@ -2947,56 +2948,6 @@ Matrix4.inverseTranspose = function (matrix, result) { | |
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * An immutable Matrix4 instance initialized to the identity matrix. | ||
| * | ||
| * @type {Matrix4} | ||
| * @constant | ||
| */ | ||
| Matrix4.IDENTITY = new Matrix4( | ||
| 1.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 1.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 1.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 1.0, | ||
| ); | ||
|
|
||
| /** | ||
| * An immutable Matrix4 instance initialized to the zero matrix. | ||
| * | ||
| * @type {Matrix4} | ||
| * @constant | ||
| */ | ||
| Matrix4.ZERO = new Matrix4( | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| ); | ||
|
|
||
| /** | ||
| * The index into Matrix4 for column 0, row 0. | ||
| * | ||
|
|
@@ -3211,4 +3162,111 @@ Matrix4.prototype.toString = function () { | |
| `(${this[3]}, ${this[7]}, ${this[11]}, ${this[15]})` | ||
| ); | ||
| }; | ||
|
|
||
| // Frozen Matrix option 1: Only implement the interface as generated by Cesium.d.ts (meaning, without exposing any Float64Array methods) | ||
| /** | ||
| * @returns {Readonly<Matrix4>} a frozen matrix | ||
| */ | ||
| // eslint-disable-next-line no-unused-vars | ||
| function makeFrozenMatrix4( | ||
| column0Row0, | ||
| column1Row0, | ||
| column2Row0, | ||
| column3Row0, | ||
| column0Row1, | ||
| column1Row1, | ||
| column2Row1, | ||
| column3Row1, | ||
| column0Row2, | ||
| column1Row2, | ||
| column2Row2, | ||
| column3Row2, | ||
| column0Row3, | ||
| column1Row3, | ||
| column2Row3, | ||
| column3Row3, | ||
| ) { | ||
| /** @type {Matrix4} */ | ||
| const matrix = { | ||
| [0]: column0Row0 ?? 0.0, | ||
| [1]: column0Row1 ?? 0.0, | ||
| [2]: column0Row2 ?? 0.0, | ||
| [3]: column0Row3 ?? 0.0, | ||
| [4]: column1Row0 ?? 0.0, | ||
| [5]: column1Row1 ?? 0.0, | ||
| [6]: column1Row2 ?? 0.0, | ||
| [7]: column1Row3 ?? 0.0, | ||
| [8]: column2Row0 ?? 0.0, | ||
| [9]: column2Row1 ?? 0.0, | ||
| [10]: column2Row2 ?? 0.0, | ||
| [11]: column2Row3 ?? 0.0, | ||
| [12]: column3Row0 ?? 0.0, | ||
| [13]: column3Row1 ?? 0.0, | ||
| [14]: column3Row2 ?? 0.0, | ||
| [15]: column3Row3 ?? 0.0, | ||
| length: Matrix4.packedLength, | ||
| }; | ||
|
|
||
| matrix.clone = Matrix4.prototype.clone.bind(matrix); | ||
| matrix.equals = Matrix4.prototype.equals.bind(matrix); | ||
| matrix.equalsEpsilon = Matrix4.prototype.equalsEpsilon.bind(matrix); | ||
| matrix.toString = Matrix4.prototype.toString.bind(matrix); | ||
|
|
||
| return Object.freeze(matrix); | ||
| } | ||
|
|
||
| /** | ||
| * An immutable Matrix4 instance initialized to the identity matrix. | ||
| * | ||
| * @type {Readonly<Matrix4>} | ||
| * @constant | ||
| */ | ||
| Matrix4.IDENTITY = freezeMatrix( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently using the The main difference is future-proofing/avoiding accidents where someone does choose to work with them as real typed arrays (maybe for performance reasons or whatever), in which case Unlike with a Proxy, there is no misdirection so V8 can still attempt to optimize these, albeit only to the performance level of the old matrices. |
||
| new Matrix4( | ||
| 1.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 1.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 1.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 1.0, | ||
| ), | ||
| ); | ||
|
|
||
| /** | ||
| * An immutable Matrix4 instance initialized to the zero matrix. | ||
| * | ||
| * @type {Readonly<Matrix4>} | ||
| * @constant | ||
| */ | ||
| Matrix4.ZERO = freezeMatrix( | ||
| new Matrix4( | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| 0.0, | ||
| ), | ||
| ); | ||
|
|
||
| export default Matrix4; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| import DeveloperError from "./DeveloperError"; | ||
|
|
||
| // Deep copies all property descriptors of the provided object. | ||
| function getPropertyDescriptorMap(object) { | ||
| /** @type {PropertyDescriptorMap} */ | ||
| const propertyDescriptorMap = {}; | ||
| let current = object; | ||
|
|
||
| while (current) { | ||
| Object.entries(Object.getOwnPropertyDescriptors(current)).forEach( | ||
| ([name, descriptor]) => { | ||
| if (propertyDescriptorMap[name]) { | ||
| return; | ||
| } | ||
| propertyDescriptorMap[name] = descriptor; | ||
| }, | ||
| ); | ||
| Object.getOwnPropertySymbols(current).forEach( | ||
| // eslint-disable-next-line no-loop-func | ||
| (symbol) => { | ||
| if (propertyDescriptorMap[symbol]) { | ||
| return; | ||
| } | ||
| propertyDescriptorMap[symbol] = { | ||
| value: object[symbol], | ||
| enumerable: false, | ||
| }; | ||
| }, | ||
| ); | ||
|
|
||
| // Move up the prototype chain | ||
| current = Object.getPrototypeOf(current); | ||
| if (current.constructor.name === "Object") { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| return propertyDescriptorMap; | ||
| } | ||
|
|
||
| // Frozen Matrix option 2: Generate a true Matrix4, including all underlying Float64Array methods. | ||
| // This has 100% parity with a regular Matrix4, with the exception that calling any methods that change the underlying matrix, | ||
| // or accessing the buffer will throw a DeveloperError | ||
| /** | ||
| * @template T | ||
| * @param {T} matrix | ||
| * @returns {Readonly<T>} a frozen matrix | ||
| */ | ||
| export default function freezeMatrix(matrix) { | ||
| const properties = getPropertyDescriptorMap(matrix); | ||
| // Get both regular and symbol keys | ||
| for (const property of Reflect.ownKeys(properties)) { | ||
| const descriptor = properties[property]; | ||
| // These methods manipulate directly or allow the manipulation of the underlying buffer. Make them illegal. | ||
| if ( | ||
| ["copyWithin", "fill", "reverse", "set", "sort", "subarray"].includes( | ||
| property, | ||
| ) | ||
| ) { | ||
| descriptor.value = function () { | ||
| throw new DeveloperError(`Cannot call ${property} on a frozen Matrix`); | ||
| }; | ||
| } else if (typeof descriptor.value === "function") { | ||
| // Proxy all functions back onto the original matrix | ||
| descriptor.value = descriptor.value.bind(matrix); | ||
| } | ||
| // Deny access the underlying buffer on the facade | ||
| if (property === "buffer") { | ||
| descriptor.get = function () { | ||
| throw new DeveloperError(`Cannot access buffer of a frozen Matrix`); | ||
| }; | ||
| delete descriptor.value; | ||
| } | ||
| } | ||
| properties.toString = { | ||
| value: matrix.toString, | ||
| }; | ||
| properties.constructor = { | ||
| value: matrix.constructor, | ||
| }; | ||
|
|
||
| // Create a fake matrix object and map all properties of the real matrix onto it | ||
| const fakeMatrix = Object.create(Object, properties); | ||
|
|
||
| // Freeze the fake matrix object. We know have a fully compatible Matrix clone which is still backed | ||
| // by an underlying Float64Array, but is completed frozen to external changes. | ||
| return Object.freeze(fakeMatrix); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is basically just creating the old matrix type and manually binding the few added functions.