Commit 8f391791 authored by Andreas Haas's avatar Andreas Haas Committed by Commit Bot

Revert "[arraybuffer] Organize views in a hash_set"

This reverts commit f366e7dd.

Reason for revert: Performance regression, see https://crbug.com/1026763.
The original implementation optimized better for the case where there exists
exactly one view.

Original change's description:
> [arraybuffer] Organize views in a hash_set
>
> This is a first step of getting rid of blink::ArrayBuffer.
>
> blink::ArrayBuffer serves two purposes by now:
> 1) Managing a list of ArrayBufferViews;
> 2) Managing the detached flag;
>
> With this CL I encapsulate the list of ArrayBufferViews. In follow-up
> CLs I then want to pass the list and an ArrayBufferContents object
> to the ArrayBufferView instead of the ArrayBuffer object. This will
> bring us one step closer to removing blink::ArrayBuffer.
>
> Update: ArrayBuffer seems to be very involved. It is the connecting
> data structure between DOMArrayBuffer, ArrayBufferContents, and
> ArrayBufferView. Even though it is not used anymore outside these
> classes, it is not obvious if we can replace it within these classes.
> I still think that refactoring the organization of views here is
> good.
>
> R=​haraken@chromium.org
>
> Bug: chromium:1008840
> Change-Id: I533f5496b7c51312e77a0138ac8249f9f0e55ffd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1911207
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Andreas Haas <ahaas@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#714880}

TBR=haraken@chromium.org,ahaas@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: chromium:1008840, chromium:1026763
Change-Id: Ifad3b3b8cf46290b9bf0f2d4ef958fdd7c414ed4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1930817Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarAndreas Haas <ahaas@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718117}
parent 10230a08
...@@ -30,13 +30,6 @@ ...@@ -30,13 +30,6 @@
namespace blink { namespace blink {
ArrayBuffer::ArrayBuffer(ArrayBufferContents& contents) : is_detached_(false) {
if (contents.IsShared())
contents.ShareWith(contents_);
else
contents.Transfer(contents_);
}
bool ArrayBuffer::Transfer(ArrayBufferContents& result) { bool ArrayBuffer::Transfer(ArrayBufferContents& result) {
DCHECK(!IsShared()); DCHECK(!IsShared());
scoped_refptr<ArrayBuffer> keep_alive(this); scoped_refptr<ArrayBuffer> keep_alive(this);
...@@ -47,19 +40,19 @@ bool ArrayBuffer::Transfer(ArrayBufferContents& result) { ...@@ -47,19 +40,19 @@ bool ArrayBuffer::Transfer(ArrayBufferContents& result) {
} }
bool all_views_are_detachable = true; bool all_views_are_detachable = true;
for (auto* view : views_) { for (ArrayBufferView* i = first_view_; i; i = i->next_view_) {
if (!view->IsDetachable()) { if (!i->IsDetachable())
all_views_are_detachable = false; all_views_are_detachable = false;
}
} }
if (all_views_are_detachable) { if (all_views_are_detachable) {
contents_.Transfer(result); contents_.Transfer(result);
for (auto* view : views_) { while (first_view_) {
view->Detach(); ArrayBufferView* current = first_view_;
RemoveView(current);
current->Detach();
} }
views_.clear();
is_detached_ = true; is_detached_ = true;
} else { } else {
...@@ -103,12 +96,23 @@ bool ArrayBuffer::ShareNonSharedForInternalUse(ArrayBufferContents& result) { ...@@ -103,12 +96,23 @@ bool ArrayBuffer::ShareNonSharedForInternalUse(ArrayBufferContents& result) {
} }
void ArrayBuffer::AddView(ArrayBufferView* view) { void ArrayBuffer::AddView(ArrayBufferView* view) {
views_.insert(view); view->buffer_ = this;
view->prev_view_ = nullptr;
view->next_view_ = first_view_;
if (first_view_)
first_view_->prev_view_ = view;
first_view_ = view;
} }
void ArrayBuffer::RemoveView(ArrayBufferView* view) { void ArrayBuffer::RemoveView(ArrayBufferView* view) {
DCHECK(views_.Contains(view)); DCHECK_EQ(this, view->buffer_.get());
views_.erase(view); if (view->next_view_)
view->next_view_->prev_view_ = view->prev_view_;
if (view->prev_view_)
view->prev_view_->next_view_ = view->next_view_;
if (first_view_ == view)
first_view_ = view->next_view_;
view->prev_view_ = view->next_view_ = nullptr;
} }
} // namespace blink } // namespace blink
...@@ -94,7 +94,7 @@ class CORE_EXPORT ArrayBuffer : public RefCounted<ArrayBuffer> { ...@@ -94,7 +94,7 @@ class CORE_EXPORT ArrayBuffer : public RefCounted<ArrayBuffer> {
~ArrayBuffer() = default; ~ArrayBuffer() = default;
protected: protected:
explicit ArrayBuffer(ArrayBufferContents&); inline explicit ArrayBuffer(ArrayBufferContents&);
private: private:
static inline scoped_refptr<ArrayBuffer> Create( static inline scoped_refptr<ArrayBuffer> Create(
...@@ -113,7 +113,7 @@ class CORE_EXPORT ArrayBuffer : public RefCounted<ArrayBuffer> { ...@@ -113,7 +113,7 @@ class CORE_EXPORT ArrayBuffer : public RefCounted<ArrayBuffer> {
inline unsigned ClampIndex(unsigned index) const; inline unsigned ClampIndex(unsigned index) const;
ArrayBufferContents contents_; ArrayBufferContents contents_;
HashSet<ArrayBufferView*> views_; ArrayBufferView* first_view_;
bool is_detached_; bool is_detached_;
}; };
...@@ -207,6 +207,14 @@ scoped_refptr<ArrayBuffer> ArrayBuffer::CreateShared( ...@@ -207,6 +207,14 @@ scoped_refptr<ArrayBuffer> ArrayBuffer::CreateShared(
return base::AdoptRef(new ArrayBuffer(contents)); return base::AdoptRef(new ArrayBuffer(contents));
} }
ArrayBuffer::ArrayBuffer(ArrayBufferContents& contents)
: first_view_(nullptr), is_detached_(false) {
if (contents.IsShared())
contents.ShareWith(contents_);
else
contents.Transfer(contents_);
}
void* ArrayBuffer::Data() { void* ArrayBuffer::Data() {
return contents_.Data(); return contents_.Data();
} }
......
...@@ -33,7 +33,9 @@ ArrayBufferView::ArrayBufferView(scoped_refptr<ArrayBuffer> buffer, ...@@ -33,7 +33,9 @@ ArrayBufferView::ArrayBufferView(scoped_refptr<ArrayBuffer> buffer,
size_t byte_offset) size_t byte_offset)
: byte_offset_(byte_offset), : byte_offset_(byte_offset),
is_detachable_(true), is_detachable_(true),
buffer_(std::move(buffer)) { buffer_(std::move(buffer)),
prev_view_(nullptr),
next_view_(nullptr) {
base_address_ = base_address_ =
buffer_ ? (static_cast<char*>(buffer_->DataMaybeShared()) + byte_offset_) buffer_ ? (static_cast<char*>(buffer_->DataMaybeShared()) + byte_offset_)
: nullptr; : nullptr;
......
...@@ -89,6 +89,8 @@ class CORE_EXPORT ArrayBufferView : public RefCounted<ArrayBufferView> { ...@@ -89,6 +89,8 @@ class CORE_EXPORT ArrayBufferView : public RefCounted<ArrayBufferView> {
private: private:
friend class ArrayBuffer; friend class ArrayBuffer;
scoped_refptr<ArrayBuffer> buffer_; scoped_refptr<ArrayBuffer> buffer_;
ArrayBufferView* prev_view_;
ArrayBufferView* next_view_;
}; };
} // namespace blink } // namespace blink
......
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