Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
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
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1455,6 +1455,7 @@ FILE: ../../../flutter/shell/platform/windows/window_binding_handler_delegate.h
FILE: ../../../flutter/shell/platform/windows/window_state.h
FILE: ../../../flutter/shell/profiling/sampling_profiler.cc
FILE: ../../../flutter/shell/profiling/sampling_profiler.h
FILE: ../../../flutter/shell/profiling/sampling_profiler_unittest.cc
FILE: ../../../flutter/shell/version/version.cc
FILE: ../../../flutter/shell/version/version.h
FILE: ../../../flutter/sky/packages/flutter_services/lib/empty.dart
Expand Down
1 change: 1 addition & 0 deletions shell/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ if (enable_unittests) {
":shell_unittests_fixtures",
"//flutter/assets",
"//flutter/common/graphics",
"//flutter/shell/profiling:profiling_unittests",
"//flutter/shell/version",
"//third_party/googletest:gmock",
]
Expand Down
9 changes: 9 additions & 0 deletions shell/profiling/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,12 @@ source_set("profiling") {

deps = _profiler_deps
}

source_set("profiling_unittests") {
testonly = true
sources = [ "sampling_profiler_unittest.cc" ]
deps = [
":profiling",
"//flutter/testing",
]
}
27 changes: 24 additions & 3 deletions shell/profiling/sampling_profiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,13 @@ SamplingProfiler::SamplingProfiler(
sampler_(std::move(sampler)),
num_samples_per_sec_(num_samples_per_sec) {}

void SamplingProfiler::Start() const {
SamplingProfiler::~SamplingProfiler() {
if (is_running_) {
Stop();
Copy link
Member

Choose a reason for hiding this comment

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

Can we add some tests? i.e. you can start and stop and start etc and check the scheduling of samples (even if you mock out the meaty parts). And check that destroying indeed stops.

Copy link
Member Author

Choose a reason for hiding this comment

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

Man, what a doozy to test, specially since there wasn't any existing tests. Ok, added.

}
}

void SamplingProfiler::Start() {
if (!profiler_task_runner_) {
return;
}
Expand All @@ -26,12 +32,23 @@ void SamplingProfiler::Start() const {
double delay_between_samples = 1.0 / num_samples_per_sec_;
auto task_delay = fml::TimeDelta::FromSecondsF(delay_between_samples);
UpdateObservatoryThreadName();
is_running_ = true;
SampleRepeatedly(task_delay);
}

void SamplingProfiler::Stop() {
FML_DCHECK(is_running_);
auto latch = std::make_unique<fml::AutoResetWaitableEvent>();
shutdown_latch_.store(latch.get());
latch->Wait();
shutdown_latch_.store(nullptr);
is_running_ = false;
}

void SamplingProfiler::SampleRepeatedly(fml::TimeDelta task_delay) const {
profiler_task_runner_->PostDelayedTask(
[profiler = this, task_delay = task_delay, sampler = sampler_]() {
[profiler = this, task_delay = task_delay, sampler = sampler_,
&shutdown_latch = shutdown_latch_]() {
// TODO(kaushikiska): consider buffering these every n seconds to
// avoid spamming the trace buffer.
const ProfileSample usage = sampler();
Expand Down Expand Up @@ -60,7 +77,11 @@ void SamplingProfiler::SampleRepeatedly(fml::TimeDelta task_delay) const {
TRACE_EVENT_INSTANT1("flutter::profiling", "GpuUsage", "gpu_usage",
gpu_usage.c_str());
}
profiler->SampleRepeatedly(task_delay);
if (shutdown_latch.load()) {
shutdown_latch.load()->Signal();
} else {
profiler->SampleRepeatedly(task_delay);
}
},
task_delay);
}
Expand Down
9 changes: 8 additions & 1 deletion shell/profiling/sampling_profiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <optional>
#include <string>

#include "flutter/fml/synchronization/count_down_latch.h"
#include "flutter/fml/task_runner.h"
#include "flutter/fml/trace_event.h"

Expand Down Expand Up @@ -97,17 +98,23 @@ class SamplingProfiler {
Sampler sampler,
int num_samples_per_sec);

~SamplingProfiler();

/**
* @brief Starts the SamplingProfiler by triggering `SampleRepeatedly`.
*
*/
void Start() const;
void Start();

void Stop();

private:
const std::string thread_label_;
const fml::RefPtr<fml::TaskRunner> profiler_task_runner_;
const Sampler sampler_;
const uint32_t num_samples_per_sec_;
bool is_running_ = false;
std::atomic<fml::AutoResetWaitableEvent*> shutdown_latch_ = nullptr;

void SampleRepeatedly(fml::TimeDelta task_delay) const;

Expand Down
64 changes: 64 additions & 0 deletions shell/profiling/sampling_profiler_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// Copyright 2013 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.

#include "flutter/shell/profiling/sampling_profiler.h"
#include "flutter/fml/message_loop_impl.h"
#include "flutter/fml/thread.h"
#include "flutter/testing/testing.h"
#include "gmock/gmock.h"

using testing::_;
using testing::Invoke;

namespace fml {
namespace {
class MockTaskRunner : public fml::TaskRunner {
public:
inline static RefPtr<MockTaskRunner> Create() {
return AdoptRef(new MockTaskRunner());
}
MOCK_METHOD1(PostTask, void(const fml::closure& task));
MOCK_METHOD2(PostTaskForTime,
void(const fml::closure& task, fml::TimePoint target_time));
MOCK_METHOD2(PostDelayedTask,
void(const fml::closure& task, fml::TimeDelta delay));
MOCK_METHOD0(RunsTasksOnCurrentThread, bool());
MOCK_METHOD0(GetTaskQueueId, TaskQueueId());

private:
MockTaskRunner() : TaskRunner(fml::RefPtr<MessageLoopImpl>()) {}
};
} // namespace
} // namespace fml

namespace flutter {

TEST(SamplingProfilerTest, DeleteAfterStart) {
auto thread =
std::make_unique<fml::Thread>(flutter::testing::GetCurrentTestName());
auto task_runner = fml::MockTaskRunner::Create();
std::atomic<int> invoke_count = 0;

// Ignore calls to PostTask since that would require mocking out calls to
// Dart.
EXPECT_CALL(*task_runner, PostDelayedTask(_, _))
.WillRepeatedly(
Invoke([&](const fml::closure& task, fml::TimeDelta delay) {
invoke_count.fetch_add(1);
thread->GetTaskRunner()->PostTask(task);
}));

{
auto profiler = SamplingProfiler(
"profiler",
/*profiler_task_runner=*/task_runner, [] { return ProfileSample(); },
/*num_samples_per_sec=*/1000);
profiler.Start();
}
int invoke_count_at_delete = invoke_count.load();
std::this_thread::sleep_for(std::chrono::milliseconds(2)); // nyquist
ASSERT_EQ(invoke_count_at_delete, invoke_count.load());
}

} // namespace flutter