Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Closed
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
2 changes: 2 additions & 0 deletions common/settings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ std::string Settings::ToString() const {
stream << "icu_data_path: " << icu_data_path << std::endl;
stream << "assets_dir: " << assets_dir << std::endl;
stream << "assets_path: " << assets_path << std::endl;
stream << "pipeline_state_observer set: " << !!pipeline_state_observer
<< std::endl;
return stream.str();
}

Expand Down
7 changes: 7 additions & 0 deletions common/settings.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "flutter/fml/closure.h"
#include "flutter/fml/mapping.h"
#include "flutter/fml/unique_fd.h"
#include "flutter/shell/common/pipeline.h"

namespace flutter {

Expand Down Expand Up @@ -149,6 +150,12 @@ struct Settings {
fml::UniqueFD::traits_type::InvalidValue();
std::string assets_path;

// An optional callback that allows the embedder to observe when pipeline
// items change state. For example, it could be used to have access to
// pipeline item metrics in a region of code that has access to platform
// specific APIs. Must be thread safe.
PipelineStateObserver pipeline_state_observer;

std::string ToString() const;
};

Expand Down
8 changes: 6 additions & 2 deletions shell/common/animator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "flutter/shell/common/animator.h"

#include "flutter/fml/trace_event.h"
#include "flutter/shell/common/pipeline.h"
Copy link
Member

Choose a reason for hiding this comment

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

How are you able to include this file here? This is in the common library which the shell depends on. This would make it a circular dependency. Did you run GN with the --check option?

#include "third_party/dart/runtime/include/dart_tools_api.h"

namespace flutter {
Expand All @@ -21,13 +22,16 @@ constexpr fml::TimeDelta kNotifyIdleTaskWaitTime =

Animator::Animator(Delegate& delegate,
TaskRunners task_runners,
std::unique_ptr<VsyncWaiter> waiter)
std::unique_ptr<VsyncWaiter> waiter,
PipelineStateObserver pipeline_state_observer)
: delegate_(delegate),
task_runners_(std::move(task_runners)),
waiter_(std::move(waiter)),
last_begin_frame_time_(),
dart_frame_deadline_(0),
layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(2)),
layer_tree_pipeline_(fml::MakeRefCounted<LayerTreePipeline>(
2,
std::move(pipeline_state_observer))),
pending_frame_semaphore_(1),
frame_number_(1),
paused_(false),
Expand Down
3 changes: 2 additions & 1 deletion shell/common/animator.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ class Animator final {

Animator(Delegate& delegate,
TaskRunners task_runners,
std::unique_ptr<VsyncWaiter> waiter);
std::unique_ptr<VsyncWaiter> waiter,
PipelineStateObserver pipeline_state_observer);

~Animator();

Expand Down
65 changes: 57 additions & 8 deletions shell/common/pipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
#ifndef FLUTTER_SHELL_COMMON_PIPELINE_H_
#define FLUTTER_SHELL_COMMON_PIPELINE_H_

#include <functional>
#include <memory>
#include <mutex>
#include <queue>

#include "flutter/fml/macros.h"
#include "flutter/fml/memory/ref_counted.h"
#include "flutter/fml/synchronization/semaphore.h"
#include "flutter/fml/trace_event.h"

#include <memory>
#include <mutex>
#include <queue>

namespace flutter {

enum class PipelineConsumeResult {
Expand All @@ -22,6 +23,26 @@ enum class PipelineConsumeResult {
MoreAvailable,
};

enum class PipelineItemStateChange {
// Indicates that a new pipeline item has started.
kBegin,
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion, maybe call this kProducerBegin, kProducerEnd, kConsumerBegin, kConsumerEnd/Abort. Since this is a single producer single consumer queue, I believe that would make the enum values clearer. As you noted, kStep should not happen more than once. So let's just make the names clearer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I think this Pipeline is not UI thread or GPU thread specific, as it can be used in other threads/TaskRunners, right? If so, maybe only mention UI and GPU thread as an example, and explain the high-level concept only using producer/consumer.

// Indicates that the pipeline item has advanced to its next phase. In
// practice this will only happen once, and means that the pipeline item has
// finished its UI thread work, and that GPU thread work is about to begin.
kStep,
// Indicates that the pipeline item is done.
kEnd,
// Indicates that the pipeline item was dropped.
kAbort,
};

// Called when pipeline items change states. See
// |Settings::pipeline_state_observer| for further details.
using PipelineStateObserver =
std::function<void(intptr_t pipeline_id,
Copy link
Member

Choose a reason for hiding this comment

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

A shell is guaranteed to have just one frame pipeline. Specifying the ID here is redundant as that information can just be associated in the std::function captures in the embedder.

size_t pipeline_item,
PipelineItemStateChange state_change)>;

size_t GetNextPipelineTraceID();

template <class R>
Expand Down Expand Up @@ -86,7 +107,10 @@ class Pipeline : public fml::RefCountedThreadSafe<Pipeline<R>> {
FML_DISALLOW_COPY_AND_ASSIGN(ProducerContinuation);
};

explicit Pipeline(uint32_t depth) : empty_(depth), available_(0) {}
Pipeline(uint32_t depth, PipelineStateObserver pipeline_state_observer)
: empty_(depth),
available_(0),
pipeline_state_observer_(std::move(pipeline_state_observer)) {}

~Pipeline() = default;

Expand All @@ -97,10 +121,17 @@ class Pipeline : public fml::RefCountedThreadSafe<Pipeline<R>> {
return {};
}

size_t trace_id = GetNextPipelineTraceID();
if (pipeline_state_observer_) {
pipeline_state_observer_(PipelineId(), trace_id,
PipelineItemStateChange::kBegin);
}

return ProducerContinuation{
std::bind(&Pipeline::ProducerCommit, this, std::placeholders::_1,
std::placeholders::_2), // continuation
GetNextPipelineTraceID()}; // trace id
trace_id // trace id
};
}

using Consumer = std::function<void(ResourcePtr)>;
Expand Down Expand Up @@ -135,7 +166,10 @@ class Pipeline : public fml::RefCountedThreadSafe<Pipeline<R>> {

TRACE_FLOW_END("flutter", "PipelineItem", trace_id);
TRACE_EVENT_ASYNC_END0("flutter", "PipelineItem", trace_id);

if (pipeline_state_observer_) {
pipeline_state_observer_(PipelineId(), trace_id,
PipelineItemStateChange::kEnd);
}
return items_count > 0 ? PipelineConsumeResult::MoreAvailable
: PipelineConsumeResult::Done;
}
Expand All @@ -145,18 +179,33 @@ class Pipeline : public fml::RefCountedThreadSafe<Pipeline<R>> {
fml::Semaphore available_;
std::mutex queue_mutex_;
std::queue<std::pair<ResourcePtr, size_t>> queue_;
PipelineStateObserver pipeline_state_observer_;

void ProducerCommit(ResourcePtr resource, size_t trace_id) {
bool had_resource = !!resource;

{
std::lock_guard<std::mutex> lock(queue_mutex_);
queue_.emplace(std::move(resource), trace_id);
}

// Ensure the queue mutex is not held as that would be a pessimization.
available_.Signal();

if (pipeline_state_observer_) {
Copy link
Member

Choose a reason for hiding this comment

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

I think the resource is always valid. You can probably just assert that here. I case of an aborted item, I believe that item is just dropped on the floor. So I don't actually think you will ever get an abort.

Copy link
Member

Choose a reason for hiding this comment

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

If you add these state observer fire events right next to the calls to TRACE_EVENT_ASYNC BEGIN, STEP and END, you should get the right behavior.

if (had_resource) {
pipeline_state_observer_(PipelineId(), trace_id,
PipelineItemStateChange::kStep);
} else {
pipeline_state_observer_(PipelineId(), trace_id,
PipelineItemStateChange::kAbort);
}
}
}

FML_DISALLOW_COPY_AND_ASSIGN(Pipeline);
intptr_t PipelineId() const { return intptr_t(this); }

FML_DISALLOW_COPY_ASSIGN_AND_MOVE(Pipeline);
};

} // namespace flutter
Expand Down
9 changes: 6 additions & 3 deletions shell/common/shell.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,18 @@ std::unique_ptr<Shell> Shell::CreateShellOnPlatformThread(
shared_snapshot = std::move(shared_snapshot), //
vsync_waiter = std::move(vsync_waiter), //
snapshot_delegate = std::move(snapshot_delegate), //
io_manager = io_manager->GetWeakPtr() //
io_manager = io_manager->GetWeakPtr(), //
pipeline_state_observer = //
std::move(settings.pipeline_state_observer) //
]() mutable {
TRACE_EVENT0("flutter", "ShellSetupUISubsystem");
const auto& task_runners = shell->GetTaskRunners();

// The animator is owned by the UI thread but it gets its vsync pulses
// from the platform.
auto animator = std::make_unique<Animator>(*shell, task_runners,
std::move(vsync_waiter));
auto animator = std::make_unique<Animator>(
*shell, task_runners, std::move(vsync_waiter),
std::move(pipeline_state_observer));

engine = std::make_unique<Engine>(*shell, //
*shell->GetDartVM(), //
Expand Down