Skip to content

Commit

Permalink
Fix view preallocation during cloning
Browse files Browse the repository at this point in the history
Summary:
changelog: [internal]

This code assured that view preallocation is only triggered if ShadowNode is cloned from revision 0 to revision 1 (first time ShadowNode is cloned).
Node can go from virtual to view forming in subsequent clones, not just the first one. This is more of a case in Concurrent React where the node can be cloned many times before it is first mounted.

Reviewed By: mdvacca

Differential Revision: D31237719

fbshipit-source-id: 13fe6d10fdc815dbdae785d1d9d86d1c8fd36be4
  • Loading branch information
sammy-SC authored and facebook-github-bot committed Sep 29, 2021
1 parent 155a1a8 commit dd99475
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,9 @@ void Binding::installFabricUIManager(
enableFabricLogs_ =
config->getBool("react_fabric:enabled_android_fabric_logs");

disableRevisionCheckForPreallocation_ =
config->getBool("react_fabric:disable_revision_check_for_preallocation");

if (enableFabricLogs_) {
LOG(WARNING) << "Binding::installFabricUIManager() was called (address: "
<< this << ").";
Expand Down Expand Up @@ -1242,11 +1245,13 @@ void Binding::schedulerDidCloneShadowNode(
// 1. The revision is exactly 1
// 2. At revision 0 (the old node), View Preallocation would have been skipped

if (newShadowNode.getProps()->revision != 1) {
return;
}
if (oldShadowNode.getProps()->revision != 0) {
return;
if (!disableRevisionCheckForPreallocation_) {
if (newShadowNode.getProps()->revision != 1) {
return;
}
if (oldShadowNode.getProps()->revision != 0) {
return;
}
}

// If the new node is concrete and the old wasn't, we can preallocate
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ class Binding : public jni::HybridClass<Binding>,
bool disablePreallocateViews_{false};
bool enableFabricLogs_{false};
bool enableEarlyEventEmitterUpdate_{false};
bool disableRevisionCheckForPreallocation_{false};
};

} // namespace react
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -910,10 +910,6 @@ public void preallocateView(
// We treat this as a perf problem and not a logical error. View Preallocation or unexpected
// changes to Differ or C++ Binding could cause some redundant Create instructions.
if (getNullableViewState(reactTag) != null) {
ReactSoftExceptionLogger.logSoftException(
TAG,
new IllegalStateException(
"Cannot Preallocate view with tag [" + reactTag + "], already exists."));
return;
}

Expand Down

0 comments on commit dd99475

Please sign in to comment.