Skip to content

Commit feec8d0

Browse files
cortinicofacebook-github-bot
authored andcommitted
Hide JS FPS on performance overlay as not accurate (facebook#52000)
Summary: Pull Request resolved: facebook#52000 Fixes facebook#50338 The current JS FPS value is incorrect because the frame skipping logic hasn't been reimplemented in Fabric. As we're looking into moving this into the performance panel, I've discussed with huntie and agreed we'll just remove the value for now to don't show inaccurate informations. Changelog: [Android] [Changed] - Hide JS FPS on performance overlay as not accurate Reviewed By: huntie Differential Revision: D76590909 fbshipit-source-id: 90b0d9c84f9aefa9197243ebb57f4e86107d6c01
1 parent 05521ad commit feec8d0

File tree

2 files changed

+28
-7
lines changed

2 files changed

+28
-7
lines changed

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/FpsView.kt

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ internal class FpsView(reactContext: ReactContext?) : FrameLayout(reactContext!!
3333
textView = findViewById<View>(R.id.fps_text) as TextView
3434
frameCallback = FpsDebugFrameCallback(reactContext!!)
3535
fpsMonitorRunnable = FPSMonitorRunnable()
36-
setCurrentFPS(0.0, 0.0, 0, 0)
36+
setCurrentFPS(0.0, 0.0, 0, 0, frameCallback.isRunningOnFabric)
3737
}
3838

3939
override fun onAttachedToWindow() {
@@ -53,16 +53,21 @@ internal class FpsView(reactContext: ReactContext?) : FrameLayout(reactContext!!
5353
currentFPS: Double,
5454
currentJSFPS: Double,
5555
droppedUIFrames: Int,
56-
total4PlusFrameStutters: Int
56+
total4PlusFrameStutters: Int,
57+
runningOnFabric: Boolean
5758
) {
58-
val fpsString =
59+
var fpsString =
5960
String.format(
6061
Locale.US,
61-
"UI: %.1f fps\n%d dropped so far\n%d stutters (4+) so far\nJS: %.1f fps",
62+
"UI: %.1f fps\n%d dropped so far\n%d stutters (4+) so far",
6263
currentFPS,
6364
droppedUIFrames,
64-
total4PlusFrameStutters,
65-
currentJSFPS)
65+
total4PlusFrameStutters)
66+
if (!runningOnFabric) {
67+
// The JS FPS is only relevant for the legacy architecture, as Fabric we don't use
68+
// BridgeIdleDebugListener to track JS frame drops.
69+
fpsString += String.format(Locale.US, "\nJS: %.1f fps", currentJSFPS)
70+
}
6671
textView.text = fpsString
6772
FLog.d(ReactConstants.TAG, fpsString)
6873
}
@@ -80,7 +85,11 @@ internal class FpsView(reactContext: ReactContext?) : FrameLayout(reactContext!!
8085
totalFramesDropped += frameCallback.expectedNumFrames - frameCallback.numFrames
8186
total4PlusFrameStutters += frameCallback.get4PlusFrameStutters()
8287
setCurrentFPS(
83-
frameCallback.fps, frameCallback.jsFPS, totalFramesDropped, total4PlusFrameStutters)
88+
frameCallback.fps,
89+
frameCallback.jsFPS,
90+
totalFramesDropped,
91+
total4PlusFrameStutters,
92+
frameCallback.isRunningOnFabric)
8493
frameCallback.reset()
8594
postDelayed(this, UPDATE_INTERVAL_MS.toLong())
8695
}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/debug/FpsDebugFrameCallback.kt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,11 @@ internal class FpsDebugFrameCallback(private val reactContext: ReactContext) :
6565
val uiManagerModule = reactContext.getNativeModule(UIManagerModule::class.java)
6666
if (!reactContext.isBridgeless) {
6767
reactContext.catalystInstance.addBridgeIdleDebugListener(didJSUpdateUiDuringFrameDetector)
68+
isRunningOnFabric = false
69+
} else {
70+
// T172641976 Consider either implementing a mechanism similar to addBridgeIdleDebugListener
71+
// for Fabric or point users to use RNDT.
72+
isRunningOnFabric = true
6873
}
6974
uiManagerModule?.setViewHierarchyUpdateDebugListener(didJSUpdateUiDuringFrameDetector)
7075
}
@@ -97,6 +102,10 @@ internal class FpsDebugFrameCallback(private val reactContext: ReactContext) :
97102
0.0
98103
} else numFrames.toDouble() * 1e9 / (lastFrameTime - firstFrameTime)
99104

105+
/**
106+
* Please note that this value is not relevant if running on Fabric. That's because we don't
107+
* implement addBridgeIdleDebugListener on Fabric.
108+
*/
100109
val jsFPS: Double
101110
get() =
102111
if (lastFrameTime == firstFrameTime) {
@@ -115,6 +124,9 @@ internal class FpsDebugFrameCallback(private val reactContext: ReactContext) :
115124
return (targetFps * totalTimeMS / 1000 + 1).toInt()
116125
}
117126

127+
var isRunningOnFabric = true
128+
private set
129+
118130
fun get4PlusFrameStutters(): Int = fourPlusFrameStutters
119131

120132
private val totalTimeMS: Int

0 commit comments

Comments
 (0)