Skip to content

Commit

Permalink
remove gating around improved onLayout batching (#42631)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #42631

changelog: [General][Breaking] onLayout event batching changes

Reviewed By: mdvacca, veviego

Differential Revision: D52955970

fbshipit-source-id: 9e8a340c47bd0e39a55cc1d16a6361959342c55f
  • Loading branch information
sammy-SC authored and facebook-github-bot committed Feb 5, 2024
1 parent fe69f71 commit 57e49b5
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 75 deletions.
4 changes: 0 additions & 4 deletions packages/react-native/React/Fabric/RCTSurfacePresenter.mm
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,6 @@ - (RCTScheduler *)_createScheduler
CoreFeatures::enableMountHooks = true;
}

if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:enable_default_async_batched_priority")) {
CoreFeatures::enableDefaultAsyncBatchedPriority = true;
}

if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:enable_cloneless_state_progression")) {
CoreFeatures::enableClonelessStateProgression = true;
}
Expand Down
1 change: 0 additions & 1 deletion packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -1896,7 +1896,6 @@ public class com/facebook/react/config/ReactFeatureFlags {
public static field enableBridgelessArchitectureSoftExceptions Z
public static field enableClonelessStateProgression Z
public static field enableCppPropsIteratorSetter Z
public static field enableDefaultAsyncBatchedPriority Z
public static field enableEagerRootViewAttachment Z
public static field enableFabricLogs Z
public static field enableFabricPendingEventQueue Z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,6 @@ public class ReactFeatureFlags {
/** Use native view configs in bridgeless mode. */
public static boolean useNativeViewConfigsInBridgelessMode = false;

/** Default state updates and events to async batched priority. */
public static boolean enableDefaultAsyncBatchedPriority = false;

/** Utilize shared Event C++ pipeline with fabric's renderer */
public static boolean enableFabricSharedEventPipeline = true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -414,8 +414,6 @@ void Binding::installFabricUIManager(

CoreFeatures::enablePropIteratorSetter =
getFeatureFlagValue("enableCppPropsIteratorSetter");
CoreFeatures::enableDefaultAsyncBatchedPriority =
getFeatureFlagValue("enableDefaultAsyncBatchedPriority");
CoreFeatures::enableClonelessStateProgression =
getFeatureFlagValue("enableClonelessStateProgression");
CoreFeatures::excludeYogaFromRawProps =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

#include "BaseViewEventEmitter.h"
#include <react/utils/CoreFeatures.h>

namespace facebook::react {

Expand Down Expand Up @@ -80,42 +79,37 @@ void BaseViewEventEmitter::onLayout(const LayoutMetrics& layoutMetrics) const {
layoutEventState->isDispatching = true;
}

dispatchEvent(
"layout",
[layoutEventState](jsi::Runtime& runtime) {
auto frame = Rect{};

{
std::scoped_lock guard(layoutEventState->mutex);

layoutEventState->isDispatching = false;

// If some *particular* `frame` was already dispatched before,
// and since then there were no other new values of the `frame`
// observed, do nothing.
if (layoutEventState->wasDispatched) {
return jsi::Value::null();
}

frame = layoutEventState->frame;

// If some *particular* `frame` was *not* already dispatched before,
// it's time to dispatch it and mark as dispatched.
layoutEventState->wasDispatched = true;
}

auto layout = jsi::Object(runtime);
layout.setProperty(runtime, "x", frame.origin.x);
layout.setProperty(runtime, "y", frame.origin.y);
layout.setProperty(runtime, "width", frame.size.width);
layout.setProperty(runtime, "height", frame.size.height);
auto payload = jsi::Object(runtime);
payload.setProperty(runtime, "layout", std::move(layout));
return jsi::Value(std::move(payload));
},
CoreFeatures::enableDefaultAsyncBatchedPriority
? EventPriority::AsynchronousBatched
: EventPriority::AsynchronousUnbatched);
dispatchEvent("layout", [layoutEventState](jsi::Runtime& runtime) {
auto frame = Rect{};

{
std::scoped_lock guard(layoutEventState->mutex);

layoutEventState->isDispatching = false;

// If some *particular* `frame` was already dispatched before,
// and since then there were no other new values of the `frame`
// observed, do nothing.
if (layoutEventState->wasDispatched) {
return jsi::Value::null();
}

frame = layoutEventState->frame;

// If some *particular* `frame` was *not* already dispatched before,
// it's time to dispatch it and mark as dispatched.
layoutEventState->wasDispatched = true;
}

auto layout = jsi::Object(runtime);
layout.setProperty(runtime, "x", frame.origin.x);
layout.setProperty(runtime, "y", frame.origin.y);
layout.setProperty(runtime, "width", frame.size.width);
layout.setProperty(runtime, "height", frame.size.height);
auto payload = jsi::Object(runtime);
payload.setProperty(runtime, "layout", std::move(layout));
return jsi::Value(std::move(payload));
});
}

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

#include <react/debug/react_native_assert.h>
#include <react/renderer/core/State.h>
#include <react/utils/CoreFeatures.h>

namespace facebook::react {

Expand Down Expand Up @@ -68,11 +67,7 @@ class ConcreteState : public State {
}

void updateState(Data&& newData) const {
updateState(
std::move(newData),
CoreFeatures::enableDefaultAsyncBatchedPriority
? EventPriority::AsynchronousBatched
: EventPriority::AsynchronousUnbatched);
updateState(std::move(newData), EventPriority::AsynchronousBatched);
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include <cxxreact/JSExecutor.h>
#include <logger/react_native_log.h>
#include <react/utils/CoreFeatures.h>
#include "EventEmitter.h"
#include "EventLogger.h"
#include "EventQueue.h"
Expand Down Expand Up @@ -71,11 +70,6 @@ void EventQueueProcessor::flushEvents(
reactPriority,
*event.eventPayload);

// We run the "Conclusion" per-event when unbatched
if (!CoreFeatures::enableDefaultAsyncBatchedPriority) {
eventPipeConclusion_(runtime);
}

if (eventLogger != nullptr) {
eventLogger->onEventProcessingEnd(event.loggingTag);
}
Expand All @@ -84,10 +78,9 @@ void EventQueueProcessor::flushEvents(
hasContinuousEventStarted_ = true;
}
}

// We only run the "Conclusion" once per event group when batched.
if (CoreFeatures::enableDefaultAsyncBatchedPriority) {
eventPipeConclusion_(runtime);
}
eventPipeConclusion_(runtime);

// No need to lock `EventEmitter::DispatchMutex()` here.
// The mutex protects from a situation when the `instanceHandle` can be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,11 @@ Scheduler::Scheduler(
runtime, eventTarget, type, priority, payload);
},
runtime);

// We only want to run this per-event if we are not batching by default
if (!CoreFeatures::enableDefaultAsyncBatchedPriority) {
if (runtimeScheduler != nullptr) {
runtimeScheduler->callExpiredTasks(runtime);
}
}
};

auto eventPipeConclusion =
[runtimeScheduler = runtimeScheduler.get()](jsi::Runtime& runtime) {
if (CoreFeatures::enableDefaultAsyncBatchedPriority &&
runtimeScheduler != nullptr) {
if (runtimeScheduler != nullptr) {
runtimeScheduler->callExpiredTasks(runtime);
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ bool CoreFeatures::cacheLastTextMeasurement = false;
bool CoreFeatures::enableGranularScrollViewStateUpdatesIOS = false;
bool CoreFeatures::enableMountHooks = false;
bool CoreFeatures::enableGranularShadowTreeStateReconciliation = false;
bool CoreFeatures::enableDefaultAsyncBatchedPriority = false;
bool CoreFeatures::enableClonelessStateProgression = false;
bool CoreFeatures::excludeYogaFromRawProps = false;
bool CoreFeatures::enableReportEventPaintTime = false;
Expand Down
3 changes: 0 additions & 3 deletions packages/react-native/ReactCommon/react/utils/CoreFeatures.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ class CoreFeatures {
// state and the last commit that updated state changed before committing.
static bool enableGranularShadowTreeStateReconciliation;

// Default state updates and events to async batched priority.
static bool enableDefaultAsyncBatchedPriority;

// When enabled, Fabric will avoid cloning notes to perform state progression.
static bool enableClonelessStateProgression;

Expand Down

0 comments on commit 57e49b5

Please sign in to comment.