Skip to content
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

Request DartPerformanceMode.latency during transitions #110600

Merged
merged 15 commits into from
Sep 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
97 changes: 96 additions & 1 deletion packages/flutter/lib/src/scheduler/binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import 'dart:async';
import 'dart:collection';
import 'dart:developer' show Flow, Timeline, TimelineTask;
import 'dart:ui' show AppLifecycleState, FramePhase, FrameTiming, PlatformDispatcher, TimingsCallback;
import 'dart:ui' show AppLifecycleState, DartPerformanceMode, FramePhase, FrameTiming, PlatformDispatcher, TimingsCallback;

import 'package:collection/collection.dart' show HeapPriorityQueue, PriorityQueue;
import 'package:flutter/foundation.dart';
Expand Down Expand Up @@ -183,6 +183,34 @@ enum SchedulerPhase {
postFrameCallbacks,
}

/// This callback is invoked when a request for [DartPerformanceMode] is disposed.
///
/// See also:
///
/// * [PerformanceModeRequestHandle] for more information on the lifecycle of the handle.
typedef _PerformanceModeCleaupCallback = VoidCallback;
Comment on lines +186 to +191
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just delete this and use VoidCallback directly wherever _PerformanceModeCleaupCallback is currently used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's useful for the reader, makes it easy to understand that this type is called when the performance mode is resolved. Let me know if you feel strongly about it, and I can change it up in a follow-up PR.


/// An opaque handle that keeps a request for [DartPerformanceMode] active until
/// disposed.
///
/// To create a [PerformanceModeRequestHandle], use [SchedulerBinding.requestPerformanceMode].
/// The component that makes the request is responsible for disposing the handle.
class PerformanceModeRequestHandle {
PerformanceModeRequestHandle._(_PerformanceModeCleaupCallback this._cleanup);

_PerformanceModeCleaupCallback? _cleanup;

/// Call this method to signal to [SchedulerBinding] that a request for a [DartPerformanceMode]
/// is no longer needed.
///
/// This method must only be called once per object.
void dispose() {
assert(_cleanup != null);
_cleanup!();
_cleanup = null;
}
}

/// Scheduler for running the following:
///
/// * _Transient callbacks_, triggered by the system's
Expand Down Expand Up @@ -605,6 +633,20 @@ mixin SchedulerBinding on BindingBase {
return true;
}

/// Asserts that there are no pending performance mode requests in debug mode.
///
/// Throws a [FlutterError] if there are pending performance mode requests,
/// as this indicates a potential memory leak.
bool debugAssertNoPendingPerformanceModeRequests(String reason) {
assert(() {
if (_performanceMode != null) {
throw FlutterError(reason);
}
return true;
}());
return true;
}

/// Prints the stack for where the current transient callback was registered.
///
/// A transient frame callback is one that was registered with
Expand Down Expand Up @@ -1085,6 +1127,59 @@ mixin SchedulerBinding on BindingBase {
}
}

DartPerformanceMode? _performanceMode;
int _numPerformanceModeRequests = 0;

/// Request a specific [DartPerformanceMode].
///
/// Returns `null` if the request was not successful due to conflicting performance mode requests.
/// Two requests are said to be in conflict if they are not of the same [DartPerformanceMode] type,
/// and an explicit request for a performance mode has been made prior.
///
/// Requestor is responsible for calling [PerformanceModeRequestHandle.dispose] when it no longer
/// requires the performance mode.
PerformanceModeRequestHandle? requestPerformanceMode(DartPerformanceMode mode) {
// conflicting requests are not allowed.
if (_performanceMode != null && _performanceMode != mode) {
return null;
}

if (_performanceMode == mode) {
assert(_numPerformanceModeRequests > 0);
_numPerformanceModeRequests++;
iskakaushik marked this conversation as resolved.
Show resolved Hide resolved
} else if (_performanceMode == null) {
assert(_numPerformanceModeRequests == 0);
_performanceMode = mode;
iskakaushik marked this conversation as resolved.
Show resolved Hide resolved
_numPerformanceModeRequests = 1;
}

return PerformanceModeRequestHandle._(_disposePerformanceModeRequest);
}

/// Remove a request for a specific [DartPerformanceMode].
///
/// If all the pending requests have been disposed, the engine will revert to the
/// [DartPerformanceMode.balanced] performance mode.
void _disposePerformanceModeRequest() {
_numPerformanceModeRequests--;
if (_numPerformanceModeRequests == 0) {
_performanceMode = null;
PlatformDispatcher.instance.requestDartPerformanceMode(DartPerformanceMode.balanced);
}
}

/// Returns the current [DartPerformanceMode] requested or `null` if no requests have
/// been made.
///
/// This is only supported in debug and profile modes, returns `null` in release mode.
DartPerformanceMode? debugGetRequestedPerformanceMode() {
if (!(kDebugMode || kProfileMode)) {
return null;
} else {
return _performanceMode;
}
}

/// Called by the engine to produce a new frame.
///
/// This method is called immediately after [handleBeginFrame]. It calls all
Expand Down
17 changes: 17 additions & 0 deletions packages/flutter/lib/src/widgets/routes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,14 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
Future<T?> get completed => _transitionCompleter.future;
final Completer<T?> _transitionCompleter = Completer<T?>();

/// Handle to the performance mode request.
///
/// When the route is animating, the performance mode is requested. It is then
/// disposed when the animation ends. Requesting [DartPerformanceMode.latency]
/// indicates to the engine that the transition is latency sensitive and to delay
/// non-essential work while this handle is active.
PerformanceModeRequestHandle? _performanceModeRequestHandle;

/// {@template flutter.widgets.TransitionRoute.transitionDuration}
/// The duration the transition going forwards.
///
Expand Down Expand Up @@ -221,12 +229,17 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
if (overlayEntries.isNotEmpty) {
overlayEntries.first.opaque = opaque;
}
_performanceModeRequestHandle?.dispose();
_performanceModeRequestHandle = null;
break;
case AnimationStatus.forward:
case AnimationStatus.reverse:
if (overlayEntries.isNotEmpty) {
overlayEntries.first.opaque = false;
}
_performanceModeRequestHandle ??=
SchedulerBinding.instance
.requestPerformanceMode(ui.DartPerformanceMode.latency);
break;
case AnimationStatus.dismissed:
// We might still be an active route if a subclass is controlling the
Expand All @@ -236,6 +249,8 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
if (!isActive) {
navigator!.finalizeRoute(this);
_popFinalized = true;
_performanceModeRequestHandle?.dispose();
_performanceModeRequestHandle = null;
}
break;
}
Expand Down Expand Up @@ -465,6 +480,8 @@ abstract class TransitionRoute<T> extends OverlayRoute<T> {
void dispose() {
assert(!_transitionCompleter.isCompleted, 'Cannot dispose a $runtimeType twice.');
_animation?.removeStatusListener(_handleStatusChanged);
_performanceModeRequestHandle?.dispose();
_performanceModeRequestHandle = null;
if (willDisposeAnimationController) {
_controller?.dispose();
}
Expand Down
23 changes: 23 additions & 0 deletions packages/flutter/test/cupertino/nav_bar_transition_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
// Fails with "flutter test --test-randomize-ordering-seed=456"
@Tags(<String>['no-shuffle'])

import 'dart:ui';

import 'package:flutter/cupertino.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/scheduler.dart';
import 'package:flutter_test/flutter_test.dart';

Future<void> startTransitionBetween(
Expand Down Expand Up @@ -443,6 +446,26 @@ void main() {
);
});

testWidgets('DartPerformanceMode is latency mid-animation', (WidgetTester tester) async {
DartPerformanceMode? mode;

// before the animation starts, no requests are active.
mode = SchedulerBinding.instance.debugGetRequestedPerformanceMode();
expect(mode, isNull);

await startTransitionBetween(tester, fromTitle: 'Page 1');

// mid-transition, latency mode is expected.
await tester.pump(const Duration(milliseconds: 50));
mode = SchedulerBinding.instance.debugGetRequestedPerformanceMode();
expect(mode, equals(DartPerformanceMode.latency));

// end of transitio, go back to no requests active.
await tester.pump(const Duration(milliseconds: 500));
mode = SchedulerBinding.instance.debugGetRequestedPerformanceMode();
expect(mode, isNull);
});

testWidgets('Multiple nav bars tags do not conflict if in different navigators', (WidgetTester tester) async {
await tester.pumpWidget(
CupertinoApp(
Expand Down
56 changes: 56 additions & 0 deletions packages/flutter/test/scheduler/performance_mode_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:ui';
iskakaushik marked this conversation as resolved.
Show resolved Hide resolved

import 'package:flutter/scheduler.dart';
import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
late SchedulerBinding binding;

setUpAll(() {
WidgetsFlutterBinding.ensureInitialized();
binding = SchedulerBinding.instance;
});

test('PerformanceModeHandler make one request', () async {
final PerformanceModeRequestHandle? requestHandle = binding.requestPerformanceMode(DartPerformanceMode.latency);
expect(requestHandle, isNotNull);
expect(binding.debugGetRequestedPerformanceMode(), equals(DartPerformanceMode.latency));
requestHandle?.dispose();
expect(binding.debugGetRequestedPerformanceMode(), isNull);
});

test('PerformanceModeHandler make conflicting requests', () async {
final PerformanceModeRequestHandle? requestHandle1 = binding.requestPerformanceMode(DartPerformanceMode.latency);
expect(requestHandle1, isNotNull);

final PerformanceModeRequestHandle? requestHandle2 = binding.requestPerformanceMode(DartPerformanceMode.throughput);
expect(requestHandle2, isNull);

expect(binding.debugGetRequestedPerformanceMode(), equals(DartPerformanceMode.latency));

requestHandle1?.dispose();
expect(binding.debugGetRequestedPerformanceMode(), isNull);
});

test('PerformanceModeHandler revert only after last requestor disposed',
() async {
final PerformanceModeRequestHandle? requestHandle1 = binding.requestPerformanceMode(DartPerformanceMode.latency);
expect(requestHandle1, isNotNull);

expect(binding.debugGetRequestedPerformanceMode(), equals(DartPerformanceMode.latency));

final PerformanceModeRequestHandle? requestHandle2 = binding.requestPerformanceMode(DartPerformanceMode.latency);
expect(requestHandle2, isNotNull);

expect(binding.debugGetRequestedPerformanceMode(), equals(DartPerformanceMode.latency));
requestHandle1?.dispose();
expect(binding.debugGetRequestedPerformanceMode(), equals(DartPerformanceMode.latency));
requestHandle2?.dispose();
expect(binding.debugGetRequestedPerformanceMode(), isNull);
});
}
iskakaushik marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions packages/flutter_test/lib/src/binding.dart
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,9 @@ abstract class TestWidgetsFlutterBinding extends BindingBase
assert(debugAssertNoTransientCallbacks(
'An animation is still running even after the widget tree was disposed.'
));
assert(debugAssertNoPendingPerformanceModeRequests(
'A performance mode was requested and not disposed by a test.'
));
assert(debugAssertAllFoundationVarsUnset(
'The value of a foundation debug variable was changed by the test.',
debugPrintOverride: debugPrintOverride,
Expand Down
1 change: 1 addition & 0 deletions packages/integration_test/test/binding_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Future<void> main() async {
));
expect(tester.binding, binding);
binding.reportData = <String, dynamic>{'answer': 42};
await tester.pump();
});

testWidgets('hitTesting works when using setSurfaceSize', (WidgetTester tester) async {
Expand Down