Commit 8943a9a4 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

Simplify DisplayItemList initial capacity

Change ContiguousContainer to allocate the initial buffer when adding
the first element, so that the initial capacity can be passed as a
parameter of the constructor.

Remove ShrinkToFit() because it does nothing as we don't remove display
items (and we don't have other users of ContiguousContainer). It was
needed when we had paired display items and we may remove display
items.

Bug: 1130242
Change-Id: I263fc95db20b1571739fcbd6bb0ce13b393416f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2446480Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#813826}
parent 8db6ec01
...@@ -158,7 +158,7 @@ PaintChunk::Id DefaultId() { ...@@ -158,7 +158,7 @@ PaintChunk::Id DefaultId() {
struct TestChunks { struct TestChunks {
Vector<PaintChunk> chunks; Vector<PaintChunk> chunks;
DisplayItemList items = DisplayItemList(0); DisplayItemList items;
// Add a paint chunk with a non-empty paint record and given property nodes. // Add a paint chunk with a non-empty paint record and given property nodes.
void AddChunk( void AddChunk(
......
...@@ -14,10 +14,6 @@ ...@@ -14,10 +14,6 @@
namespace blink { namespace blink {
// Default number of max-sized elements to allocate space for, if there is no
// initial buffer.
static const unsigned kDefaultInitialBufferSize = 32;
class ContiguousContainerBase::Buffer { class ContiguousContainerBase::Buffer {
USING_FAST_MALLOC(Buffer); USING_FAST_MALLOC(Buffer);
...@@ -57,7 +53,7 @@ class ContiguousContainerBase::Buffer { ...@@ -57,7 +53,7 @@ class ContiguousContainerBase::Buffer {
} }
private: private:
// m_begin <= m_end <= m_begin + m_capacity // begin_ <= end_ <= begin_ + capacity_
char* begin_; char* begin_;
char* end_; char* end_;
wtf_size_t capacity_; wtf_size_t capacity_;
...@@ -65,12 +61,18 @@ class ContiguousContainerBase::Buffer { ...@@ -65,12 +61,18 @@ class ContiguousContainerBase::Buffer {
DISALLOW_COPY_AND_ASSIGN(Buffer); DISALLOW_COPY_AND_ASSIGN(Buffer);
}; };
ContiguousContainerBase::ContiguousContainerBase(wtf_size_t max_object_size) ContiguousContainerBase::ContiguousContainerBase(
: end_index_(0), max_object_size_(max_object_size) {} wtf_size_t max_object_size,
wtf_size_t initial_capacity_in_bytes)
: end_index_(0),
max_object_size_(max_object_size),
initial_capacity_in_bytes_(
std::max(max_object_size, initial_capacity_in_bytes)) {}
ContiguousContainerBase::ContiguousContainerBase( ContiguousContainerBase::ContiguousContainerBase(
ContiguousContainerBase&& source) ContiguousContainerBase&& source)
: ContiguousContainerBase(source.max_object_size_) { : ContiguousContainerBase(source.max_object_size_,
source.initial_capacity_in_bytes_) {
Swap(source); Swap(source);
} }
...@@ -101,11 +103,6 @@ wtf_size_t ContiguousContainerBase::MemoryUsageInBytes() const { ...@@ -101,11 +103,6 @@ wtf_size_t ContiguousContainerBase::MemoryUsageInBytes() const {
elements_.capacity() * sizeof(elements_[0]); elements_.capacity() * sizeof(elements_[0]);
} }
void ContiguousContainerBase::ReserveInitialCapacity(wtf_size_t buffer_size,
const char* type_name) {
AllocateNewBufferForNextAllocation(buffer_size, type_name);
}
void* ContiguousContainerBase::Allocate(wtf_size_t object_size, void* ContiguousContainerBase::Allocate(wtf_size_t object_size,
const char* type_name) { const char* type_name) {
DCHECK_LE(object_size, max_object_size_); DCHECK_LE(object_size, max_object_size_);
...@@ -120,8 +117,8 @@ void* ContiguousContainerBase::Allocate(wtf_size_t object_size, ...@@ -120,8 +117,8 @@ void* ContiguousContainerBase::Allocate(wtf_size_t object_size,
} }
if (!buffer_for_alloc) { if (!buffer_for_alloc) {
wtf_size_t new_buffer_size = wtf_size_t new_buffer_size = buffers_.IsEmpty()
buffers_.IsEmpty() ? kDefaultInitialBufferSize * max_object_size_ ? initial_capacity_in_bytes_
: 2 * buffers_.back()->Capacity(); : 2 * buffers_.back()->Capacity();
buffer_for_alloc = buffer_for_alloc =
AllocateNewBufferForNextAllocation(new_buffer_size, type_name); AllocateNewBufferForNextAllocation(new_buffer_size, type_name);
...@@ -158,13 +155,7 @@ void ContiguousContainerBase::Swap(ContiguousContainerBase& other) { ...@@ -158,13 +155,7 @@ void ContiguousContainerBase::Swap(ContiguousContainerBase& other) {
buffers_.swap(other.buffers_); buffers_.swap(other.buffers_);
std::swap(end_index_, other.end_index_); std::swap(end_index_, other.end_index_);
std::swap(max_object_size_, other.max_object_size_); std::swap(max_object_size_, other.max_object_size_);
} std::swap(initial_capacity_in_bytes_, other.initial_capacity_in_bytes_);
void ContiguousContainerBase::ShrinkToFit() {
while (end_index_ < buffers_.size() - 1) {
DCHECK(buffers_.back()->IsEmpty());
buffers_.pop_back();
}
} }
ContiguousContainerBase::Buffer* ContiguousContainerBase::Buffer*
......
...@@ -41,7 +41,9 @@ class PLATFORM_EXPORT ContiguousContainerBase { ...@@ -41,7 +41,9 @@ class PLATFORM_EXPORT ContiguousContainerBase {
DISALLOW_NEW(); DISALLOW_NEW();
protected: protected:
explicit ContiguousContainerBase(wtf_size_t max_object_size); // The initial capacity will be allocated when the first item is added.
ContiguousContainerBase(wtf_size_t max_object_size,
wtf_size_t initial_capacity_in_bytes);
ContiguousContainerBase(ContiguousContainerBase&&); ContiguousContainerBase(ContiguousContainerBase&&);
~ContiguousContainerBase(); ~ContiguousContainerBase();
...@@ -54,16 +56,11 @@ class PLATFORM_EXPORT ContiguousContainerBase { ...@@ -54,16 +56,11 @@ class PLATFORM_EXPORT ContiguousContainerBase {
wtf_size_t MemoryUsageInBytes() const; wtf_size_t MemoryUsageInBytes() const;
// These do not invoke constructors or destructors. // These do not invoke constructors or destructors.
void ReserveInitialCapacity(wtf_size_t, const char* type_name);
void* Allocate(wtf_size_t object_size, const char* type_name); void* Allocate(wtf_size_t object_size, const char* type_name);
void RemoveLast(); void RemoveLast();
void Clear(); void Clear();
void Swap(ContiguousContainerBase&); void Swap(ContiguousContainerBase&);
// Discards excess buffer capacity. Intended for use when no more appending
// is anticipated.
void ShrinkToFit();
Vector<void*> elements_; Vector<void*> elements_;
private: private:
...@@ -74,6 +71,7 @@ class PLATFORM_EXPORT ContiguousContainerBase { ...@@ -74,6 +71,7 @@ class PLATFORM_EXPORT ContiguousContainerBase {
Vector<std::unique_ptr<Buffer>> buffers_; Vector<std::unique_ptr<Buffer>> buffers_;
unsigned end_index_; unsigned end_index_;
wtf_size_t max_object_size_; wtf_size_t max_object_size_;
wtf_size_t initial_capacity_in_bytes_;
DISALLOW_COPY_AND_ASSIGN(ContiguousContainerBase); DISALLOW_COPY_AND_ASSIGN(ContiguousContainerBase);
}; };
...@@ -138,14 +136,10 @@ class ContiguousContainer : public ContiguousContainerBase { ...@@ -138,14 +136,10 @@ class ContiguousContainer : public ContiguousContainerBase {
using value_type = BaseElementType; using value_type = BaseElementType;
explicit ContiguousContainer(wtf_size_t max_object_size) ContiguousContainer(wtf_size_t max_object_size,
: ContiguousContainerBase(Align(max_object_size)) {} wtf_size_t initial_capacity_in_bytes)
: ContiguousContainerBase(Align(max_object_size),
ContiguousContainer(wtf_size_t max_object_size, wtf_size_t initial_size_bytes) initial_capacity_in_bytes) {}
: ContiguousContainer(max_object_size) {
ReserveInitialCapacity(std::max(max_object_size, initial_size_bytes),
WTF_HEAP_PROFILER_TYPE_NAME(BaseElementType));
}
ContiguousContainer(ContiguousContainer&& source) ContiguousContainer(ContiguousContainer&& source)
: ContiguousContainerBase(std::move(source)) {} : ContiguousContainerBase(std::move(source)) {}
...@@ -169,7 +163,6 @@ class ContiguousContainer : public ContiguousContainerBase { ...@@ -169,7 +163,6 @@ class ContiguousContainer : public ContiguousContainerBase {
using ContiguousContainerBase::CapacityInBytes; using ContiguousContainerBase::CapacityInBytes;
using ContiguousContainerBase::IsEmpty; using ContiguousContainerBase::IsEmpty;
using ContiguousContainerBase::MemoryUsageInBytes; using ContiguousContainerBase::MemoryUsageInBytes;
using ContiguousContainerBase::ShrinkToFit;
using ContiguousContainerBase::size; using ContiguousContainerBase::size;
using ContiguousContainerBase::UsedCapacityInBytes; using ContiguousContainerBase::UsedCapacityInBytes;
......
...@@ -28,8 +28,16 @@ static constexpr wtf_size_t kMaximumDisplayItemSize = ...@@ -28,8 +28,16 @@ static constexpr wtf_size_t kMaximumDisplayItemSize =
class PLATFORM_EXPORT DisplayItemList class PLATFORM_EXPORT DisplayItemList
: public ContiguousContainer<DisplayItem, kDisplayItemAlignment> { : public ContiguousContainer<DisplayItem, kDisplayItemAlignment> {
public: public:
DisplayItemList(wtf_size_t initial_size_bytes) static constexpr wtf_size_t kDefaultCapacityInBytes = 512;
: ContiguousContainer(kMaximumDisplayItemSize, initial_size_bytes) {}
// Using 0 as the default value to make 0 also fall back to
// kDefaultCapacityInBytes.
explicit DisplayItemList(wtf_size_t initial_capacity_in_bytes = 0)
: ContiguousContainer(kMaximumDisplayItemSize,
initial_capacity_in_bytes
? initial_capacity_in_bytes
: kDefaultCapacityInBytes) {}
DisplayItemList(DisplayItemList&& source) DisplayItemList(DisplayItemList&& source)
: ContiguousContainer(std::move(source)) {} : ContiguousContainer(std::move(source)) {}
......
...@@ -14,8 +14,6 @@ ...@@ -14,8 +14,6 @@
namespace blink { namespace blink {
PaintArtifact::PaintArtifact() : display_item_list_(0) {}
PaintArtifact::PaintArtifact(DisplayItemList display_items, PaintArtifact::PaintArtifact(DisplayItemList display_items,
Vector<PaintChunk> chunks) Vector<PaintChunk> chunks)
: display_item_list_(std::move(display_items)), chunks_(std::move(chunks)) { : display_item_list_(std::move(display_items)), chunks_(std::move(chunks)) {
......
...@@ -90,7 +90,7 @@ class PLATFORM_EXPORT PaintArtifact final : public RefCounted<PaintArtifact> { ...@@ -90,7 +90,7 @@ class PLATFORM_EXPORT PaintArtifact final : public RefCounted<PaintArtifact> {
const PaintChunkSubset& paint_chunks) const; const PaintChunkSubset& paint_chunks) const;
private: private:
PaintArtifact(); PaintArtifact() = default;
PaintArtifact(DisplayItemList, Vector<PaintChunk>); PaintArtifact(DisplayItemList, Vector<PaintChunk>);
DisplayItemList display_item_list_; DisplayItemList display_item_list_;
......
...@@ -15,9 +15,7 @@ ...@@ -15,9 +15,7 @@
namespace blink { namespace blink {
PaintController::PaintController(Usage usage) PaintController::PaintController(Usage usage)
: usage_(usage), : usage_(usage), current_paint_artifact_(PaintArtifact::Empty()) {
current_paint_artifact_(PaintArtifact::Empty()),
new_display_item_list_(0) {
// frame_first_paints_ should have one null frame since the beginning, so // frame_first_paints_ should have one null frame since the beginning, so
// that PaintController is robust even if it paints outside of BeginFrame // that PaintController is robust even if it paints outside of BeginFrame
// and EndFrame cycles. It will also enable us to combine the first paint // and EndFrame cycles. It will also enable us to combine the first paint
...@@ -61,7 +59,6 @@ bool PaintController::UseCachedItemIfPossible(const DisplayItemClient& client, ...@@ -61,7 +59,6 @@ bool PaintController::UseCachedItemIfPossible(const DisplayItemClient& client,
} }
++num_cached_new_items_; ++num_cached_new_items_;
EnsureNewDisplayItemListInitialCapacity();
if (!RuntimeEnabledFeatures::PaintUnderInvalidationCheckingEnabled()) if (!RuntimeEnabledFeatures::PaintUnderInvalidationCheckingEnabled())
ProcessNewItem(MoveItemFromCurrentListToNewList(cached_item)); ProcessNewItem(MoveItemFromCurrentListToNewList(cached_item));
...@@ -127,8 +124,6 @@ bool PaintController::UseCachedSubsequenceIfPossible( ...@@ -127,8 +124,6 @@ bool PaintController::UseCachedSubsequenceIfPossible(
return false; return false;
} }
EnsureNewDisplayItemListInitialCapacity();
if (next_item_to_match_ == start_item_index) { if (next_item_to_match_ == start_item_index) {
// We are matching new and cached display items sequentially. Skip the // We are matching new and cached display items sequentially. Skip the
// subsequence for later sequential matching of individual display items. // subsequence for later sequential matching of individual display items.
...@@ -506,9 +501,6 @@ void PaintController::CommitNewDisplayItems() { ...@@ -506,9 +501,6 @@ void PaintController::CommitNewDisplayItems() {
new_cached_subsequences_.swap(current_cached_subsequences_); new_cached_subsequences_.swap(current_cached_subsequences_);
new_cached_subsequences_.clear(); new_cached_subsequences_.clear();
// The new list will not be appended to again so we can release unused memory.
new_display_item_list_.ShrinkToFit();
current_paint_artifact_ = current_paint_artifact_ =
PaintArtifact::Create(std::move(new_display_item_list_), PaintArtifact::Create(std::move(new_display_item_list_),
new_paint_chunks_.ReleasePaintChunks()); new_paint_chunks_.ReleasePaintChunks());
...@@ -517,7 +509,8 @@ void PaintController::CommitNewDisplayItems() { ...@@ -517,7 +509,8 @@ void PaintController::CommitNewDisplayItems() {
out_of_order_item_id_index_map_.clear(); out_of_order_item_id_index_map_.clear();
// We'll allocate the initial buffer when we start the next paint. // We'll allocate the initial buffer when we start the next paint.
new_display_item_list_ = DisplayItemList(0); new_display_item_list_ =
DisplayItemList(GetDisplayItemList().UsedCapacityInBytes());
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
num_indexed_items_ = 0; num_indexed_items_ = 0;
......
...@@ -29,8 +29,6 @@ ...@@ -29,8 +29,6 @@
namespace blink { namespace blink {
static constexpr wtf_size_t kInitialDisplayItemListCapacityBytes = 512;
enum class PaintBenchmarkMode { enum class PaintBenchmarkMode {
kNormal, kNormal,
kForceRasterInvalidationAndConvert, kForceRasterInvalidationAndConvert,
...@@ -148,7 +146,6 @@ class PLATFORM_EXPORT PaintController { ...@@ -148,7 +146,6 @@ class PLATFORM_EXPORT PaintController {
"DisplayItem subclass alignment is not a factor of " "DisplayItem subclass alignment is not a factor of "
"kDisplayItemAlignment."); "kDisplayItemAlignment.");
EnsureNewDisplayItemListInitialCapacity();
DisplayItemClass& display_item = DisplayItemClass& display_item =
new_display_item_list_.AllocateAndConstruct<DisplayItemClass>( new_display_item_list_.AllocateAndConstruct<DisplayItemClass>(
std::forward<Args>(args)...); std::forward<Args>(args)...);
...@@ -303,17 +300,6 @@ class PLATFORM_EXPORT PaintController { ...@@ -303,17 +300,6 @@ class PLATFORM_EXPORT PaintController {
void InvalidateAllForTesting() { InvalidateAllInternal(); } void InvalidateAllForTesting() { InvalidateAllInternal(); }
void InvalidateAllInternal(); void InvalidateAllInternal();
void EnsureNewDisplayItemListInitialCapacity() {
if (new_display_item_list_.IsEmpty()) {
// TODO(wangxianzhu): Consider revisiting this heuristic.
new_display_item_list_ = DisplayItemList(
current_paint_artifact_->GetDisplayItemList().IsEmpty()
? kInitialDisplayItemListCapacityBytes
: current_paint_artifact_->GetDisplayItemList()
.UsedCapacityInBytes());
}
}
// Set new item state (cache skipping, etc) for the last new display item. // Set new item state (cache skipping, etc) for the last new display item.
void ProcessNewItem(DisplayItem&); void ProcessNewItem(DisplayItem&);
......
...@@ -18,10 +18,6 @@ ...@@ -18,10 +18,6 @@
namespace blink { namespace blink {
TestPaintArtifact::TestPaintArtifact() : display_item_list_(0) {}
TestPaintArtifact::~TestPaintArtifact() = default;
static DisplayItemClient& StaticDummyClient() { static DisplayItemClient& StaticDummyClient() {
DEFINE_STATIC_LOCAL(FakeDisplayItemClient, client, ()); DEFINE_STATIC_LOCAL(FakeDisplayItemClient, client, ());
client.Validate(); client.Validate();
......
...@@ -50,9 +50,6 @@ class TestPaintArtifact { ...@@ -50,9 +50,6 @@ class TestPaintArtifact {
STACK_ALLOCATED(); STACK_ALLOCATED();
public: public:
TestPaintArtifact();
~TestPaintArtifact();
// Add a chunk to the artifact. Each chunk will have a different automatically // Add a chunk to the artifact. Each chunk will have a different automatically
// created client. // created client.
TestPaintArtifact& Chunk() { return Chunk(NewClient()); } TestPaintArtifact& Chunk() { return Chunk(NewClient()); }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment