Optmize path volatility tracker (flutter/engine#30299)

This commit is contained in:
Dan Field
2021-12-13 16:59:04 -08:00
committed by GitHub
parent 165a82c79b
commit 87dcc5293f
8 changed files with 55 additions and 79 deletions

View File

@@ -81,16 +81,10 @@ void CanvasPath::resetVolatility() {
mutable_path().setIsVolatile(true);
tracked_path_->frame_count = 0;
tracked_path_->tracking_volatility = true;
path_tracker_->Insert(tracked_path_);
path_tracker_->Track(tracked_path_);
}
}
void CanvasPath::ReleaseDartWrappableReference() const {
FML_DCHECK(path_tracker_);
path_tracker_->Erase(tracked_path_);
RefCountedDartWrappable::ReleaseDartWrappableReference();
}
int CanvasPath::getFillType() {
return static_cast<int>(path().getFillType());
}

View File

@@ -115,8 +115,6 @@ class CanvasPath : public RefCountedDartWrappable<CanvasPath> {
static void RegisterNatives(tonic::DartLibraryNatives* natives);
virtual void ReleaseDartWrappableReference() const override;
private:
CanvasPath();

View File

@@ -67,6 +67,7 @@ TEST_F(ShellTest, PathVolatilityOldPathsBecomeNonVolatile) {
}
TEST_F(ShellTest, PathVolatilityGCRemovesPathFromTracker) {
static_assert(VolatilePathTracker::kFramesOfVolatility > 1);
auto message_latch = std::make_shared<fml::AutoResetWaitableEvent>();
auto native_validate_path = [message_latch](Dart_NativeArguments args) {
@@ -77,26 +78,23 @@ TEST_F(ShellTest, PathVolatilityGCRemovesPathFromTracker) {
EXPECT_FALSE(Dart_IsError(result));
CanvasPath* path = reinterpret_cast<CanvasPath*>(peer);
EXPECT_TRUE(path);
path->AddRef();
EXPECT_TRUE(path->path().isVolatile());
std::shared_ptr<VolatilePathTracker> tracker =
UIDartState::Current()->GetVolatilePathTracker();
EXPECT_TRUE(tracker);
static_assert(VolatilePathTracker::kFramesOfVolatility > 1);
EXPECT_EQ(GetLiveTrackedPathCount(tracker), 1ul);
EXPECT_TRUE(path->path().isVolatile());
tracker->OnFrame();
EXPECT_EQ(GetLiveTrackedPathCount(tracker), 1ul);
EXPECT_TRUE(path->path().isVolatile());
// simulate GC
path->ReleaseDartWrappableReference();
path->Release();
EXPECT_EQ(GetLiveTrackedPathCount(tracker), 0ul);
tracker->OnFrame();
// Because the path got GC'd, it was removed from the cache and we're the
// only one holding it.
EXPECT_TRUE(path->path().isVolatile());
path->Release();
EXPECT_EQ(GetLiveTrackedPathCount(tracker), 0ul);
message_latch->Signal();
};

View File

@@ -91,7 +91,7 @@ static void BM_PathVolatilityTracker(benchmark::State& state) {
fml::AutoResetWaitableEvent latch;
task_runners.GetUITaskRunner()->PostTask([&]() {
for (auto path : paths) {
tracker.Insert(path);
tracker.Track(path);
}
latch.Signal();
});
@@ -101,7 +101,7 @@ static void BM_PathVolatilityTracker(benchmark::State& state) {
task_runners.GetUITaskRunner()->PostTask([&]() { tracker.OnFrame(); });
for (int i = 0; i < path_count - 10; ++i) {
tracker.Erase(paths[i]);
paths[i].reset();
}
task_runners.GetUITaskRunner()->PostTask([&]() { tracker.OnFrame(); });

View File

@@ -11,7 +11,7 @@ VolatilePathTracker::VolatilePathTracker(
bool enabled)
: ui_task_runner_(ui_task_runner), enabled_(enabled) {}
void VolatilePathTracker::Insert(std::shared_ptr<TrackedPath> path) {
void VolatilePathTracker::Track(std::shared_ptr<TrackedPath> path) {
FML_DCHECK(ui_task_runner_->RunsTasksOnCurrentThread());
FML_DCHECK(path);
FML_DCHECK(path->path.isVolatile());
@@ -19,22 +19,7 @@ void VolatilePathTracker::Insert(std::shared_ptr<TrackedPath> path) {
path->path.setIsVolatile(false);
return;
}
paths_.insert(path);
}
void VolatilePathTracker::Erase(std::shared_ptr<TrackedPath> path) {
if (!enabled_) {
return;
}
FML_DCHECK(path);
if (ui_task_runner_->RunsTasksOnCurrentThread()) {
paths_.erase(path);
return;
}
std::scoped_lock lock(paths_to_remove_mutex_);
needs_drain_ = true;
paths_to_remove_.push_back(path);
paths_.push_back(path);
}
void VolatilePathTracker::OnFrame() {
@@ -42,44 +27,35 @@ void VolatilePathTracker::OnFrame() {
if (!enabled_) {
return;
}
#if !FLUTTER_RELEASE
std::string total_count = std::to_string(paths_.size());
TRACE_EVENT1("flutter", "VolatilePathTracker::OnFrame", "total_count",
total_count.c_str());
#else
TRACE_EVENT0("flutter", "VolatilePathTracker::OnFrame");
#endif
Drain();
paths_.erase(std::remove_if(paths_.begin(), paths_.end(),
[](std::weak_ptr<TrackedPath> weak_path) {
auto path = weak_path.lock();
if (!path) {
return true;
}
path->frame_count++;
if (path->frame_count >= kFramesOfVolatility) {
path->path.setIsVolatile(false);
path->tracking_volatility = false;
return true;
}
return false;
}),
paths_.end());
std::set<std::shared_ptr<TrackedPath>> surviving_paths_;
for (const std::shared_ptr<TrackedPath>& path : paths_) {
path->frame_count++;
if (path->frame_count >= kFramesOfVolatility) {
path->path.setIsVolatile(false);
path->tracking_volatility = false;
} else {
surviving_paths_.insert(path);
}
}
paths_.swap(surviving_paths_);
#if !FLUTTER_RELEASE
std::string post_removal_count = std::to_string(paths_.size());
TRACE_EVENT_INSTANT1("flutter", "VolatilePathTracker::OnFrame",
"remaining_count", post_removal_count.c_str());
}
void VolatilePathTracker::Drain() {
if (needs_drain_) {
TRACE_EVENT0("flutter", "VolatilePathTracker::Drain");
std::deque<std::shared_ptr<TrackedPath>> paths_to_remove;
{
std::scoped_lock lock(paths_to_remove_mutex_);
paths_to_remove.swap(paths_to_remove_);
needs_drain_ = false;
}
std::string count = std::to_string(paths_to_remove.size());
TRACE_EVENT_INSTANT1("flutter", "VolatilePathTracker::Drain", "count",
count.c_str());
for (auto& path : paths_to_remove) {
paths_.erase(path);
}
}
#endif
}
} // namespace flutter

View File

@@ -6,8 +6,9 @@
#define FLUTTER_LIB_VOLATILE_PATH_TRACKER_H_
#include <deque>
#include <memory>
#include <mutex>
#include <set>
#include <vector>
#include "flutter/fml/macros.h"
#include "flutter/fml/task_runner.h"
@@ -16,6 +17,10 @@
namespace flutter {
namespace testing {
class ShellTest;
} // namespace testing
/// A cache for paths drawn from dart:ui.
///
/// Whenever a flutter::CanvasPath is created, it must Insert an entry into
@@ -46,12 +51,7 @@ class VolatilePathTracker {
// Must be called from the UI task runner.
//
// Callers should only insert paths that are currently volatile.
void Insert(std::shared_ptr<TrackedPath> path);
// Removes a path from tracking.
//
// May be called from any thread.
void Erase(std::shared_ptr<TrackedPath> path);
void Track(std::shared_ptr<TrackedPath> path);
// Called by the shell at the end of a frame after notifying Dart about idle
// time.
@@ -66,13 +66,10 @@ class VolatilePathTracker {
private:
fml::RefPtr<fml::TaskRunner> ui_task_runner_;
std::atomic_bool needs_drain_ = false;
std::mutex paths_to_remove_mutex_;
std::deque<std::shared_ptr<TrackedPath>> paths_to_remove_;
std::set<std::shared_ptr<TrackedPath>> paths_;
std::vector<std::weak_ptr<TrackedPath>> paths_;
bool enabled_ = true;
void Drain();
friend class testing::ShellTest;
FML_DISALLOW_COPY_AND_ASSIGN(VolatilePathTracker);
};

View File

@@ -402,5 +402,14 @@ bool ShellTest::IsAnimatorRunning(Shell* shell) {
return running;
}
size_t ShellTest::GetLiveTrackedPathCount(
std::shared_ptr<VolatilePathTracker> tracker) {
return std::count_if(
tracker->paths_.begin(), tracker->paths_.end(),
[](std::weak_ptr<VolatilePathTracker::TrackedPath> path) {
return path.lock();
});
}
} // namespace testing
} // namespace flutter

View File

@@ -15,6 +15,7 @@
#include "flutter/fml/build_config.h"
#include "flutter/fml/macros.h"
#include "flutter/fml/time/time_point.h"
#include "flutter/lib/ui/volatile_path_tracker.h"
#include "flutter/lib/ui/window/platform_message.h"
#include "flutter/shell/common/run_configuration.h"
#include "flutter/shell/common/shell_test_external_view_embedder.h"
@@ -124,6 +125,9 @@ class ShellTest : public FixtureTest {
// is unpredictive.
static int UnreportedTimingsCount(Shell* shell);
static size_t GetLiveTrackedPathCount(
std::shared_ptr<VolatilePathTracker> tracker);
private:
ThreadHost thread_host_;