Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Do not call Dart_NotifyIdle when in Dart_PerformanceMode_Latency #37499

Merged
merged 1 commit into from
Nov 11, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion lib/ui/window/platform_configuration.cc
Original file line number Diff line number Diff line change
Expand Up @@ -371,9 +371,17 @@ void PlatformConfigurationNativeApi::SetIsolateDebugName(
UIDartState::Current()->SetDebugName(name);
}

Dart_PerformanceMode PlatformConfigurationNativeApi::current_performace_mode_ =
Dart_PerformanceMode_Default;

Dart_PerformanceMode PlatformConfigurationNativeApi::GetDartPerformanceMode() {
return current_performace_mode_;
}

int PlatformConfigurationNativeApi::RequestDartPerformanceMode(int mode) {
UIDartState::ThrowIfUIOperationsProhibited();
return Dart_SetPerformanceMode(static_cast<Dart_PerformanceMode>(mode));
current_performace_mode_ = static_cast<Dart_PerformanceMode>(mode);
return Dart_SetPerformanceMode(current_performace_mode_);
}

Dart_Handle PlatformConfigurationNativeApi::GetPersistentIsolateData() {
Expand Down
9 changes: 9 additions & 0 deletions lib/ui/window/platform_configuration.h
Original file line number Diff line number Diff line change
Expand Up @@ -515,9 +515,18 @@ class PlatformConfigurationNativeApi {
///
static int RequestDartPerformanceMode(int mode);

//--------------------------------------------------------------------------
/// @brief Returns the current performance mode of the Dart VM. Defaults
/// to `Dart_PerformanceMode_Default` if no prior requests to change the
/// performance mode have been made.
static Dart_PerformanceMode GetDartPerformanceMode();

static int64_t GetRootIsolateToken();

static void RegisterBackgroundIsolate(int64_t root_isolate_token);

private:
static Dart_PerformanceMode current_performace_mode_;
};

} // namespace flutter
Expand Down
6 changes: 6 additions & 0 deletions runtime/runtime_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,12 @@ bool RuntimeController::NotifyIdle(fml::TimePoint deadline) {

tonic::DartState::Scope scope(root_isolate);

Dart_PerformanceMode performance_mode =
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be better if we could cut this off in the animator instead, and avoid even scheduling the delayed task during this time.

However, I'm also looking into refactoring that logic a bit right now anyway, since I think it's doing duplicate work in some cases.

PlatformConfigurationNativeApi::GetDartPerformanceMode();
if (performance_mode == Dart_PerformanceMode::Dart_PerformanceMode_Latency) {
return false;
}

Dart_NotifyIdle(deadline.ToEpochDelta().ToMicroseconds());

// Idle notifications being in isolate scope are part of the contract.
Expand Down
8 changes: 8 additions & 0 deletions shell/common/fixtures/shell_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,14 @@ void canAccessIsolateLaunchData() {
);
}

@pragma('vm:entry-point')
void performanceModeImpactsNotifyIdle() {
notifyNativeBool(false);
PlatformDispatcher.instance.requestDartPerformanceMode(DartPerformanceMode.latency);
notifyNativeBool(true);
PlatformDispatcher.instance.requestDartPerformanceMode(DartPerformanceMode.balanced);
}

@pragma('vm:external-name', 'NotifyMessage')
external void notifyMessage(String string);

Expand Down
41 changes: 41 additions & 0 deletions shell/common/shell_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3891,6 +3891,47 @@ TEST_F(ShellTest, PluginUtilitiesCallbackHandleErrorHandling) {
DestroyShell(std::move(shell));
}

TEST_F(ShellTest, NotifyIdleNotCalledInLatencyMode) {
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
Settings settings = CreateSettingsForFixture();
ThreadHost thread_host("io.flutter.test." + GetCurrentTestName() + ".",
ThreadHost::Type::Platform | ThreadHost::UI |
ThreadHost::IO | ThreadHost::RASTER);
auto platform_task_runner = thread_host.platform_thread->GetTaskRunner();
TaskRunners task_runners("test", thread_host.platform_thread->GetTaskRunner(),
thread_host.raster_thread->GetTaskRunner(),
thread_host.ui_thread->GetTaskRunner(),
thread_host.io_thread->GetTaskRunner());
auto shell = CreateShell(settings, task_runners);
ASSERT_TRUE(DartVMRef::IsInstanceRunning());
ASSERT_TRUE(ValidateShell(shell.get()));

// we start off in balanced mode, where we expect idle notifications to
// succeed. After the first `NotifyNativeBool` we expect to be in latency
// mode, where we expect idle notifications to fail.
fml::CountDownLatch latch(2);
AddNativeCallback(
"NotifyNativeBool", CREATE_NATIVE_ENTRY([&](auto args) {
Dart_Handle exception = nullptr;
bool is_in_latency_mode =
tonic::DartConverter<bool>::FromArguments(args, 0, exception);
auto runtime_controller = const_cast<RuntimeController*>(
shell->GetEngine()->GetRuntimeController());
bool success = runtime_controller->NotifyIdle(fml::TimePoint::Now());
EXPECT_EQ(success, !is_in_latency_mode);
latch.CountDown();
}));

auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("performanceModeImpactsNotifyIdle");
RunEngine(shell.get(), std::move(configuration));

latch.Wait();

DestroyShell(std::move(shell), task_runners);
ASSERT_FALSE(DartVMRef::IsInstanceRunning());
}

} // namespace testing
} // namespace flutter

Expand Down