From cb48b502902a54c9becef7ecc5d83c5b1e753136 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Thu, 27 Aug 2020 12:31:42 -0700 Subject: [PATCH] Fabric: Storing commit revision number in `MountingTelemetry` Summary: Now we store a revision number of a Shadow Tree that leads to a transaction for which the concrete instance of MountingTelemetry corresponds. This is useful to understand how many actual transactions were skipped during a mounting phase (a mounting transaction does not directly correspond to a commit operation). Changelog: [Internal] Fabric-specific internal change. Reviewed By: mdvacca Differential Revision: D23364663 fbshipit-source-id: 32b86bcdfc1ae97d8fff3b97a8615cc5a5b4d4a9 --- .../main/java/com/facebook/react/fabric/jni/Binding.cpp | 7 ++++--- .../react/renderer/mounting/MountingTelemetry.cpp | 9 ++++++--- ReactCommon/react/renderer/mounting/MountingTelemetry.h | 8 ++++---- ReactCommon/react/renderer/mounting/ShadowTree.cpp | 1 + ReactCommon/react/renderer/mounting/SurfaceTelemetry.cpp | 5 +++++ ReactCommon/react/renderer/mounting/SurfaceTelemetry.h | 2 ++ .../renderer/mounting/tests/MountingTelemetryTest.cpp | 3 +++ 7 files changed, 25 insertions(+), 10 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp index 37b995cf52f..ec5e08a0474 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp @@ -609,7 +609,8 @@ void Binding::schedulerDidFinishTransaction( } } } - int64_t commitNumber = telemetry.getCommitNumber(); + + auto revisionNumber = telemetry.getRevisionNumber(); std::vector> queue; // Upper bound estimation of mount items to be delivered to Java side. @@ -813,7 +814,7 @@ void Binding::schedulerDidFinishTransaction( surfaceId, position == 0 ? nullptr : mountItemsArray.get(), position, - commitNumber); + revisionNumber); static auto scheduleMountItem = jni::findClassStatic(Binding::UIManagerJavaDescriptor) @@ -833,7 +834,7 @@ void Binding::schedulerDidFinishTransaction( scheduleMountItem( localJavaUIManager, batch.get(), - telemetry.getCommitNumber(), + telemetry.getRevisionNumber(), telemetryTimePointToMilliseconds(telemetry.getCommitStartTime()), telemetryTimePointToMilliseconds(telemetry.getDiffStartTime()), telemetryTimePointToMilliseconds(telemetry.getDiffEndTime()), diff --git a/ReactCommon/react/renderer/mounting/MountingTelemetry.cpp b/ReactCommon/react/renderer/mounting/MountingTelemetry.cpp index 8e52c76ec36..58680b2d059 100644 --- a/ReactCommon/react/renderer/mounting/MountingTelemetry.cpp +++ b/ReactCommon/react/renderer/mounting/MountingTelemetry.cpp @@ -30,7 +30,6 @@ void MountingTelemetry::willCommit() { assert(commitStartTime_ == kTelemetryUndefinedTimePoint); assert(commitEndTime_ == kTelemetryUndefinedTimePoint); commitStartTime_ = telemetryTimePointNow(); - commitNumber_++; } void MountingTelemetry::didCommit() { @@ -79,6 +78,10 @@ void MountingTelemetry::didMount() { mountEndTime_ = telemetryTimePointNow(); } +void MountingTelemetry::setRevisionNumber(int revisionNumber) { + revisionNumber_ = revisionNumber; +} + TelemetryTimePoint MountingTelemetry::getDiffStartTime() const { assert(diffStartTime_ != kTelemetryUndefinedTimePoint); assert(diffEndTime_ != kTelemetryUndefinedTimePoint); @@ -131,8 +134,8 @@ int MountingTelemetry::getNumberOfTextMeasurements() const { return numberOfTextMeasurements_; } -int MountingTelemetry::getCommitNumber() const { - return commitNumber_; +int MountingTelemetry::getRevisionNumber() const { + return revisionNumber_; } } // namespace react diff --git a/ReactCommon/react/renderer/mounting/MountingTelemetry.h b/ReactCommon/react/renderer/mounting/MountingTelemetry.h index 52f04deb70d..9082830b75b 100644 --- a/ReactCommon/react/renderer/mounting/MountingTelemetry.h +++ b/ReactCommon/react/renderer/mounting/MountingTelemetry.h @@ -42,6 +42,8 @@ class MountingTelemetry final { void willMount(); void didMount(); + void setRevisionNumber(int revisionNumber); + /* * Reading */ @@ -55,8 +57,7 @@ class MountingTelemetry final { TelemetryTimePoint getMountEndTime() const; int getNumberOfTextMeasurements() const; - - int getCommitNumber() const; + int getRevisionNumber() const; private: TelemetryTimePoint diffStartTime_{kTelemetryUndefinedTimePoint}; @@ -69,8 +70,7 @@ class MountingTelemetry final { TelemetryTimePoint mountEndTime_{kTelemetryUndefinedTimePoint}; int numberOfTextMeasurements_{0}; - - int commitNumber_{0}; + int revisionNumber_{0}; }; } // namespace react diff --git a/ReactCommon/react/renderer/mounting/ShadowTree.cpp b/ReactCommon/react/renderer/mounting/ShadowTree.cpp index 3ab3194d183..9f48e008b05 100644 --- a/ReactCommon/react/renderer/mounting/ShadowTree.cpp +++ b/ReactCommon/react/renderer/mounting/ShadowTree.cpp @@ -361,6 +361,7 @@ bool ShadowTree::tryCommit( emitLayoutEvents(affectedLayoutableNodes); telemetry.didCommit(); + telemetry.setRevisionNumber(revisionNumber); mountingCoordinator_->push( ShadowTreeRevision{newRootShadowNode, revisionNumber, telemetry}); diff --git a/ReactCommon/react/renderer/mounting/SurfaceTelemetry.cpp b/ReactCommon/react/renderer/mounting/SurfaceTelemetry.cpp index 5588dedbc94..11e48f9386d 100644 --- a/ReactCommon/react/renderer/mounting/SurfaceTelemetry.cpp +++ b/ReactCommon/react/renderer/mounting/SurfaceTelemetry.cpp @@ -23,6 +23,7 @@ void SurfaceTelemetry::incorporate( numberOfTransactions_++; numberOfMutations_ += numberOfMutations; numberOfTextMeasurements_ += telemetry.getNumberOfTextMeasurements(); + lastRevisionNumber_ = telemetry.getRevisionNumber(); while (recentCommitTelemetries_.size() >= kMaxNumberOfRecordedCommitTelemetries) { @@ -60,6 +61,10 @@ int SurfaceTelemetry::getNumberOfTextMeasurements() const { return numberOfTextMeasurements_; } +int SurfaceTelemetry::getLastRevisionNumber() const { + return lastRevisionNumber_; +} + std::vector SurfaceTelemetry::getRecentCommitTelemetries() const { auto result = std::vector{}; diff --git a/ReactCommon/react/renderer/mounting/SurfaceTelemetry.h b/ReactCommon/react/renderer/mounting/SurfaceTelemetry.h index f6f106864b1..2f2edca343c 100644 --- a/ReactCommon/react/renderer/mounting/SurfaceTelemetry.h +++ b/ReactCommon/react/renderer/mounting/SurfaceTelemetry.h @@ -35,6 +35,7 @@ class SurfaceTelemetry final { int getNumberOfTransactions() const; int getNumberOfMutations() const; int getNumberOfTextMeasurements() const; + int getLastRevisionNumber() const; std::vector getRecentCommitTelemetries() const; @@ -53,6 +54,7 @@ class SurfaceTelemetry final { int numberOfTransactions_{}; int numberOfMutations_{}; int numberOfTextMeasurements_{}; + int lastRevisionNumber_{}; better::small_vector recentCommitTelemetries_{}; diff --git a/ReactCommon/react/renderer/mounting/tests/MountingTelemetryTest.cpp b/ReactCommon/react/renderer/mounting/tests/MountingTelemetryTest.cpp index 3a93dd2b2c8..45ac98a2bde 100644 --- a/ReactCommon/react/renderer/mounting/tests/MountingTelemetryTest.cpp +++ b/ReactCommon/react/renderer/mounting/tests/MountingTelemetryTest.cpp @@ -57,6 +57,8 @@ TEST(MountingTelemetryTest, normalUseCase) { sleep(0.1); telemetry.didCommit(); + telemetry.setRevisionNumber(42); + telemetry.unsetAsThreadLocal(); sleep(0.3); @@ -77,6 +79,7 @@ TEST(MountingTelemetryTest, normalUseCase) { EXPECT_EQ_WITH_THRESHOLD(mountDuration, 100, threshold); EXPECT_EQ(telemetry.getNumberOfTextMeasurements(), 3); + EXPECT_EQ(telemetry.getRevisionNumber(), 42); } TEST(MountingTelemetryTest, abnormalUseCases) {