Skip to content

Commit

Permalink
Avoid calling Dart_HintFreed more than once every five seconds (flutt…
Browse files Browse the repository at this point in the history
  • Loading branch information
dnfield committed May 3, 2021
1 parent aa2672f commit d7117dd
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 5 deletions.
2 changes: 1 addition & 1 deletion runtime/runtime_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ class RuntimeController : public PlatformConfigurationClient {
///
/// @return If the idle notification was forwarded to the running isolate.
///
bool NotifyIdle(int64_t deadline, size_t freed_hint);
virtual bool NotifyIdle(int64_t deadline, size_t freed_hint);

//----------------------------------------------------------------------------
/// @brief Returns if the root isolate is running. The isolate must be
Expand Down
17 changes: 14 additions & 3 deletions shell/common/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -226,15 +226,26 @@ void Engine::ReportTimings(std::vector<int64_t> timings) {
}

void Engine::HintFreed(size_t size) {
hint_freed_bytes_since_last_idle_ += size;
hint_freed_bytes_since_last_call_ += size;
}

void Engine::NotifyIdle(int64_t deadline) {
auto trace_event = std::to_string(deadline - Dart_TimelineGetMicros());
TRACE_EVENT1("flutter", "Engine::NotifyIdle", "deadline_now_delta",
trace_event.c_str());
runtime_controller_->NotifyIdle(deadline, hint_freed_bytes_since_last_idle_);
hint_freed_bytes_since_last_idle_ = 0;
// Avoid asking the RuntimeController to call Dart_HintFreed more than once
// every 5 seconds.
// This is to avoid GCs happening too frequently e.g. when an animated GIF is
// playing and disposing of an image every frame.
fml::TimePoint now = delegate_.GetCurrentTimePoint();
fml::TimeDelta delta = now - last_hint_freed_call_time_;
size_t hint_freed_bytes = 0;
if (delta.ToMilliseconds() > 5000 && hint_freed_bytes_since_last_call_ > 0) {
hint_freed_bytes = hint_freed_bytes_since_last_call_;
hint_freed_bytes_since_last_call_ = 0;
last_hint_freed_call_time_ = now;
}
runtime_controller_->NotifyIdle(deadline, hint_freed_bytes);
}

std::optional<uint32_t> Engine::GetUIIsolateReturnCode() {
Expand Down
9 changes: 8 additions & 1 deletion shell/common/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,12 @@ class Engine final : public RuntimeDelegate,
/// library to load.
///
virtual void RequestDartDeferredLibrary(intptr_t loading_unit_id) = 0;

//--------------------------------------------------------------------------
/// @brief Returns the current fml::TimePoint.
/// This method is primarily provided to allow tests to control
/// Any methods that rely on advancing the clock.
virtual fml::TimePoint GetCurrentTimePoint() = 0;
};

//----------------------------------------------------------------------------
Expand Down Expand Up @@ -916,7 +922,8 @@ class Engine final : public RuntimeDelegate,
std::shared_ptr<FontCollection> font_collection_;
ImageDecoder image_decoder_;
TaskRunners task_runners_;
size_t hint_freed_bytes_since_last_idle_ = 0;
size_t hint_freed_bytes_since_last_call_ = 0;
fml::TimePoint last_hint_freed_call_time_;
fml::WeakPtrFactory<Engine> weak_factory_;

// |RuntimeDelegate|
Expand Down
50 changes: 50 additions & 0 deletions shell/common/engine_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ namespace flutter {

namespace {
class MockDelegate : public Engine::Delegate {
public:
MOCK_METHOD2(OnEngineUpdateSemantics,
void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates));
MOCK_METHOD1(OnEngineHandlePlatformMessage,
Expand All @@ -34,6 +35,7 @@ class MockDelegate : public Engine::Delegate {
std::unique_ptr<std::vector<std::string>>(
const std::vector<std::string>&));
MOCK_METHOD1(RequestDartDeferredLibrary, void(intptr_t));
MOCK_METHOD0(GetCurrentTimePoint, fml::TimePoint());
};

class MockResponse : public PlatformMessageResponse {
Expand Down Expand Up @@ -69,6 +71,7 @@ class MockRuntimeController : public RuntimeController {
MOCK_METHOD3(LoadDartDeferredLibraryError,
void(intptr_t, const std::string, bool));
MOCK_CONST_METHOD0(GetDartVM, DartVM*());
MOCK_METHOD2(NotifyIdle, bool(int64_t, size_t));
};

fml::RefPtr<PlatformMessage> MakePlatformMessage(
Expand Down Expand Up @@ -299,4 +302,51 @@ TEST_F(EngineTest, PassesLoadDartDeferredLibraryErrorToRuntime) {
});
}

TEST_F(EngineTest, NotifyIdleMoreThanOncePerFiveSeconds) {
PostUITaskSync([this] {
MockRuntimeDelegate client;
auto mock_runtime_controller =
std::make_unique<MockRuntimeController>(client, task_runners_);

// Verify we get an initial call with the bytes, since it's 10 seconds.
EXPECT_CALL(*mock_runtime_controller, NotifyIdle(200, 100)).Times(1);
// Verify that we do not get called again, since it's only been 3 more
// seconds.
EXPECT_CALL(*mock_runtime_controller, NotifyIdle(400, 0)).Times(1);
// Verify that we get called again, since it's now been 6 seconds.
EXPECT_CALL(*mock_runtime_controller, NotifyIdle(600, 300 + 500)).Times(1);
// Set up mocking of the timepoint logic. Calls at 10, 13, and 16 seconds.
EXPECT_CALL(delegate_, GetCurrentTimePoint())
.Times(3)
.WillOnce(::testing::Return(
fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromSeconds(10))))
.WillOnce(::testing::Return(
fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromSeconds(13))))
.WillOnce(::testing::Return(
fml::TimePoint::FromEpochDelta(fml::TimeDelta::FromSeconds(16))));

auto engine = std::make_unique<Engine>(
/*delegate=*/delegate_,
/*dispatcher_maker=*/dispatcher_maker_,
/*image_decoder_task_runner=*/image_decoder_task_runner_,
/*task_runners=*/task_runners_,
/*settings=*/settings_,
/*animator=*/std::move(animator_),
/*io_manager=*/io_manager_,
/*font_collection=*/std::make_shared<FontCollection>(),
/*runtime_controller=*/std::move(mock_runtime_controller));

// Verifications are above, since we std::moved the unique_ptr for the
// mock.
engine->HintFreed(100);
engine->NotifyIdle(200);

engine->HintFreed(300);
engine->NotifyIdle(400);

engine->HintFreed(500);
engine->NotifyIdle(600);
});
}

} // namespace flutter
4 changes: 4 additions & 0 deletions shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1846,4 +1846,8 @@ void Shell::OnDisplayUpdates(DisplayUpdateType update_type,
display_manager_->HandleDisplayUpdates(update_type, displays);
}

fml::TimePoint Shell::GetCurrentTimePoint() {
return fml::TimePoint::Now();
}

} // namespace flutter
3 changes: 3 additions & 0 deletions shell/common/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,9 @@ class Shell final : public PlatformView::Delegate,
// |Engine::Delegate|
void RequestDartDeferredLibrary(intptr_t loading_unit_id) override;

// |Engine::Delegate|
fml::TimePoint GetCurrentTimePoint() override;

// |Rasterizer::Delegate|
void OnFrameRasterized(const FrameTiming&) override;

Expand Down

0 comments on commit d7117dd

Please sign in to comment.