Skip to content
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
49 changes: 29 additions & 20 deletions bridge/foundation/ui_command_ring_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "ui_command_ring_buffer.h"
#include <algorithm>
#include <cstring>
#include <limits>
#include "bindings/qjs/native_string_utils.h"
#include "core/executing_context.h"
#include "foundation/logging.h"
Expand Down Expand Up @@ -201,9 +202,6 @@ void UICommandPackage::AddCommand(const UICommandItem& item) {
}

bool UICommandPackage::ShouldSplit(UICommand next_command) const {
// Split package based on command type strategy
UICommandKind next_kind = GetKindFromUICommand(next_command);

// Always split on certain commands
switch (next_command) {
case UICommand::kStartRecordingCommand:
Expand All @@ -214,13 +212,9 @@ bool UICommandPackage::ShouldSplit(UICommand next_command) const {
break;
}

// Split if mixing incompatible command types
if ((kind_mask & static_cast<uint32_t>(UICommandKind::kNodeCreation)) &&
(static_cast<uint32_t>(next_kind) & static_cast<uint32_t>(UICommandKind::kNodeMutation))) {
return true;
}

// Split if package is getting too large
// Keep alternating create/insert DOM command streams together. Splitting on
// every node mutation turns large mounts into thousands of tiny packages and
// overwhelms the package ring buffer before Dart can drain it.
if (commands.size() >= 1000) {
return true;
}
Expand Down Expand Up @@ -330,8 +324,17 @@ void UICommandPackageRingBuffer::PushPackage(std::unique_ptr<UICommandPackage> p
if (next_write_idx == read_index_.load(std::memory_order_acquire)) {
// Buffer full, use overflow
std::lock_guard<std::mutex> lock(overflow_mutex_);
WEBF_LOG(WARN) << "[UICommandPackageRingBuffer] PUSH PACKAGE TO OVERFLOW " << package.get();
overflow_packages_.push_back(std::move(package));

uint64_t overflow_count = overflow_push_count_.fetch_add(1, std::memory_order_relaxed) + 1;
if (overflow_count == 1 || overflow_count % 64 == 0) {
size_t ring_count = (write_idx - read_index_.load(std::memory_order_acquire)) & capacity_mask_;
const auto* newest_package = overflow_packages_.back().get();
size_t newest_size = newest_package ? newest_package->commands.size() : 0;
WEBF_LOG(WARN) << "[UICommandPackageRingBuffer] overflow packages=" << overflow_packages_.size()
<< " total_overflows=" << overflow_count << " ring_backlog=" << ring_count
<< " newest_package_commands=" << newest_size << " capacity=" << capacity_;
}
return;
}

Expand All @@ -344,21 +347,25 @@ void UICommandPackageRingBuffer::PushPackage(std::unique_ptr<UICommandPackage> p
}

std::unique_ptr<UICommandPackage> UICommandPackageRingBuffer::PopPackage() {
// First check overflow
size_t read_idx = read_index_.load(std::memory_order_relaxed);
bool ring_ready = packages_[read_idx].ready.load(std::memory_order_acquire);
uint64_t ring_sequence = ring_ready && packages_[read_idx].package
? packages_[read_idx].package->sequence_number
: std::numeric_limits<uint64_t>::max();

{
std::lock_guard<std::mutex> lock(overflow_mutex_);
if (!overflow_packages_.empty()) {
auto package = std::move(overflow_packages_.front());
WEBF_LOG(VERBOSE) << " POP OVERFLOW PACKAGE " << package.get();
overflow_packages_.erase(overflow_packages_.begin());
return package;
uint64_t overflow_sequence = overflow_packages_.front()->sequence_number;
if (!ring_ready || overflow_sequence < ring_sequence) {
auto package = std::move(overflow_packages_.front());
overflow_packages_.erase(overflow_packages_.begin());
return package;
}
}
}

// Then check ring buffer
size_t read_idx = read_index_.load(std::memory_order_relaxed);

if (!packages_[read_idx].ready.load(std::memory_order_acquire)) {
if (!ring_ready) {
return nullptr; // No package available
}

Expand Down Expand Up @@ -432,6 +439,8 @@ void UICommandPackageRingBuffer::Clear() {
std::lock_guard<std::mutex> overflow_lock(overflow_mutex_);
overflow_packages_.clear();
}

overflow_push_count_.store(0, std::memory_order_relaxed);
}

bool UICommandPackageRingBuffer::ShouldCreateNewPackage(UICommand command) const {
Expand Down
1 change: 1 addition & 0 deletions bridge/foundation/ui_command_ring_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ class UICommandPackageRingBuffer {
// Overflow handling
std::mutex overflow_mutex_;
std::vector<std::unique_ptr<UICommandPackage>> overflow_packages_;
std::atomic<uint64_t> overflow_push_count_{0};

// Helper methods
bool ShouldCreateNewPackage(UICommand command) const;
Expand Down
24 changes: 21 additions & 3 deletions bridge/foundation/ui_command_ring_buffer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,8 @@ TEST_F(UICommandRingBufferTest, CommandBatchingStrategy) {
package.AddCommand(UICommandItem(static_cast<int32_t>(UICommand::kCreateTextNode), nullptr, nullptr, nullptr));
EXPECT_FALSE(package.ShouldSplit(UICommand::kCreateComment));

// Test split on node mutation after creation
EXPECT_TRUE(package.ShouldSplit(UICommand::kInsertAdjacentNode));
// Keep the common create/insert sequence in the same package.
EXPECT_FALSE(package.ShouldSplit(UICommand::kInsertAdjacentNode));

// Test split on special commands
package.Clear();
Expand All @@ -187,6 +187,24 @@ TEST_F(UICommandRingBufferTest, CommandBatchingStrategy) {
EXPECT_TRUE(package.ShouldSplit(UICommand::kAsyncCaller));
}

TEST_F(UICommandRingBufferTest, PackageOverflowPreservesSequenceOrder) {
UICommandPackageRingBuffer buffer(nullptr, 4);

for (intptr_t i = 1; i <= 5; ++i) {
buffer.AddCommand(UICommand::kCreateElement, nullptr, reinterpret_cast<void*>(i), nullptr);
buffer.FlushCurrentPackage();
}

for (intptr_t expected = 1; expected <= 5; ++expected) {
auto package = buffer.PopPackage();
ASSERT_NE(package, nullptr);
ASSERT_EQ(package->commands.size(), 1u);
EXPECT_EQ(package->commands[0].nativePtr, expected);
}

EXPECT_EQ(buffer.PopPackage(), nullptr);
}

// Test SharedUICommandRingBuffer integration
TEST_F(UICommandRingBufferTest, SharedUICommandIntegration) {
// Note: This test is disabled as it requires a proper ExecutingContext
Expand Down Expand Up @@ -234,4 +252,4 @@ TEST_F(UICommandRingBufferTest, StressTestHighVolume) {
EXPECT_TRUE(buffer.Empty());
}

} // namespace webf
} // namespace webf
Loading
Loading