Skip to content

Commit

Permalink
blink: removes ElementRecord and folds into stackitem
Browse files Browse the repository at this point in the history 
This simplifies some of the consuming code, in so far as it doesn't
need to iterate over the ElementRecords, but can instead iterate over
the HTMLStackItems, which is really what it wants.

Doing this avoids an allocation per element, and gives a slight
win on some benchmarks. ~.5% on the Vanilla tests. Overall
speedometer score doesn't doesn't move in a statistically
significant way. See last two pinpoint runs for details.

Bug: 1406507


Change-Id: I9f855913801ff7a59d2e734e7c66e5dd2071b632
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4179378
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096520}
  • Loading branch information
Scott Violet authored and Chromium LUCI CQ committed Jan 25, 2023
1 parent 7dc6207 commit e318c2a
Show file tree
Hide file tree
Showing 6 changed files with 162 additions and 190 deletions.
34 changes: 12 additions & 22 deletions third_party/blink/renderer/core/html/parser/html_construction_site.cc
View file
Expand Up @@ -831,8 +831,7 @@ void HTMLConstructionSite::InsertHTMLTemplateElement(
// TODO(crbug.com/1063157): Add an attribute for imperative slot
// assignment.
auto slot_assignment_mode = SlotAssignmentMode::kNamed;
HTMLStackItem* shadow_host_stack_item =
open_elements_.TopRecord()->StackItem();
HTMLStackItem* shadow_host_stack_item = open_elements_.TopStackItem();
Element* host = shadow_host_stack_item->GetElement();
ShadowRootType type = declarative_shadow_root_type ==
DeclarativeShadowRootType::kStreamingOpen
Expand Down Expand Up @@ -884,7 +883,7 @@ void HTMLConstructionSite::InsertFormattingElement(AtomicHTMLToken* token) {
// Possible active formatting elements include:
// a, b, big, code, em, font, i, nobr, s, small, strike, strong, tt, and u.
InsertHTMLElement(token);
active_formatting_elements_.Append(CurrentElementRecord()->StackItem());
active_formatting_elements_.Append(CurrentStackItem());
}

void HTMLConstructionSite::InsertScriptElement(AtomicHTMLToken* token) {
Expand Down Expand Up @@ -963,25 +962,16 @@ void HTMLConstructionSite::InsertTextNode(const StringView& string,
whitespace_mode);
}

void HTMLConstructionSite::Reparent(HTMLElementStack::ElementRecord* new_parent,
HTMLElementStack::ElementRecord* child) {
HTMLConstructionSiteTask task(HTMLConstructionSiteTask::kReparent);
task.parent = new_parent->GetNode();
task.child = child->GetNode();
QueueTask(task, true);
}

void HTMLConstructionSite::Reparent(HTMLElementStack::ElementRecord* new_parent,
void HTMLConstructionSite::Reparent(HTMLStackItem* new_parent,
HTMLStackItem* child) {
HTMLConstructionSiteTask task(HTMLConstructionSiteTask::kReparent);
task.parent = new_parent->GetNode();
task.child = child->GetNode();
QueueTask(task, true);
}

void HTMLConstructionSite::InsertAlreadyParsedChild(
HTMLStackItem* new_parent,
HTMLElementStack::ElementRecord* child) {
void HTMLConstructionSite::InsertAlreadyParsedChild(HTMLStackItem* new_parent,
HTMLStackItem* child) {
if (new_parent->CausesFosterParenting()) {
FosterParent(child->GetNode());
return;
Expand All @@ -994,9 +984,8 @@ void HTMLConstructionSite::InsertAlreadyParsedChild(
QueueTask(task, true);
}

void HTMLConstructionSite::TakeAllChildren(
HTMLStackItem* new_parent,
HTMLElementStack::ElementRecord* old_parent) {
void HTMLConstructionSite::TakeAllChildren(HTMLStackItem* new_parent,
HTMLStackItem* old_parent) {
HTMLConstructionSiteTask task(HTMLConstructionSiteTask::kTakeAllChildren);
task.parent = new_parent->GetNode();
task.child = old_parent->GetNode();
Expand Down Expand Up @@ -1265,15 +1254,16 @@ bool HTMLConstructionSite::InQuirksMode() {
// https://html.spec.whatwg.org/C/#appropriate-place-for-inserting-a-node
void HTMLConstructionSite::FindFosterSite(HTMLConstructionSiteTask& task) {
// 2.1
HTMLElementStack::ElementRecord* last_template =
HTMLStackItem* last_template =
open_elements_.Topmost(html_names::HTMLTag::kTemplate);

// 2.2
HTMLElementStack::ElementRecord* last_table =
HTMLStackItem* last_table =
open_elements_.Topmost(html_names::HTMLTag::kTable);

// 2.3
if (last_template && (!last_table || last_template->IsAbove(last_table))) {
if (last_template &&
(!last_table || last_template->IsAboveItemInStack(last_table))) {
task.parent = last_template->GetElement();
return;
}
Expand All @@ -1293,7 +1283,7 @@ void HTMLConstructionSite::FindFosterSite(HTMLConstructionSiteTask& task) {
}

// 2.6, 2.7
task.parent = last_table->Next()->GetElement();
task.parent = last_table->NextItemInStack()->GetElement();
}

bool HTMLConstructionSite::ShouldFosterParent() const {
Expand Down
13 changes: 3 additions & 10 deletions third_party/blink/renderer/core/html/parser/html_construction_site.h
View file
Expand Up @@ -163,18 +163,14 @@ class HTMLConstructionSite final {
void InsertHTMLHtmlStartTagInBody(AtomicHTMLToken*);
void InsertHTMLBodyStartTagInBody(AtomicHTMLToken*);

void Reparent(HTMLElementStack::ElementRecord* new_parent,
HTMLElementStack::ElementRecord* child);
void Reparent(HTMLElementStack::ElementRecord* new_parent,
HTMLStackItem* child);
void Reparent(HTMLStackItem* new_parent, HTMLStackItem* child);
// insertAlreadyParsedChild assumes that |child| has already been parsed
// (i.e., we're just moving it around in the tree rather than parsing it for
// the first time). That means this function doesn't call beginParsingChildren
// / finishParsingChildren.
void InsertAlreadyParsedChild(HTMLStackItem* new_parent,
HTMLElementStack::ElementRecord* child);
void TakeAllChildren(HTMLStackItem* new_parent,
HTMLElementStack::ElementRecord* old_parent);
HTMLStackItem* child);
void TakeAllChildren(HTMLStackItem* new_parent, HTMLStackItem* old_parent);

HTMLStackItem* CreateElementFromSavedToken(HTMLStackItem*);

Expand All @@ -191,9 +187,6 @@ class HTMLConstructionSite final {
bool InQuirksMode();

bool IsEmpty() const { return !open_elements_.StackDepth(); }
HTMLElementStack::ElementRecord* CurrentElementRecord() const {
return open_elements_.TopRecord();
}
Element* CurrentElement() const { return open_elements_.Top(); }
ContainerNode* CurrentNode() const { return open_elements_.TopNode(); }
HTMLStackItem* CurrentStackItem() const {
Expand Down
147 changes: 69 additions & 78 deletions third_party/blink/renderer/core/html/parser/html_element_stack.cc
View file
Expand Up @@ -174,40 +174,14 @@ inline bool IsSelectScopeMarker(HTMLStackItem* item) {

} // namespace

HTMLElementStack::ElementRecord::ElementRecord(HTMLStackItem* item,
ElementRecord* next)
: item_(item), next_(next) {
DCHECK(item_);
}

void HTMLElementStack::ElementRecord::ReplaceElement(HTMLStackItem* item) {
DCHECK(item);
DCHECK(!item_ || item_->IsElementNode());
// FIXME: Should this call finishParsingChildren?
item_ = item;
}

bool HTMLElementStack::ElementRecord::IsAbove(ElementRecord* other) const {
for (ElementRecord* below = Next(); below; below = below->Next()) {
if (below == other)
return true;
}
return false;
}

void HTMLElementStack::ElementRecord::Trace(Visitor* visitor) const {
visitor->Trace(item_);
visitor->Trace(next_);
}

HTMLElementStack::HTMLElementStack()
: root_node_(nullptr),
head_element_(nullptr),
body_element_(nullptr),
stack_depth_(0) {}

bool HTMLElementStack::HasOnlyOneElement() const {
return !TopRecord()->Next();
return !TopStackItem()->NextItemInStack();
}

bool HTMLElementStack::SecondElementIsHTMLBodyElement() const {
Expand Down Expand Up @@ -246,7 +220,7 @@ void HTMLElementStack::PopAll() {
if (auto* select = DynamicTo<HTMLSelectElement>(node))
select->SetBlocksFormSubmission(true);
}
top_ = top_->ReleaseNext();
top_ = top_->ReleaseNextItemInStack();
}
}

Expand Down Expand Up @@ -381,44 +355,42 @@ void HTMLElementStack::Push(HTMLStackItem* item) {
}

void HTMLElementStack::InsertAbove(HTMLStackItem* item,
ElementRecord* record_below) {
HTMLStackItem* item_below) {
DCHECK(!item->NextItemInStack());
DCHECK(item);
DCHECK(record_below);
DCHECK(item_below);
DCHECK(top_);
DCHECK(!item->HasTagName(html_names::kHTMLTag));
DCHECK(!item->HasTagName(html_names::kHeadTag));
DCHECK(!item->HasTagName(html_names::kBodyTag));
DCHECK(root_node_);
if (record_below == top_) {
if (item_below == top_) {
Push(item);
return;
}

for (ElementRecord* record_above = top_.Get(); record_above;
record_above = record_above->Next()) {
if (record_above->Next() != record_below)
for (HTMLStackItem* item_above = top_.Get(); item_above;
item_above = item_above->NextItemInStack()) {
if (item_above->NextItemInStack() != item_below) {
continue;
}

stack_depth_++;
record_above->SetNext(
MakeGarbageCollected<ElementRecord>(item, record_above->ReleaseNext()));
record_above->Next()->GetElement()->BeginParsingChildren();
item->SetNextItemInStack(item_above->ReleaseNextItemInStack());
item_above->SetNextItemInStack(item);
item->GetElement()->BeginParsingChildren();
return;
}
NOTREACHED();
}

HTMLElementStack::ElementRecord* HTMLElementStack::TopRecord() const {
DCHECK(top_);
return top_.Get();
}

HTMLStackItem* HTMLElementStack::OneBelowTop() const {
// We should never call this if there are fewer than 2 elements on the stack.
DCHECK(top_);
DCHECK(top_->Next());
if (top_->Next()->StackItem()->IsElementNode())
return top_->Next()->StackItem();
DCHECK(top_->NextItemInStack());
if (top_->NextItemInStack()->IsElementNode()) {
return top_->NextItemInStack();
}
return nullptr;
}

Expand All @@ -441,24 +413,22 @@ void HTMLElementStack::Remove(Element* element) {
RemoveNonTopCommon(element);
}

HTMLElementStack::ElementRecord* HTMLElementStack::Find(
Element* element) const {
for (ElementRecord* pos = top_.Get(); pos; pos = pos->Next()) {
if (pos->GetNode() == element)
return pos;
HTMLStackItem* HTMLElementStack::Find(Element* element) const {
for (HTMLStackItem* item = top_.Get(); item; item = item->NextItemInStack()) {
if (item->GetNode() == element) {
return item;
}
}
return nullptr;
}

HTMLElementStack::ElementRecord* HTMLElementStack::Topmost(
html_names::HTMLTag tag) const {
HTMLStackItem* HTMLElementStack::Topmost(html_names::HTMLTag tag) const {
// kUnknown by itself is not enough to uniquely a tag. This code should only
// be called with HTMLTags other than kUnknown.
DCHECK_NE(tag, HTMLTag::kUnknown);
for (ElementRecord* pos = top_.Get(); pos; pos = pos->Next()) {
if (pos->StackItem()->IsHTMLNamespace() &&
tag == pos->StackItem()->GetHTMLTag()) {
return pos;
for (HTMLStackItem* item = top_.Get(); item; item = item->NextItemInStack()) {
if (item->IsHTMLNamespace() && tag == item->GetHTMLTag()) {
return item;
}
}
return nullptr;
Expand All @@ -469,13 +439,11 @@ bool HTMLElementStack::Contains(Element* element) const {
}

template <bool isMarker(HTMLStackItem*)>
bool InScopeCommon(HTMLElementStack::ElementRecord* top,
html_names::HTMLTag tag) {
bool InScopeCommon(HTMLStackItem* top, html_names::HTMLTag tag) {
// kUnknown by itself is not enough to uniquely a tag. This code should only
// be called with HTMLTags other than kUnknown.
DCHECK_NE(HTMLTag::kUnknown, tag);
for (HTMLElementStack::ElementRecord* pos = top; pos; pos = pos->Next()) {
HTMLStackItem* item = pos->StackItem();
for (HTMLStackItem* item = top; item; item = item->NextItemInStack()) {
if (tag == item->GetHTMLTag() && item->IsHTMLNamespace())
return true;
if (isMarker(item))
Expand All @@ -486,8 +454,7 @@ bool InScopeCommon(HTMLElementStack::ElementRecord* top,
}

bool HTMLElementStack::HasNumberedHeaderElementInScope() const {
for (ElementRecord* record = top_.Get(); record; record = record->Next()) {
HTMLStackItem* item = record->StackItem();
for (HTMLStackItem* item = top_.Get(); item; item = item->NextItemInStack()) {
if (item->IsNumberedHeaderElement())
return true;
if (IsScopeMarker(item))
Expand All @@ -498,8 +465,7 @@ bool HTMLElementStack::HasNumberedHeaderElementInScope() const {
}

bool HTMLElementStack::InScope(Element* target_element) const {
for (ElementRecord* pos = top_.Get(); pos; pos = pos->Next()) {
HTMLStackItem* item = pos->StackItem();
for (HTMLStackItem* item = top_.Get(); item; item = item->NextItemInStack()) {
if (item->GetNode() == target_element)
return true;
if (IsScopeMarker(item))
Expand Down Expand Up @@ -557,15 +523,16 @@ void HTMLElementStack::PushCommon(HTMLStackItem* item) {
DCHECK(root_node_);

stack_depth_++;
top_ = MakeGarbageCollected<ElementRecord>(item, top_.Release());
item->SetNextItemInStack(top_.Release());
top_ = item;
}

void HTMLElementStack::PopCommon() {
DCHECK(!TopStackItem()->HasTagName(html_names::kHTMLTag));
DCHECK(!TopStackItem()->HasTagName(html_names::kHeadTag) || !head_element_);
DCHECK(!TopStackItem()->HasTagName(html_names::kBodyTag) || !body_element_);
Top()->FinishParsingChildren();
top_ = top_->ReleaseNext();
top_ = top_->ReleaseNextItemInStack();

stack_depth_--;
}
Expand All @@ -574,33 +541,56 @@ void HTMLElementStack::RemoveNonTopCommon(Element* element) {
DCHECK(!IsA<HTMLHtmlElement>(element));
DCHECK(!IsA<HTMLBodyElement>(element));
DCHECK_NE(Top(), element);
for (ElementRecord* pos = top_.Get(); pos; pos = pos->Next()) {
if (pos->Next()->GetElement() == element) {
for (HTMLStackItem* item = top_.Get(); item; item = item->NextItemInStack()) {
if (item->NextItemInStack()->GetElement() == element) {
// FIXME: Is it OK to call finishParsingChildren()
// when the children aren't actually finished?
element->FinishParsingChildren();
pos->SetNext(pos->Next()->ReleaseNext());
item->SetNextItemInStack(
item->ReleaseNextItemInStack()->ReleaseNextItemInStack());
stack_depth_--;
return;
}
}
NOTREACHED();
}

HTMLElementStack::ElementRecord*
HTMLElementStack::FurthestBlockForFormattingElement(
HTMLStackItem* HTMLElementStack::FurthestBlockForFormattingElement(
Element* formatting_element) const {
ElementRecord* furthest_block = nullptr;
for (ElementRecord* pos = top_.Get(); pos; pos = pos->Next()) {
if (pos->GetElement() == formatting_element)
HTMLStackItem* furthest_block = nullptr;
for (HTMLStackItem* item = top_.Get(); item; item = item->NextItemInStack()) {
if (item->GetElement() == formatting_element) {
return furthest_block;
if (pos->StackItem()->IsSpecialNode())
furthest_block = pos;
}
if (item->IsSpecialNode()) {
furthest_block = item;
}
}
NOTREACHED();
return nullptr;
}

void HTMLElementStack::Replace(HTMLStackItem* old_item,
HTMLStackItem* new_item) {
DCHECK(old_item);
DCHECK(new_item);
DCHECK(!new_item->NextItemInStack());
HTMLStackItem* previous_item = nullptr;
for (HTMLStackItem* item = top_.Get(); item; item = item->NextItemInStack()) {
if (item == old_item) {
if (previous_item) {
previous_item->ReleaseNextItemInStack();
previous_item->SetNextItemInStack(new_item);
}
new_item->SetNextItemInStack(old_item->ReleaseNextItemInStack());
return;
}
previous_item = item;
}
// This should only be called with items in the stack.
NOTREACHED();
}

void HTMLElementStack::Trace(Visitor* visitor) const {
visitor->Trace(top_);
visitor->Trace(root_node_);
Expand All @@ -611,8 +601,9 @@ void HTMLElementStack::Trace(Visitor* visitor) const {
#ifndef NDEBUG

void HTMLElementStack::Show() {
for (ElementRecord* record = top_.Get(); record; record = record->Next())
LOG(INFO) << *record->GetElement();
for (HTMLStackItem* item = top_.Get(); item; item = item->NextItemInStack()) {
LOG(INFO) << *item->GetElement();
}
}

#endif
Expand Down

0 comments on commit e318c2a

Please sign in to comment.