Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect timing for events that do not update the UI #46253

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
ShadowNodeFamily::Shared createFamily(
const ShadowNodeFamilyFragment& fragment) const override {
auto eventEmitter = std::make_shared<const ConcreteEventEmitter>(
std::make_shared<EventTarget>(fragment.instanceHandle),
std::make_shared<EventTarget>(
fragment.instanceHandle, fragment.surfaceId),
eventDispatcher_);
return std::make_shared<ShadowNodeFamily>(
fragment, std::move(eventEmitter), eventDispatcher_, *this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ void EventDispatcher::dispatchEvent(RawEvent&& rawEvent) const {

auto eventLogger = eventLogger_.lock();
if (eventLogger != nullptr) {
rawEvent.loggingTag = eventLogger->onEventStart(rawEvent.type);
rawEvent.loggingTag =
eventLogger->onEventStart(rawEvent.type, rawEvent.eventTarget);
}
eventQueue_.enqueueEvent(std::move(rawEvent));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#pragma once

#include <react/renderer/core/EventTarget.h>
#include <string_view>

namespace facebook::react {
Expand All @@ -27,7 +28,9 @@ class EventLogger {
* Called when an event is first created, returns and unique tag for this
* event, which can be used to log further event processing stages.
*/
virtual EventTag onEventStart(std::string_view name) = 0;
virtual EventTag onEventStart(
std::string_view name,
SharedEventTarget target) = 0;

/*
* Called when event starts getting dispatched (processed by the handlers, if
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ namespace facebook::react {

using Tag = EventTarget::Tag;

EventTarget::EventTarget(InstanceHandle::Shared instanceHandle)
EventTarget::EventTarget(
InstanceHandle::Shared instanceHandle,
SurfaceId surfaceId)
: instanceHandle_(std::move(instanceHandle)),
surfaceId_(surfaceId),
strongInstanceHandle_(jsi::Value::null()) {}

void EventTarget::setEnabled(bool enabled) const {
Expand Down Expand Up @@ -64,6 +67,10 @@ jsi::Value EventTarget::getInstanceHandle(jsi::Runtime& runtime) const {
return jsi::Value(runtime, strongInstanceHandle_);
}

SurfaceId EventTarget::getSurfaceId() const {
return surfaceId_;
}

Tag EventTarget::getTag() const {
return instanceHandle_->getTag();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ class EventTarget {
/*
* Constructs an EventTarget from a weak instance handler and a tag.
*/
explicit EventTarget(InstanceHandle::Shared instanceHandle);
explicit EventTarget(
InstanceHandle::Shared instanceHandle,
SurfaceId surfaceId);

/*
* Sets the `enabled` flag that allows creating a strong instance handle from
Expand All @@ -59,13 +61,16 @@ class EventTarget {
*/
jsi::Value getInstanceHandle(jsi::Runtime& runtime) const;

SurfaceId getSurfaceId() const;

/*
* Deprecated. Do not use.
*/
Tag getTag() const;

private:
const InstanceHandle::Shared instanceHandle_;
const SurfaceId surfaceId_;
mutable bool enabled_{false}; // Protected by `EventEmitter::DispatchMutex()`.
mutable jsi::Value strongInstanceHandle_; // Protected by `jsi::Runtime &`.
mutable size_t retainCount_{0}; // Protected by `jsi::Runtime &`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <react/renderer/core/EventLogger.h>
#include <react/renderer/core/EventPipe.h>
#include <react/renderer/core/EventQueueProcessor.h>
#include <react/renderer/core/EventTarget.h>
#include <react/renderer/core/StatePipe.h>
#include <react/renderer/core/ValueFactoryEventPayload.h>

Expand All @@ -20,7 +21,8 @@
namespace facebook::react {

class MockEventLogger : public EventLogger {
EventTag onEventStart(std::string_view /*name*/) override {
EventTag onEventStart(std::string_view /*name*/, SharedEventTarget /*target*/)
override {
return EMPTY_EVENT_TAG;
}
void onEventProcessingStart(EventTag /*tag*/) override {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ TEST(EventTargetTests, getInstanceHandle) {

EXPECT_EQ(instanceHandle->getTag(), 1);

auto eventTarget = EventTarget(std::move(instanceHandle));
auto eventTarget = EventTarget(std::move(instanceHandle), 41);

EXPECT_EQ(eventTarget.getTag(), 1);

EXPECT_EQ(eventTarget.getSurfaceId(), 41);

EXPECT_TRUE(eventTarget.getInstanceHandle(*runtime).isNull());

eventTarget.retain(*runtime);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ add_library(react_render_observers_events OBJECT ${react_render_observers_events
target_include_directories(react_render_observers_events PUBLIC ${REACT_COMMON_DIR})
target_link_libraries(react_render_observers_events
react_performance_timeline
react_timing
react_render_core
react_render_runtimescheduler
react_featureflags
react_render_uimanager
react_utils)
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,29 @@
#include "EventPerformanceLogger.h"

#include <react/featureflags/ReactNativeFeatureFlags.h>
#include <react/timing/primitives.h>
#include <react/utils/CoreFeatures.h>
#include <unordered_map>

namespace facebook::react {

namespace {

bool isTargetInRootShadowNode(
const SharedEventTarget& target,
const RootShadowNode::Shared& rootShadowNode) {
return target && rootShadowNode &&
target->getSurfaceId() == rootShadowNode->getSurfaceId();
}

bool hasPendingRenderingUpdates(
const SharedEventTarget& target,
const std::unordered_set<SurfaceId>&
surfaceIdsWithPendingRenderingUpdates) {
return target != nullptr &&
surfaceIdsWithPendingRenderingUpdates.contains(target->getSurfaceId());
}

struct StrKey {
size_t key;
StrKey(std::string_view s) : key(std::hash<std::string_view>{}(s)) {}
Expand Down Expand Up @@ -85,7 +101,9 @@ EventPerformanceLogger::EventPerformanceLogger(
std::weak_ptr<PerformanceEntryReporter> performanceEntryReporter)
: performanceEntryReporter_(std::move(performanceEntryReporter)) {}

EventTag EventPerformanceLogger::onEventStart(std::string_view name) {
EventTag EventPerformanceLogger::onEventStart(
std::string_view name,
SharedEventTarget target) {
auto performanceEntryReporter = performanceEntryReporter_.lock();
if (performanceEntryReporter == nullptr) {
return EMPTY_EVENT_TAG;
Expand All @@ -104,7 +122,8 @@ EventTag EventPerformanceLogger::onEventStart(std::string_view name) {
auto timeStamp = performanceEntryReporter->getCurrentTimeStamp();
{
std::lock_guard lock(eventsInFlightMutex_);
eventsInFlight_.emplace(eventTag, EventEntry{reportedName, timeStamp, 0.0});
eventsInFlight_.emplace(
eventTag, EventEntry{reportedName, target, timeStamp, 0.0});
}
return eventTag;
}
Expand Down Expand Up @@ -138,12 +157,13 @@ void EventPerformanceLogger::onEventProcessingEnd(EventTag tag) {
if (it == eventsInFlight_.end()) {
return;
}

auto& entry = it->second;
entry.processingEndTime = timeStamp;

if (ReactNativeFeatureFlags::enableReportEventPaintTime()) {
// If reporting paint time, don't send the entry just yet and wait for the
// mount hook callback to be called
// task to finish.
return;
}

Expand All @@ -160,8 +180,45 @@ void EventPerformanceLogger::onEventProcessingEnd(EventTag tag) {
}
}

void EventPerformanceLogger::dispatchPendingEventTimingEntries(
const std::unordered_set<SurfaceId>&
surfaceIdsWithPendingRenderingUpdates) {
if (!ReactNativeFeatureFlags::enableReportEventPaintTime()) {
return;
}

auto performanceEntryReporter = performanceEntryReporter_.lock();
if (performanceEntryReporter == nullptr) {
return;
}

std::lock_guard lock(eventsInFlightMutex_);
auto it = eventsInFlight_.begin();
while (it != eventsInFlight_.end()) {
auto& entry = it->second;

if (entry.isWaitingForDispatch() || entry.isWaitingForMount) {
++it;
} else if (hasPendingRenderingUpdates(
entry.target, surfaceIdsWithPendingRenderingUpdates)) {
// We'll wait for mount to report the event
entry.isWaitingForMount = true;
++it;
} else {
performanceEntryReporter->logEventEntry(
std::string(entry.name),
entry.startTime,
performanceEntryReporter->getCurrentTimeStamp() - entry.startTime,
entry.processingStartTime,
entry.processingEndTime,
entry.interactionId);
it = eventsInFlight_.erase(it);
}
}
}

void EventPerformanceLogger::shadowTreeDidMount(
const RootShadowNode::Shared& /*rootShadowNode*/,
const RootShadowNode::Shared& rootShadowNode,
double mountTime) noexcept {
if (!ReactNativeFeatureFlags::enableReportEventPaintTime()) {
return;
Expand All @@ -176,20 +233,19 @@ void EventPerformanceLogger::shadowTreeDidMount(
auto it = eventsInFlight_.begin();
while (it != eventsInFlight_.end()) {
const auto& entry = it->second;
if (entry.processingEndTime == 0.0 || entry.processingEndTime > mountTime) {
// This mount doesn't correspond to the event
if (entry.isWaitingForMount &&
isTargetInRootShadowNode(entry.target, rootShadowNode)) {
performanceEntryReporter->logEventEntry(
std::string(entry.name),
entry.startTime,
mountTime - entry.startTime,
entry.processingStartTime,
entry.processingEndTime,
entry.interactionId);
it = eventsInFlight_.erase(it);
} else {
++it;
continue;
}

performanceEntryReporter->logEventEntry(
std::string(entry.name),
entry.startTime,
mountTime - entry.startTime,
entry.processingStartTime,
entry.processingEndTime,
entry.interactionId);
it = eventsInFlight_.erase(it);
}
}

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

#include <react/performance/timeline/PerformanceEntryReporter.h>
#include <react/renderer/core/EventLogger.h>
#include <react/renderer/runtimescheduler/RuntimeSchedulerEventTimingDelegate.h>
#include <react/renderer/uimanager/UIManagerMountHook.h>
#include <memory>
#include <mutex>
Expand All @@ -17,17 +18,26 @@

namespace facebook::react {

class EventPerformanceLogger : public EventLogger, public UIManagerMountHook {
class EventPerformanceLogger : public EventLogger,
public RuntimeSchedulerEventTimingDelegate,
public UIManagerMountHook {
public:
explicit EventPerformanceLogger(
std::weak_ptr<PerformanceEntryReporter> performanceEntryReporter);

#pragma mark - EventLogger

EventTag onEventStart(std::string_view name) override;
EventTag onEventStart(std::string_view name, SharedEventTarget target)
override;
void onEventProcessingStart(EventTag tag) override;
void onEventProcessingEnd(EventTag tag) override;

#pragma mark - RuntimeSchedulerEventTimingDelegate

void dispatchPendingEventTimingEntries(
const std::unordered_set<SurfaceId>&
surfaceIdsWithPendingRenderingUpdates) override;

#pragma mark - UIManagerMountHook

void shadowTreeDidMount(
Expand All @@ -37,13 +47,20 @@ class EventPerformanceLogger : public EventLogger, public UIManagerMountHook {
private:
struct EventEntry {
std::string_view name;
SharedEventTarget target{nullptr};
DOMHighResTimeStamp startTime{0.0};
DOMHighResTimeStamp processingStartTime{0.0};
DOMHighResTimeStamp processingEndTime{0.0};

bool isWaitingForMount{false};

// TODO: Define the way to assign interaction IDs to the event chains
// (T141358175)
PerformanceEntryInteractionId interactionId{0};

bool isWaitingForDispatch() {
return processingEndTime == 0.0;
}
};

// Registry to store the events that are currently ongoing.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,10 @@ void RuntimeScheduler::callExpiredTasks(jsi::Runtime& runtime) {
}

void RuntimeScheduler::scheduleRenderingUpdate(
SurfaceId surfaceId,
RuntimeSchedulerRenderingUpdate&& renderingUpdate) {
return runtimeSchedulerImpl_->scheduleRenderingUpdate(
std::move(renderingUpdate));
surfaceId, std::move(renderingUpdate));
}

void RuntimeScheduler::setShadowTreeRevisionConsistencyManager(
Expand All @@ -119,4 +120,9 @@ void RuntimeScheduler::setPerformanceEntryReporter(
performanceEntryReporter);
}

void RuntimeScheduler::setEventTimingDelegate(
RuntimeSchedulerEventTimingDelegate* eventTimingDelegate) {
return runtimeSchedulerImpl_->setEventTimingDelegate(eventTimingDelegate);
}

} // namespace facebook::react
Loading
Loading