Faster manual slot assignment [chromium/src : main]
Aleks Totic (Gerrit)
Attention is currently required from: Mason Freed.
Aleks Totic would like Mason Freed to review this change.
Faster manual slot assignment
Fix for 30% regression in
linux-perf/blink_perf.shadow_dom/imperative-api-detail-summary-large/imperative-api-detail-summary-large.html
Do not assign slots that have already been assigned.
Bug: 1251041
Change-Id: Ia500af5f34acfae5e07472e7ecf0c7b0c9e89289
---
M third_party/blink/renderer/core/html/html_details_element.cc
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/third_party/blink/renderer/core/html/html_details_element.cc b/third_party/blink/renderer/core/html/html_details_element.cc
index d6c4b93..92505acb 100644
--- a/third_party/blink/renderer/core/html/html_details_element.cc
+++ b/third_party/blink/renderer/core/html/html_details_element.cc
@@ -161,14 +161,14 @@
if (!child.IsSlotable())
continue;
if (IsFirstSummary(child)) {
- summary_nodes.push_back(child);
+ if (child.ManuallyAssignedSlot() != summary_slot)
+ summary_nodes.push_back(child);
} else {
- content_nodes.push_back(child);
+ if (child.ManuallyAssignedSlot() != content_slot)
+ content_nodes.push_back(child);
}
}
- summary_slot->ClearAssignedNodes();
summary_slot->Assign(summary_nodes);
- content_slot->ClearAssignedNodes();
content_slot->Assign(content_nodes);
}
To view, visit change 3184021. To unsubscribe, or for help writing mail filters, visit settings.
Aleks Totic (Gerrit)
Attention is currently required from: Mason Freed.
1 comment:
Patchset:
ptal
looks like a small fix will fix the regression
To view, visit change 3184021. To unsubscribe, or for help writing mail filters, visit settings.
Mason Freed (Gerrit)
Attention is currently required from: Aleks Totic.
1 comment:
Patchset:
Hmm, the code looks good, but from the tests, it broke the <details>. The summary element is wrong. I don't see the bug...
To view, visit change 3184021. To unsubscribe, or for help writing mail filters, visit settings.
Aleks Totic (Gerrit)
Attention is currently required from: Mason Freed.
1 comment:
Patchset:
Hmm, the code looks good, but from the tests, it broke the <details>. The summary element is wrong. […]
It turns out code wrong, I misunderstood Assign() API.
Now that I've fixed it, it is only 2ms faster.
I needed to make up 11ms for parity.My current hypothesis is that manually assigned nodes might be
inherently slower.My next step is to do a pprof profile and see where the time goes.
To view, visit change 3184021. To unsubscribe, or for help writing mail filters, visit settings.
Mason Freed (Gerrit)
Attention is currently required from: Aleks Totic.
To view, visit change 3184021. To unsubscribe, or for help writing mail filters, visit settings.
Aleks Totic (Gerrit)
Aleks Totic abandoned this change.
To view, visit change 3184021. To unsubscribe, or for help writing mail filters, visit settings.