Skip to content

Commit

Permalink
Core: ViewEventEmitter should not emit duplicate onLayout events
Browse files Browse the repository at this point in the history
Summary:
Simple hack to prevent duplicate onLayout events from being emitted to JS.

Because of the addition of State Reconciliation to the Fabric lifecycle (see previous diff), in certain circumstances entire subtrees can be relayed-out many, many times even though they aren't changing. For JS product code that responds to every onLayout and forces a ReactJS tree commit (see: some usages of VirtualizedList) this can cause an async infinite loop of commits and layouts even though the tree and the LayoutMetrics aren't actually changing. Even though nothing is changing, it can still cause serious performance regressions and even some bugs since the internals of various state machines may assume onLayout won't be called many times.

Changelog: [Internal]

Reviewed By: shergin

Differential Revision: D19715280

fbshipit-source-id: d879e24f1c7b1f710ad430b7473aa9293d093dea
  • Loading branch information
JoshuaGross authored and facebook-github-bot committed Feb 5, 2020
1 parent 93c4c24 commit 928fe12
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 0 deletions.
11 changes: 11 additions & 0 deletions ReactCommon/fabric/components/view/ViewEventEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,17 @@ void ViewEventEmitter::onAccessibilityEscape() const {
#pragma mark - Layout

void ViewEventEmitter::onLayout(const LayoutMetrics &layoutMetrics) const {
// Due to State Reconciliation, `onLayout` can be called potentially many
// times with identical layoutMetrics. Ensure that the JS event is only
// dispatched when the value changes.
auto lastLayoutMetricsValue = lastLayoutMetrics_.load();
// compare_exchange_strong atomically swap the values if they're different, or
// return false if the values are the same.
if (!lastLayoutMetrics_.compare_exchange_strong(
lastLayoutMetricsValue, layoutMetrics)) {
return;
}

dispatchEvent("layout", [frame = layoutMetrics.frame](jsi::Runtime &runtime) {
auto layout = jsi::Object(runtime);
layout.setProperty(runtime, "x", frame.origin.x);
Expand Down
4 changes: 4 additions & 0 deletions ReactCommon/fabric/components/view/ViewEventEmitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#pragma once

#include <atomic>
#include <memory>

#include <react/core/LayoutMetrics.h>
Expand Down Expand Up @@ -35,6 +36,9 @@ class ViewEventEmitter : public TouchEventEmitter {
#pragma mark - Layout

void onLayout(const LayoutMetrics &layoutMetrics) const;

private:
mutable std::atomic<LayoutMetrics> lastLayoutMetrics_;
};

} // namespace react
Expand Down

0 comments on commit 928fe12

Please sign in to comment.