-
Notifications
You must be signed in to change notification settings - Fork 322
feat: expose functionality to start benchmarking manually #1757
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
base: main
Are you sure you want to change the base?
Conversation
c26b169
to
786465d
Compare
786465d
to
05d1da6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for manually triggering benchmarks at runtime and replaces internal recyclerlistview_unsafe
accesses with the public imperative methods on the FlashList instance.
- Expose
startBenchmark
function andisBenchmarkRunning
flag; introducestartManually
param to defer automatic start - Swap out
recyclerlistview_unsafe
forgetWindowSize
/getChildContainerDimensions
- Update fixture examples to use new hook APIs
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/benchmark/useFlatListBenchmark.ts | Add state, startBenchmark callback, manual start |
src/benchmark/useBenchmark.ts | Mirror manual-start API; swap internal RecyclerListView calls |
src/benchmark/useBlankAreaTracker.ts | Remove recyclerlistview_unsafe ; use public FlashList methods |
fixture/react-native/src/twitter/TwitterBenchmark.tsx | Switch from useFlatListBenchmark to useBenchmark |
fixture/react-native/src/ManualFlatListBenchmarkExample.tsx | Add manual FlatList benchmark example |
fixture/react-native/src/ManualBenchmarkExample.tsx | Add manual FlashList benchmark example |
Comments suppressed due to low confidence (2)
src/benchmark/useFlatListBenchmark.ts:1
- [nitpick] Add a JSDoc comment for the new
startManually
and returned{ startBenchmark, isBenchmarkRunning }
API so that users understand its purpose and return shape.
import { useCallback, useEffect, useRef, useState } from "react";
src/benchmark/useFlatListBenchmark.ts:47
- The function
runScrollBenchmark
is invoked but not imported in this file. Addimport { runScrollBenchmark } from './AutoScrollHelper';
or from the correct module to avoid a runtime ReferenceError.
await runScrollBenchmark(
|
||
setIsBenchmarkRunning(true); | ||
|
||
const runBenchmark = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Wrap the body of runBenchmark
in a try/finally block so that setIsBenchmarkRunning(false)
always runs even if an exception is thrown, ensuring the hook state is correctly reset.
Copilot uses AI. Check for mistakes.
@@ -89,14 +107,38 @@ export function useBenchmark( | |||
result.formattedString = getFormattedString(result); | |||
} | |||
callback(result); | |||
setIsBenchmarkRunning(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider moving this call into a finally
block inside runBenchmark
to guarantee that benchmarking state is reset even if callback
or metrics gathering throws an error.
Copilot uses AI. Check for mistakes.
05d1da6
to
ff2b2cb
Compare
Description
useBenchmark
anduseFlatListBenchmark
to be started manually. This supports runtime benchmarking - an in-app button or other input can be used to programmatically start a benchmark.recyclerlistview_unsafe
property.Reviewers’ hat-rack 🎩
TBC
Screenshots or videos (if needed)
TBC