[web] Fix race condition in widget benchmarks (#53952)
diff --git a/dev/benchmarks/macrobenchmarks/lib/src/web/recorder.dart b/dev/benchmarks/macrobenchmarks/lib/src/web/recorder.dart
index b21908b..12a5530 100644
--- a/dev/benchmarks/macrobenchmarks/lib/src/web/recorder.dart
+++ b/dev/benchmarks/macrobenchmarks/lib/src/web/recorder.dart
@@ -305,8 +305,7 @@
/// }
/// }
/// ```
-abstract class WidgetRecorder extends Recorder
- implements RecordingWidgetsBindingListener {
+abstract class WidgetRecorder extends Recorder implements FrameRecorder {
WidgetRecorder({@required String name}) : super._(name);
/// Creates a widget to be benchmarked.
@@ -318,9 +317,10 @@
Widget createWidget();
@override
- Profile profile;
+ VoidCallback didStop;
- final Completer<Profile> _profileCompleter = Completer<Profile>();
+ Profile profile;
+ Completer<void> _runCompleter;
Stopwatch _drawFrameStopwatch;
@@ -340,27 +340,32 @@
if (profile.shouldContinue()) {
window.scheduleFrame();
} else {
- _profileCompleter.complete(profile);
+ didStop();
+ _runCompleter.complete();
}
}
@override
void _onError(dynamic error, StackTrace stackTrace) {
- _profileCompleter.completeError(error, stackTrace);
+ _runCompleter.completeError(error, stackTrace);
}
@override
- Future<Profile> run() {
- profile = Profile(name: name);
+ Future<Profile> run() async {
+ _runCompleter = Completer<void>();
+ final Profile localProfile = profile = Profile(name: name);
final _RecordingWidgetsBinding binding =
_RecordingWidgetsBinding.ensureInitialized();
final Widget widget = createWidget();
binding._beginRecording(this, widget);
- _profileCompleter.future.whenComplete(() {
+ try {
+ await _runCompleter.future;
+ return localProfile;
+ } finally {
+ _runCompleter = null;
profile = null;
- });
- return _profileCompleter.future;
+ }
}
}
@@ -371,8 +376,7 @@
/// another frame that clears the screen. It repeats this process, measuring the
/// performance of frames that render the widget and ignoring the frames that
/// clear the screen.
-abstract class WidgetBuildRecorder extends Recorder
- implements RecordingWidgetsBindingListener {
+abstract class WidgetBuildRecorder extends Recorder implements FrameRecorder {
WidgetBuildRecorder({@required String name}) : super._(name);
/// Creates a widget to be benchmarked.
@@ -383,9 +387,10 @@
Widget createWidget();
@override
- Profile profile;
+ VoidCallback didStop;
- final Completer<Profile> _profileCompleter = Completer<Profile>();
+ Profile profile;
+ Completer<void> _runCompleter;
Stopwatch _drawFrameStopwatch;
@@ -427,26 +432,31 @@
showWidget = !showWidget;
_hostState._setStateTrampoline();
} else {
- _profileCompleter.complete(profile);
+ didStop();
+ _runCompleter.complete();
}
}
@override
void _onError(dynamic error, StackTrace stackTrace) {
- _profileCompleter.completeError(error, stackTrace);
+ _runCompleter.completeError(error, stackTrace);
}
@override
- Future<Profile> run() {
- profile = Profile(name: name);
+ Future<Profile> run() async {
+ _runCompleter = Completer<void>();
+ final Profile localProfile = profile = Profile(name: name);
final _RecordingWidgetsBinding binding =
_RecordingWidgetsBinding.ensureInitialized();
binding._beginRecording(this, _WidgetBuildRecorderHost(this));
- _profileCompleter.future.whenComplete(() {
+ try {
+ await _runCompleter.future;
+ return localProfile;
+ } finally {
+ _runCompleter = null;
profile = null;
- });
- return _profileCompleter.future;
+ }
}
}
@@ -698,9 +708,10 @@
/// Implemented by recorders that use [_RecordingWidgetsBinding] to receive
/// frame life-cycle calls.
-abstract class RecordingWidgetsBindingListener {
- /// The profile where the benchmark is collecting metrics.
- Profile profile;
+abstract class FrameRecorder {
+ /// Called by the recorder when it stops recording and doesn't need to collect
+ /// any more data.
+ set didStop(VoidCallback cb);
/// Called just before calling [SchedulerBinding.handleDrawFrame].
void frameWillDraw();
@@ -739,19 +750,31 @@
return WidgetsBinding.instance as _RecordingWidgetsBinding;
}
- RecordingWidgetsBindingListener _listener;
+ FrameRecorder _recorder;
bool _hasErrored = false;
- void _beginRecording(
- RecordingWidgetsBindingListener recorder, Widget widget) {
+ /// To short-circuit all frame lifecycle methods when the benchmark has
+ /// stopped collecting data.
+ bool _benchmarkStopped = false;
+
+ void _beginRecording(FrameRecorder recorder, Widget widget) {
+ if (_recorder != null) {
+ throw Exception(
+ 'Cannot call _RecordingWidgetsBinding._beginRecording more than once',
+ );
+ }
final FlutterExceptionHandler originalOnError = FlutterError.onError;
+ recorder.didStop = () {
+ _benchmarkStopped = true;
+ };
+
// Fail hard and fast on errors. Benchmarks should not have any errors.
FlutterError.onError = (FlutterErrorDetails details) {
_haltBenchmarkWithError(details.exception, details.stack);
originalOnError(details);
};
- _listener = recorder;
+ _recorder = recorder;
runApp(widget);
}
@@ -759,22 +782,17 @@
if (_hasErrored) {
return;
}
- _listener._onError(error, stackTrace);
+ _recorder._onError(error, stackTrace);
_hasErrored = true;
}
- /// To avoid calling [Profile.shouldContinue] every time [scheduleFrame] is
- /// called, we cache this value at the beginning of the frame.
- bool _benchmarkStopped = false;
-
@override
void handleBeginFrame(Duration rawTimeStamp) {
- // Don't keep on truckin' if there's an error.
- if (_hasErrored) {
+ // Don't keep on truckin' if there's an error or the benchmark has stopped.
+ if (_hasErrored || _benchmarkStopped) {
return;
}
try {
- _benchmarkStopped = !_listener.profile.shouldContinue();
super.handleBeginFrame(rawTimeStamp);
} catch (error, stackTrace) {
_haltBenchmarkWithError(error, stackTrace);
@@ -784,22 +802,23 @@
@override
void scheduleFrame() {
- // Don't keep on truckin' if there's an error.
- if (!_benchmarkStopped && !_hasErrored) {
- super.scheduleFrame();
+ // Don't keep on truckin' if there's an error or the benchmark has stopped.
+ if (_hasErrored || _benchmarkStopped) {
+ return;
}
+ super.scheduleFrame();
}
@override
void handleDrawFrame() {
- // Don't keep on truckin' if there's an error.
- if (_hasErrored) {
+ // Don't keep on truckin' if there's an error or the benchmark has stopped.
+ if (_hasErrored || _benchmarkStopped) {
return;
}
try {
- _listener.frameWillDraw();
+ _recorder.frameWillDraw();
super.handleDrawFrame();
- _listener.frameDidDraw();
+ _recorder.frameDidDraw();
} catch (error, stackTrace) {
_haltBenchmarkWithError(error, stackTrace);
rethrow;
