Skip to content

Reduce MatrixMathHelper allocations (#58275) - #58275

Open
javache wants to merge 1 commit into
react:mainfrom
javache:export-D118277120
Open

Reduce MatrixMathHelper allocations (#58275)#58275
javache wants to merge 1 commit into
react:mainfrom
javache:export-D118277120

Conversation

@javache

@javache javache commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Summary:

Matrix decomposition runs while applying view transforms and currently allocates a nested normalized matrix plus new arrays for each vector operation. Flatten the normalized matrix and update the temporary rows in place to remove ten allocations per decomposition without changing the public helpers.

Changelog: [Internal]

Reviewed By: cortinico

Differential Revision: D118277120

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 1, 2026
@facebook-github-tools facebook-github-tools Bot added p: Facebook Partner: Facebook Partner labels Sep 1, 2026
@meta-codesync

meta-codesync Bot commented Sep 1, 2026

Copy link
Copy Markdown

@javache has exported this pull request. If you are a Meta employee, you can view the originating Diff in D118277120.

@cortinico cortinico left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review automatically exported from Phabricator review in Meta.

@Abbondanzo

Copy link
Copy Markdown
Contributor

/review

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

🤖 AI code review

Decision: Ready for human review

Overall PR risk: Medium. The change modifies existing behavior rather than adding an isolated path, affecting Android matrix decomposition for view transforms and JNI bridge deserialization via ReadableNativeMap/ReadableNativeArray. The handoff notes the most plausible breakage is incorrect decomposition of perspective or zero-scale matrices and key/value alignment regressions, with blast radius bounded to Android UI transforms and bridge reads and rollback via direct revert. No critical or warning findings were reported.

No findings.


This review is advisory — it never blocks a merge and never auto-approves.

Summary:

Matrix decomposition runs while applying view transforms and currently allocates a nested normalized matrix plus new arrays for each vector operation. Flatten the normalized matrix and update the temporary rows in place to remove ten allocations per decomposition without changing the public helpers.

Changelog: [Internal]

Reviewed By: cortinico

Differential Revision: D118277120
@meta-codesync meta-codesync Bot changed the title Reduce MatrixMathHelper allocations Reduce MatrixMathHelper allocations (#58275) Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. meta-exported p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants