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
35 changes: 22 additions & 13 deletions shell/common/pipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,16 +111,22 @@ class Pipeline : public fml::RefCountedThreadSafe<Pipeline<R>> {
GetNextPipelineTraceID()}; // trace id
}

// Pushes task to the front of the pipeline.
//
// If we exceed the depth completing this continuation, we drop the
// last frame to preserve the depth of the pipeline.
//
// Note: Use |Pipeline::Produce| where possible. This should only be
// used to en-queue high-priority resources.
ProducerContinuation ProduceToFront() {
// Create a `ProducerContinuation` that will only push the task if the queue
// is empty.
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be useful to add a brief description for the recommended usage of this method. Something along the lines of:

Prefer using Produce. ProducerContinuation returned by this method doesn't  guarantee that the frame will be rendered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

// Prefer using |Produce|. ProducerContinuation returned by this method
// doesn't guarantee that the frame will be rendered.
ProducerContinuation ProduceIfEmpty() {
if (!empty_.TryWait()) {
return {};
}
++inflight_;
FML_TRACE_COUNTER("flutter", "Pipeline Depth",
reinterpret_cast<int64_t>(this), //
"frames in flight", inflight_.load() //
);

return ProducerContinuation{
std::bind(&Pipeline::ProducerCommitFront, this, std::placeholders::_1,
std::bind(&Pipeline::ProducerCommitIfEmpty, this, std::placeholders::_1,
std::placeholders::_2), // continuation
GetNextPipelineTraceID()}; // trace id
}
Expand Down Expand Up @@ -181,13 +187,16 @@ class Pipeline : public fml::RefCountedThreadSafe<Pipeline<R>> {
available_.Signal();
}

void ProducerCommitFront(ResourcePtr resource, size_t trace_id) {
void ProducerCommitIfEmpty(ResourcePtr resource, size_t trace_id) {
{
std::scoped_lock lock(queue_mutex_);
queue_.emplace_front(std::move(resource), trace_id);
while (queue_.size() > depth_) {
queue_.pop_back();
if (!queue_.empty()) {
// Bail if the queue is not empty, opens up spaces to produce other
// frames.
empty_.Signal();
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a note here describing why this semaphore needs to be signaled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return;
}
queue_.emplace_back(std::move(resource), trace_id);
}

// Ensure the queue mutex is not held as that would be a pessimization.
Expand Down
28 changes: 8 additions & 20 deletions shell/common/pipeline_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,46 +89,34 @@ TEST(PipelineTest, PushingMultiProcessesInOrder) {
ASSERT_EQ(consume_result_2, PipelineConsumeResult::Done);
}

TEST(PipelineTest, PushingToFrontOverridesOrder) {
TEST(PipelineTest, ProduceIfEmptyDoesNotConsumeWhenQueueIsNotEmpty) {
const int depth = 2;
fml::RefPtr<IntPipeline> pipeline = fml::MakeRefCounted<IntPipeline>(depth);

Continuation continuation_1 = pipeline->Produce();
Continuation continuation_2 = pipeline->ProduceToFront();
Continuation continuation_2 = pipeline->ProduceIfEmpty();

const int test_val_1 = 1, test_val_2 = 2;
continuation_1.Complete(std::make_unique<int>(test_val_1));
continuation_2.Complete(std::make_unique<int>(test_val_2));

PipelineConsumeResult consume_result_1 = pipeline->Consume(
[&test_val_2](std::unique_ptr<int> v) { ASSERT_EQ(*v, test_val_2); });
ASSERT_EQ(consume_result_1, PipelineConsumeResult::MoreAvailable);

PipelineConsumeResult consume_result_2 = pipeline->Consume(
[&test_val_1](std::unique_ptr<int> v) { ASSERT_EQ(*v, test_val_1); });
ASSERT_EQ(consume_result_2, PipelineConsumeResult::Done);
ASSERT_EQ(consume_result_1, PipelineConsumeResult::Done);
}

TEST(PipelineTest, PushingToFrontDropsLastResource) {
const int depth = 2;
TEST(PipelineTest, ProduceIfEmptySuccessfulIfQueueIsEmpty) {
const int depth = 1;
fml::RefPtr<IntPipeline> pipeline = fml::MakeRefCounted<IntPipeline>(depth);

Continuation continuation_1 = pipeline->Produce();
Continuation continuation_2 = pipeline->Produce();
Continuation continuation_3 = pipeline->ProduceToFront();
Continuation continuation_1 = pipeline->ProduceIfEmpty();

const int test_val_1 = 1, test_val_2 = 2, test_val_3 = 3;
const int test_val_1 = 1;
continuation_1.Complete(std::make_unique<int>(test_val_1));
continuation_2.Complete(std::make_unique<int>(test_val_2));
continuation_3.Complete(std::make_unique<int>(test_val_3));

PipelineConsumeResult consume_result_1 = pipeline->Consume(
[&test_val_3](std::unique_ptr<int> v) { ASSERT_EQ(*v, test_val_3); });
ASSERT_EQ(consume_result_1, PipelineConsumeResult::MoreAvailable);

PipelineConsumeResult consume_result_2 = pipeline->Consume(
[&test_val_1](std::unique_ptr<int> v) { ASSERT_EQ(*v, test_val_1); });
ASSERT_EQ(consume_result_2, PipelineConsumeResult::Done);
ASSERT_EQ(consume_result_1, PipelineConsumeResult::Done);
}

} // namespace testing
Expand Down
3 changes: 1 addition & 2 deletions shell/common/rasterizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,8 @@ void Rasterizer::Draw(fml::RefPtr<Pipeline<flutter::LayerTree>> pipeline) {
// if the raster status is to resubmit the frame, we push the frame to the
// front of the queue and also change the consume status to more available.
if (raster_status == RasterStatus::kResubmit) {
auto front_continuation = pipeline->ProduceToFront();
auto front_continuation = pipeline->ProduceIfEmpty();
front_continuation.Complete(std::move(resubmitted_layer_tree_));
consume_result = PipelineConsumeResult::MoreAvailable;
} else if (raster_status == RasterStatus::kEnqueuePipeline) {
consume_result = PipelineConsumeResult::MoreAvailable;
}
Expand Down