Skip to content

Commit

Permalink
Move timing definitions to a specific target (facebook#45492)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#45492

Changelog: [internal]

When testing the changes in facebook#45473 / D55491870, I saw that the reported time spans for long tasks didn't perfectly align with the long tasks themselves in traces (Perfetto).

Taking a closer look, I realized that I wasn't doing the conversion between times and durations from `chrono` and `DOMHighResTimeStamp` (a `double`) correctly, and we're doing this conversion very often.

This moves the definition of `DOMHighResTimeStamp` to its own library and adds conversion methods to make sure we don't make this mistake in the future.

Reviewed By: rshest

Differential Revision: D59820241
  • Loading branch information
rubennorte authored and facebook-github-bot committed Jul 17, 2024
1 parent 37d1e8e commit 49c2437
Show file tree
Hide file tree
Showing 10 changed files with 158 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ add_react_common_subdir(jserrorhandler)
add_react_common_subdir(react/runtime)
add_react_common_subdir(react/runtime/hermes)
add_react_common_subdir(react/runtime/nativeviewconfig)
add_react_common_subdir(react/timing)

# ReactAndroid JNI targets
add_react_build_subdir(generated/source/codegen/jni)
Expand Down
1 change: 1 addition & 0 deletions packages/react-native/ReactCommon/cxxreact/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,5 @@ target_link_libraries(react_cxxreact
logger
reactperflogger
runtimeexecutor
react_timing
react_debug)
9 changes: 2 additions & 7 deletions packages/react-native/ReactCommon/cxxreact/JSExecutor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <folly/Conv.h>
#include <jsinspector-modern/ReactCdp.h>

#include <react/timing/primitives.h>
#include <chrono>

namespace facebook::react {
Expand All @@ -26,13 +27,7 @@ std::string JSExecutor::getSyntheticBundlePath(
}

double JSExecutor::performanceNow() {
auto time = std::chrono::steady_clock::now();
auto duration = std::chrono::duration_cast<std::chrono::nanoseconds>(
time.time_since_epoch())
.count();

constexpr double NANOSECONDS_IN_MILLISECOND = 1000000.0;
return duration / NANOSECONDS_IN_MILLISECOND;
return chronoToDOMHighResTimeStamp(std::chrono::steady_clock::now());
}

jsinspector_modern::RuntimeTargetDelegate&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ Pod::Spec.new do |s|
s.dependency "React-jsi", version
s.dependency "React-logger", version
s.dependency "React-debug", version
s.dependency "React-timing", version

s.resource_bundles = {'React-cxxreact_privacy' => 'PrivacyInfo.xcprivacy'}

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

#pragma once

#include <react/timing/primitives.h>
#include "BoundedConsumableBuffer.h"

#include <array>
Expand All @@ -20,8 +21,6 @@

namespace facebook::react {

using DOMHighResTimeStamp = double;

using PerformanceEntryInteractionId = uint32_t;

enum class PerformanceEntryType {
Expand Down
20 changes: 20 additions & 0 deletions packages/react-native/ReactCommon/react/timing/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

cmake_minimum_required(VERSION 3.13)
set(CMAKE_VERBOSE_MAKEFILE on)

add_compile_options(
-fexceptions
-frtti
-std=c++20
-Wall
-Wpedantic
-DLOG_TAG=\"ReactNative\")

file(GLOB react_timing_SRC CONFIGURE_DEPENDS *.cpp)
add_library(react_timing SHARED ${react_timing_SRC})

target_include_directories(react_timing PUBLIC ${REACT_COMMON_DIR})
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Copyright (c) Meta Platforms, Inc. and affiliates.
#
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.

require "json"

package = JSON.parse(File.read(File.join(__dir__, "..", "..", "..", "package.json")))
version = package['version']

source = { :git => 'https://github.com/facebook/react-native.git' }
if version == '1000.0.0'
# This is an unpublished version, use the latest commit hash of the react-native repo, which we’re presumably in.
source[:commit] = `git rev-parse HEAD`.strip if system("git rev-parse --git-dir > /dev/null 2>&1")
else
source[:tag] = "v#{version}"
end

folly_config = get_folly_config()
folly_compiler_flags = folly_config[:compiler_flags]
folly_version = folly_config[:version]

header_search_paths = [
"\"$(PODS_ROOT)/RCT-Folly\"",
"\"$(PODS_ROOT)/boost\"",
]

if ENV['USE_FRAMEWORKS']
header_search_paths << "\"$(PODS_TARGET_SRCROOT)/../..\"" # this is needed to allow the timing access its own files
end

Pod::Spec.new do |s|
s.name = "React-timing"
s.version = version
s.summary = "React performance timing utilities"
s.homepage = "https://reactnative.dev/"
s.license = package["license"]
s.author = "Meta Platforms, Inc. and its affiliates"
s.platforms = min_supported_versions
s.source = source
s.source_files = "**/*.{cpp,h}"
s.compiler_flags = folly_compiler_flags
s.header_dir = "react/timing"
s.exclude_files = "tests"
s.pod_target_xcconfig = {
"CLANG_CXX_LANGUAGE_STANDARD" => rct_cxx_language_standard(),
"HEADER_SEARCH_PATHS" => header_search_paths.join(' ')}

if ENV['USE_FRAMEWORKS']
s.module_name = "React_timing"
s.header_mappings_dir = "../.."
end
end
31 changes: 31 additions & 0 deletions packages/react-native/ReactCommon/react/timing/primitives.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#pragma once

#include <chrono>

namespace facebook::react {

// `DOMHighResTimeStamp` represents a time value in milliseconds (time point or
// duration), with sub-millisecond precision.
// On the Web, the precision can be reduced for security purposes, but that is
// not necessary in React Native.
using DOMHighResTimeStamp = double;

inline DOMHighResTimeStamp chronoToDOMHighResTimeStamp(
std::chrono::steady_clock::duration duration) {
return static_cast<std::chrono::duration<double, std::milli>>(duration)
.count();
}

inline DOMHighResTimeStamp chronoToDOMHighResTimeStamp(
std::chrono::steady_clock::time_point timePoint) {
return chronoToDOMHighResTimeStamp(timePoint.time_since_epoch());
}

} // namespace facebook::react
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#include <gtest/gtest.h>

#include "../primitives.h"

namespace facebook::react {

using Clock = std::chrono::steady_clock;
using TimePoint = std::chrono::time_point<Clock>;

TEST(chronoToDOMHighResTimeStamp, withDurations) {
EXPECT_EQ(chronoToDOMHighResTimeStamp(std::chrono::nanoseconds(10)), 0.00001);
EXPECT_EQ(chronoToDOMHighResTimeStamp(std::chrono::microseconds(10)), 0.01);
EXPECT_EQ(chronoToDOMHighResTimeStamp(std::chrono::milliseconds(10)), 10.0);
EXPECT_EQ(chronoToDOMHighResTimeStamp(std::chrono::seconds(10)), 10000.0);
EXPECT_EQ(
chronoToDOMHighResTimeStamp(
std::chrono::seconds(1) + std::chrono::nanoseconds(20)),
1000.000020);
}

TEST(chronoToDOMHighResTimeStamp, withTimePoints) {
EXPECT_EQ(
chronoToDOMHighResTimeStamp(TimePoint(std::chrono::nanoseconds(10))),
0.00001);
EXPECT_EQ(
chronoToDOMHighResTimeStamp(TimePoint(std::chrono::microseconds(10))),
0.01);
EXPECT_EQ(
chronoToDOMHighResTimeStamp(TimePoint(std::chrono::milliseconds(10))),
10.0);
EXPECT_EQ(
chronoToDOMHighResTimeStamp(TimePoint(std::chrono::seconds(10))),
10000.0);
EXPECT_EQ(
chronoToDOMHighResTimeStamp(
TimePoint(std::chrono::seconds(1) + std::chrono::nanoseconds(20))),
1000.000020);
}

} // namespace facebook::react
1 change: 1 addition & 0 deletions packages/react-native/scripts/react_native_pods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ def use_react_native! (

pod 'React-callinvoker', :path => "#{prefix}/ReactCommon/callinvoker"
pod 'React-performancetimeline', :path => "#{prefix}/ReactCommon/react/performance/timeline"
pod 'React-timing', :path => "#{prefix}/ReactCommon/react/timing"
pod 'React-runtimeexecutor', :path => "#{prefix}/ReactCommon/runtimeexecutor"
pod 'React-runtimescheduler', :path => "#{prefix}/ReactCommon/react/renderer/runtimescheduler"
pod 'React-rendererdebug', :path => "#{prefix}/ReactCommon/react/renderer/debug"
Expand Down

0 comments on commit 49c2437

Please sign in to comment.