Use std::mutex instead of never-shared std::shared_mutex in ShadowNodeFamily

[amillez]

Summary:

ShadowNodeFamily::mutex_ is declared std::shared_mutex, but it is never acquired in shared mode. All three call sites take it exclusively via unique_lock, and there is no shared_lock anywhere in the class.

Worth saying up front, since the diff reads like a downgrade: this doesn't remove any read concurrency, because there hasn't been any since February 2020.

History

git log -L148,148:packages/react-native/ReactCommon/react/renderer/core/ShadowNodeFamily.h

The mutex came from StateCoordinator, which used both modes: shared_lock for the reader, unique_lock for the writer. d418750359b ("Merge StateCoordinator into ShadowNodeFamily", Feb 3 2020) carried it over unchanged.

91540d74b15 ("Fabric: Fixing a retain cycle in between State and ShadowNodeFamily", Feb 9 2020) rewrote the reader as part of a retain cycle fix, and the lock changed along with it:

-const StateTarget &ShadowNodeFamily::getTarget() const {
-  std::shared_lock<better::shared_mutex> lock(mutex_);
+State::Shared ShadowNodeFamily::getMostRecentState() const {
+  std::unique_lock<better::shared_mutex> lock(mutex_);

afc3c97111c (Apr 2020) then added getMostRecentStateIfObsolete with unique_lock, matching the neighbouring reader.

How it compares to the rest of ReactCommon

Of the 17 classes that declare a std::shared_mutex, this is the only one that never takes a shared_lock; the other 16 use both modes. That seemed worth flagging, and blame suggests this one used to be in the other group. After this change, all 16 remaining users take both modes.

Why std::mutex rather than restoring the shared_lock

Happy to do it the other way if you'd prefer. My reasoning:

The critical section is return mostRecentState_;, a single shared_ptr copy. Lock acquisition probably dominates the body, and acquiring a shared_lock costs more than mutex::lock. The lock is also per element rather than global, so contention on any one family should already be low. If that holds, std::mutex is both smaller and slightly cheaper here, whereas restoring the shared_lock would keep the memory cost for little practical gain.

The class already holds a std::mutex (nativePropsMutex), so this is consistent with its existing style, and <mutex> is already included.

Result

	shared_mutex	std::mutex
sizeof(ShadowNodeFamily) (arm64)	384 B	272 B
mutex share of the object	36%	15%

112 bytes per family, 29% smaller. There's one family per host element, so it scales with tree size. The primitive saving is 100 bytes on Android/bionic and 104 on Apple/libc++, so it isn't platform specific.

This is not a fix for #55998, but it does reduce the per instance cost of the objects that issue reports accumulating.

ABI

ShadowNodeFamily.h ships as a prefab header, so third party native modules compile against it. Changing sizeof is a C++ ABI break, which means this should target main for a minor rather than a patch.

If I've missed something

If the exclusive reads are intentional for a reason I haven't found, I'd appreciate knowing, and I'll close this.

Changelog:

[GENERAL] [CHANGED] - Replace never-shared std::shared_mutex with std::mutex in ShadowNodeFamily

Test Plan:

The compiler checks the main claim: std::mutex has no lock_shared(), so std::shared_lock<std::mutex> doesn't compile. If anything took a shared lock on this mutex, this change wouldn't build.

• Compiled all 132 TUs in libreactnative that transitively include ShadowNodeFamily.h, before and after the change, using the project's own compile_commands.json (NDK 27, aarch64-none-linux-android24). Identical results.
• Semantics: every acquisition is exclusive. unique_lock<shared_mutex> and unique_lock<mutex> are both exclusive, non recursive, and impose the same happens-before, so with no shared acquisitions only sizeof and timing differ. Neither type guarantees fairness.
• mutex_ is private, and the two friends (ShadowNode, State) don't reference it.
• The .cpp needs no change: all three sites use CTAD (std::unique_lock lock(mutex_)), which deduces unique_lock<std::mutex> automatically.
• Size measured with a sizeof probe compiled against the real header using the project's own build flags.

Reproduce the measurement

// probe.cpp, compiled with the flags from compile_commands.json for ShadowNodeFamily.cpp
#include <react/renderer/core/ShadowNodeFamily.h>
template<int N> struct SIZE_IS;
SIZE_IS<(int)sizeof(facebook::react::ShadowNodeFamily)> x;  // the error prints the size

I'd be happy to run any other tests, just let me know what would help.

[meta-cla]

Hi @amillez!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[meta-cla]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!