Skip to content

Compare prop name pointers before their characters in RawPropsParser - #59657

Merged
javache merged 1 commit into
react:mainfrom
hryhoriiK97:rawprops-key-pointer-compare
Oct 30, 2026
Merged

javache merged 1 commit into
react:mainfrom
hryhoriiK97:rawprops-key-pointer-compare

Conversation

@hryhoriiK97

Copy link
Copy Markdown
Contributor

Summary:

RawPropsParser::at finds a prop's value by walking the parser's keys in the order the Props constructor reads them. On that path every lookup compares the requested name with the expected key using std::string_view equality, which runs memcmp over the name.

Props constructors pass the same string literal for a prop on every parse (convertRawProp(context, rawProps, "opacity", ...)), and that literal is what the parser stored in keys_ when it was prepared. So the two views almost always point to the same characters. This change compares the pointers first and only compares the characters when they differ, so the result is the same as == in every case: same pointer → equal exactly when the lengths match; different pointers → the full comparison as before.

All keys in core come from string literals with static storage (convertRawProp and the direct rawProps.at("...") calls); the prefix/suffix overload of RawProps::at, which builds a temporary string, has no callers in ReactCommon.

Changelog:

[GENERAL] [CHANGED] - Props parsing compares prop name pointers before their characters

Test Plan:

  • yarn fantom (full suite, tester rebuilt with this change): 3314 tests pass. The other failures are unrelated to props: snapshot printing in my checkout's Jest setup, and a flaky tester crash in TesterAppDelegate::loadScriptAndRunTests that also happens without this change (the one suite it hit passes on rerun).
  • clang-format --dry-run --Werror passes.
  • On a device. RNTester release builds from the same checkout without and with this change, iPhone 15 Pro, a JS-driven animation that recolors 30 texts (a props update every frame), Time Profiler for 20 s, three runs each, interleaved. JS thread:
before this PR change
memcmp in RawPropsParser::at 40 / 45 / 37 ms 1 / 1 / 0 ms gone
RawPropsParser::at 105 / 106 / 110 ms 71 / 72 / 63 ms −36%
props parsing (cloneProps and below) 245 / 263 / 233 ms 187 / 195 / 185 ms −24%
JS thread total 3981 / 3921 / 3875 ms 3805 / 3796 / 3809 ms −3.1%

🤖 Generated with Claude Code

On the in-order path of `RawPropsParser::at`, every lookup compared the
requested prop name with the expected key using `std::string_view`
equality, which runs `memcmp`. Props constructors pass the same string
literal for a prop on every parse, and that literal is what the parser
stored when it was prepared, so the two views usually point to the same
characters. Check the pointers first and fall back to comparing the
characters when they differ.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@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 Oct 30, 2026
@facebook-github-tools facebook-github-tools Bot added p: Software Mansion Partner: Software Mansion Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 30, 2026
@meta-codesync

meta-codesync Bot commented Oct 30, 2026

Copy link
Copy Markdown

@javache has imported this pull request. If you are a Meta employee, you can view this in D128573190.

@javache
javache merged commit 3b88cb2 into react:main Oct 30, 2026
107 checks passed
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. p: Software Mansion Partner: Software Mansion Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants