Skip to content

Commit 5485f8a

Browse files
javachefacebook-github-bot
javache
authored and
facebook-github-bot
committed
Unify bridge and bridgeless ReactMarker implementations
Summary: The split between `logTaggedMarkerImpl` and `logTaggedMarkerBridgelessImpl` no longer earns its keep — bridgeless callers always pass the same instance key, and the bridge mode is on its way out. Collapse the two paths into a single hook so platform code only has to wire up one slot. `LogTaggedMarkerBridgeless` becomes a type alias for `LogTaggedMarker`. The free `logMarkerBridgeless` / `logTaggedMarkerBridgeless` functions and the `logTaggedMarkerBridgelessImpl` slot stay around as `[[deprecated]]` shims. Call-sites in `ReactInstance` switch to the unified `logMarker` / `logTaggedMarker`. On the Android side, `logPerfMarkerBridgeless` and the `logPerfMarkerWithInstanceKey` indirection go away — there's only one instance key now. Changelog: [General][Deprecated] - Deprecate `logMarkerBridgeless`, `logTaggedMarkerBridgeless`, and `logTaggedMarkerBridgelessImpl` in favor of the non-bridgeless variants Differential Revision: D88068665
1 parent f07c7cc commit 5485f8a

6 files changed

Lines changed: 82 additions & 92 deletions

File tree

‎packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.cpp‎

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,15 @@
1313

1414
namespace facebook::react {
1515

16+
namespace {
17+
const int kInstanceKey = 1;
18+
}
19+
1620
void JReactMarker::setLogPerfMarkerIfNeeded() {
1721
static std::once_flag flag{};
1822
std::call_once(flag, []() {
1923
std::unique_lock lock(ReactMarker::logTaggedMarkerImplMutex);
2024
ReactMarker::logTaggedMarkerImpl = JReactMarker::logPerfMarker;
21-
ReactMarker::logTaggedMarkerBridgelessImpl =
22-
JReactMarker::logPerfMarkerBridgeless;
2325
});
2426
}
2527

@@ -51,21 +53,6 @@ void JReactMarker::logMarker(
5153
void JReactMarker::logPerfMarker(
5254
const ReactMarker::ReactMarkerId markerId,
5355
const char* tag) {
54-
const int bridgeInstanceKey = 0;
55-
logPerfMarkerWithInstanceKey(markerId, tag, bridgeInstanceKey);
56-
}
57-
58-
void JReactMarker::logPerfMarkerBridgeless(
59-
const ReactMarker::ReactMarkerId markerId,
60-
const char* tag) {
61-
const int bridgelessInstanceKey = 1;
62-
logPerfMarkerWithInstanceKey(markerId, tag, bridgelessInstanceKey);
63-
}
64-
65-
void JReactMarker::logPerfMarkerWithInstanceKey(
66-
const ReactMarker::ReactMarkerId markerId,
67-
const char* tag,
68-
const int instanceKey) {
6956
switch (markerId) {
7057
case ReactMarker::APP_STARTUP_START:
7158
JReactMarker::logMarker("APP_STARTUP_START");
@@ -80,10 +67,10 @@ void JReactMarker::logPerfMarkerWithInstanceKey(
8067
JReactMarker::logMarker("INIT_REACT_RUNTIME_END");
8168
break;
8269
case ReactMarker::RUN_JS_BUNDLE_START:
83-
JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag, instanceKey);
70+
JReactMarker::logMarker("RUN_JS_BUNDLE_START", tag, kInstanceKey);
8471
break;
8572
case ReactMarker::RUN_JS_BUNDLE_STOP:
86-
JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag, instanceKey);
73+
JReactMarker::logMarker("RUN_JS_BUNDLE_END", tag, kInstanceKey);
8774
break;
8875
case ReactMarker::CREATE_REACT_CONTEXT_STOP:
8976
JReactMarker::logMarker("CREATE_REACT_CONTEXT_END");
@@ -95,16 +82,16 @@ void JReactMarker::logPerfMarkerWithInstanceKey(
9582
JReactMarker::logMarker("loadApplicationScript_endStringConvert");
9683
break;
9784
case ReactMarker::NATIVE_MODULE_SETUP_START:
98-
JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag, instanceKey);
85+
JReactMarker::logMarker("NATIVE_MODULE_SETUP_START", tag, kInstanceKey);
9986
break;
10087
case ReactMarker::NATIVE_MODULE_SETUP_STOP:
101-
JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag, instanceKey);
88+
JReactMarker::logMarker("NATIVE_MODULE_SETUP_END", tag, kInstanceKey);
10289
break;
10390
case ReactMarker::REGISTER_JS_SEGMENT_START:
104-
JReactMarker::logMarker("REGISTER_JS_SEGMENT_START", tag, instanceKey);
91+
JReactMarker::logMarker("REGISTER_JS_SEGMENT_START", tag, kInstanceKey);
10592
break;
10693
case ReactMarker::REGISTER_JS_SEGMENT_STOP:
107-
JReactMarker::logMarker("REGISTER_JS_SEGMENT_STOP", tag, instanceKey);
94+
JReactMarker::logMarker("REGISTER_JS_SEGMENT_STOP", tag, kInstanceKey);
10895
break;
10996
case ReactMarker::NATIVE_REQUIRE_START:
11097
case ReactMarker::NATIVE_REQUIRE_STOP:

‎packages/react-native/ReactAndroid/src/main/jni/react/jni/JReactMarker.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ class JReactMarker : public facebook::jni::JavaClass<JReactMarker> {
2525
static void logMarker(const std::string &marker, const std::string &tag);
2626
static void logMarker(const std::string &marker, const std::string &tag, int instanceKey);
2727
static void logPerfMarker(ReactMarker::ReactMarkerId markerId, const char *tag);
28-
static void logPerfMarkerBridgeless(ReactMarker::ReactMarkerId markerId, const char *tag);
29-
static void logPerfMarkerWithInstanceKey(ReactMarker::ReactMarkerId markerId, const char *tag, int instanceKey);
3028
static void nativeLogMarker(jni::alias_ref<jclass> /* unused */, const std::string &markerNameStr, jlong markerTime);
3129
};
3230

‎packages/react-native/ReactCommon/cxxreact/ReactMarker.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,11 @@ void logTaggedMarker(const ReactMarkerId markerId, const char* tag) {
3838
}
3939

4040
void logMarkerBridgeless(const ReactMarkerId markerId) {
41-
logTaggedMarkerBridgeless(markerId, nullptr);
41+
logTaggedMarker(markerId, nullptr);
4242
}
4343

4444
void logTaggedMarkerBridgeless(const ReactMarkerId markerId, const char* tag) {
45-
logTaggedMarkerBridgelessImpl(markerId, tag);
45+
logTaggedMarker(markerId, tag);
4646
}
4747

4848
void logMarkerDone(const ReactMarkerId markerId, double markerTime) {

‎packages/react-native/ReactCommon/cxxreact/ReactMarker.h‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,13 @@ enum ReactMarkerId {
3737
};
3838

3939
#ifdef __APPLE__
40-
using LogTaggedMarker = std::function<void(const ReactMarkerId, const char *tag)>; // Bridge only
41-
using LogTaggedMarkerBridgeless = std::function<void(const ReactMarkerId, const char *tag)>;
40+
using LogTaggedMarker = std::function<void(const ReactMarkerId, const char *tag)>;
4241
#else
43-
typedef void (*LogTaggedMarker)(const ReactMarkerId, const char *tag); // Bridge only
44-
typedef void (*LogTaggedMarkerBridgeless)(const ReactMarkerId, const char *tag);
42+
typedef void (*LogTaggedMarker)(const ReactMarkerId, const char *tag);
4543
#endif
4644

45+
using LogTaggedMarkerBridgeless = LogTaggedMarker;
46+
4747
#ifndef RN_EXPORT
4848
#define RN_EXPORT __attribute__((visibility("default")))
4949
#endif
@@ -52,12 +52,14 @@ extern RN_EXPORT std::shared_mutex logTaggedMarkerImplMutex;
5252
/// - important: To ensure this gets read and written to in a thread safe
5353
/// manner, make use of `logTaggedMarkerImplMutex`.
5454
extern RN_EXPORT LogTaggedMarker logTaggedMarkerImpl;
55+
[[deprecated("Use logTaggedMarkerImpl instead")]]
5556
extern RN_EXPORT LogTaggedMarker logTaggedMarkerBridgelessImpl;
5657

57-
extern RN_EXPORT void logMarker(ReactMarkerId markerId); // Bridge only
58-
extern RN_EXPORT void logTaggedMarker(ReactMarkerId markerId,
59-
const char *tag); // Bridge only
58+
extern RN_EXPORT void logMarker(ReactMarkerId markerId);
59+
extern RN_EXPORT void logTaggedMarker(ReactMarkerId markerId, const char *tag);
60+
[[deprecated("Use logMarker instead")]]
6061
extern RN_EXPORT void logMarkerBridgeless(ReactMarkerId markerId);
62+
[[deprecated("Use logTaggedMarker instead")]]
6163
extern RN_EXPORT void logTaggedMarkerBridgeless(ReactMarkerId markerId, const char *tag);
6264

6365
struct ReactMarkerEvent {

‎packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp‎

Lines changed: 57 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -235,62 +235,63 @@ void ReactInstance::loadScript(
235235
std::shared_ptr<const jsi::Buffer> buffer(std::move(script));
236236
std::string scriptName = simpleBasename(sourceURL);
237237

238-
runtimeScheduler_->scheduleWork([jsErrorHandler = jsErrorHandler_,
239-
scriptName,
240-
sourceURL,
241-
buffer = std::move(buffer),
242-
weakBufferedRuntimeExecuter =
243-
std::weak_ptr<BufferedRuntimeExecutor>(
244-
bufferedRuntimeExecutor_),
245-
beforeLoad,
246-
afterLoad](jsi::Runtime& runtime) mutable {
247-
if (beforeLoad) {
248-
beforeLoad(runtime);
249-
}
250-
TraceSection s("ReactInstance::loadScript");
251-
bool hasLogger(ReactMarker::logTaggedMarkerBridgelessImpl != nullptr);
252-
if (hasLogger) {
253-
ReactMarker::logTaggedMarkerBridgeless(
254-
ReactMarker::RUN_JS_BUNDLE_START, scriptName.c_str());
255-
ReactMarker::logMarkerBridgeless(ReactMarker::INIT_REACT_RUNTIME_START);
256-
ReactMarker::logMarkerBridgeless(ReactMarker::APP_STARTUP_START);
257-
}
238+
runtimeScheduler_->scheduleWork(
239+
[jsErrorHandler = jsErrorHandler_,
240+
scriptName,
241+
sourceURL,
242+
buffer = std::move(buffer),
243+
weakBufferedRuntimeExecuter =
244+
std::weak_ptr<BufferedRuntimeExecutor>(bufferedRuntimeExecutor_),
245+
beforeLoad,
246+
afterLoad](jsi::Runtime& runtime) mutable {
247+
if (beforeLoad) {
248+
beforeLoad(runtime);
249+
}
250+
TraceSection s("ReactInstance::loadScript");
251+
bool hasLogger(ReactMarker::logTaggedMarkerImpl != nullptr);
252+
if (hasLogger) {
253+
ReactMarker::logTaggedMarker(
254+
ReactMarker::RUN_JS_BUNDLE_START, scriptName.c_str());
255+
ReactMarker::logMarker(ReactMarker::INIT_REACT_RUNTIME_START);
256+
ReactMarker::logMarker(ReactMarker::APP_STARTUP_START);
257+
}
258258

259-
// Check if the shermes unit is avaliable.
260-
auto* shUnitAPI = jsi::castInterface<hermes::IHermesSHUnit>(&runtime);
261-
auto* shUnitCreator = shUnitAPI ? shUnitAPI->getSHUnitCreator() : nullptr;
262-
if (shUnitCreator) {
263-
LOG(WARNING) << "ReactInstance: evaluateSHUnit";
264-
auto* hermesAPI = jsi::castInterface<hermes::IHermes>(&runtime);
265-
hermesAPI->evaluateSHUnit(shUnitCreator);
266-
} else {
267-
LOG(WARNING) << "ReactInstance: evaluateJavaScript() with JS bundle";
268-
runtime.evaluateJavaScript(buffer, sourceURL);
269-
}
259+
// Check if the shermes unit is avaliable.
260+
auto* shUnitAPI = jsi::castInterface<hermes::IHermesSHUnit>(&runtime);
261+
auto* shUnitCreator =
262+
shUnitAPI ? shUnitAPI->getSHUnitCreator() : nullptr;
263+
if (shUnitCreator) {
264+
LOG(WARNING) << "ReactInstance: evaluateSHUnit";
265+
auto* hermesAPI = jsi::castInterface<hermes::IHermes>(&runtime);
266+
hermesAPI->evaluateSHUnit(shUnitCreator);
267+
} else {
268+
LOG(WARNING) << "ReactInstance: evaluateJavaScript() with JS bundle";
269+
runtime.evaluateJavaScript(buffer, sourceURL);
270+
}
270271

271-
/**
272-
* TODO(T183610671): We need a safe/reliable way to enable the js
273-
* pipeline from javascript. Remove this after we figure that out, or
274-
* after we just remove the js pipeline.
275-
*/
276-
if (!jsErrorHandler->hasHandledFatalError()) {
277-
jsErrorHandler->setRuntimeReady();
278-
}
272+
/**
273+
* TODO(T183610671): We need a safe/reliable way to enable the js
274+
* pipeline from javascript. Remove this after we figure that out, or
275+
* after we just remove the js pipeline.
276+
*/
277+
if (!jsErrorHandler->hasHandledFatalError()) {
278+
jsErrorHandler->setRuntimeReady();
279+
}
279280

280-
if (hasLogger) {
281-
ReactMarker::logTaggedMarkerBridgeless(
282-
ReactMarker::RUN_JS_BUNDLE_STOP, scriptName.c_str());
283-
ReactMarker::logMarkerBridgeless(ReactMarker::INIT_REACT_RUNTIME_STOP);
284-
ReactMarker::logMarkerBridgeless(ReactMarker::APP_STARTUP_STOP);
285-
}
286-
if (auto strongBufferedRuntimeExecuter =
287-
weakBufferedRuntimeExecuter.lock()) {
288-
strongBufferedRuntimeExecuter->flush();
289-
}
290-
if (afterLoad) {
291-
afterLoad(runtime);
292-
}
293-
});
281+
if (hasLogger) {
282+
ReactMarker::logTaggedMarker(
283+
ReactMarker::RUN_JS_BUNDLE_STOP, scriptName.c_str());
284+
ReactMarker::logMarker(ReactMarker::INIT_REACT_RUNTIME_STOP);
285+
ReactMarker::logMarker(ReactMarker::APP_STARTUP_STOP);
286+
}
287+
if (auto strongBufferedRuntimeExecuter =
288+
weakBufferedRuntimeExecuter.lock()) {
289+
strongBufferedRuntimeExecuter->flush();
290+
}
291+
if (afterLoad) {
292+
afterLoad(runtime);
293+
}
294+
});
294295
}
295296

296297
/*
@@ -369,9 +370,9 @@ void ReactInstance::registerSegment(
369370
"Empty segment registered with ID " + tag + " from " + segmentPath);
370371
}
371372

372-
bool hasLogger(ReactMarker::logTaggedMarkerBridgelessImpl != nullptr);
373+
bool hasLogger(ReactMarker::logTaggedMarkerImpl != nullptr);
373374
if (hasLogger) {
374-
ReactMarker::logTaggedMarkerBridgeless(
375+
ReactMarker::logTaggedMarker(
375376
ReactMarker::REGISTER_JS_SEGMENT_START, tag.c_str());
376377
}
377378
LOG(WARNING) << "Starting to evaluate segment " << segmentId
@@ -381,7 +382,7 @@ void ReactInstance::registerSegment(
381382
LOG(WARNING) << "Finished evaluating segment " << segmentId
382383
<< " in ReactInstance::registerSegment";
383384
if (hasLogger) {
384-
ReactMarker::logTaggedMarkerBridgeless(
385+
ReactMarker::logTaggedMarker(
385386
ReactMarker::REGISTER_JS_SEGMENT_STOP, tag.c_str());
386387
}
387388
});

‎packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTPerformanceLoggerUtils.mm‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,10 @@ static void mapReactMarkerToPerformanceLogger(
6767
void registerPerformanceLoggerHooks(RCTPerformanceLogger *performanceLogger)
6868
{
6969
__weak RCTPerformanceLogger *weakPerformanceLogger = performanceLogger;
70-
ReactMarker::logTaggedMarkerBridgelessImpl = [weakPerformanceLogger](
71-
const ReactMarker::ReactMarkerId markerId, const char *tag) {
70+
std::unique_lock lock(ReactMarker::logTaggedMarkerImplMutex);
71+
ReactMarker::logTaggedMarkerImpl = [weakPerformanceLogger](
72+
const ReactMarker::ReactMarkerId markerId, const char *tag) {
73+
(void)tag;
7274
mapReactMarkerToPerformanceLogger(markerId, weakPerformanceLogger);
7375
};
7476
}

0 commit comments

Comments
 (0)