Skip to content

Commit f997d62

Browse files
rakudramacommit-bot@chromium.org
authored andcommitted
[html] Better code for Element / _ChildrenElementList method
Two tricks let us compile this more efficiently: e.children.addAll(cs); Normally, get$children is inlined, returning a _ChildrenElementList wrapper, generating: new W._ChildrenElementList(e, e.children).addAll$1(0, cs); The two tricks: 1. Split _ChildElementList.addAll into an 'unwrap' that then calls the logic in '_addAll' 2. Add information about the properties of e.children that allow it to be removed. With these tricks, dart2js can optimize the code to this version that avoids allocating a wrapper or accessing the 'children' property: W._ChildrenElementList__addAll(e, cs); Change-Id: Ifdf533ac4f9790f09f87302e67304b5696097266 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153904 Reviewed-by: Srujan Gaddam <[email protected]> Commit-Queue: Stephen Adams <[email protected]>
1 parent fdf4e36 commit f997d62

3 files changed

Lines changed: 44 additions & 8 deletions

File tree

sdk/lib/html/dart2js/html_dart2js.dart

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11722,6 +11722,10 @@ class _ChildrenElementList extends ListBase<Element>
1172211722
Iterator<Element> get iterator => toList().iterator;
1172311723

1172411724
void addAll(Iterable<Element> iterable) {
11725+
_addAll(_element, iterable);
11726+
}
11727+
11728+
static void _addAll(Element _element, Iterable<Element> iterable) {
1172511729
if (iterable is _ChildNodeListLazy) {
1172611730
iterable = new List.from(iterable);
1172711731
}
@@ -11775,6 +11779,10 @@ class _ChildrenElementList extends ListBase<Element>
1177511779
}
1177611780

1177711781
bool remove(Object? object) {
11782+
return _remove(_element, object);
11783+
}
11784+
11785+
static bool _remove(Element _element, Object? object) {
1177811786
if (object is Element) {
1177911787
Element element = object;
1178011788
if (identical(element.parentNode, _element)) {
@@ -11823,7 +11831,10 @@ class _ChildrenElementList extends ListBase<Element>
1182311831
return result;
1182411832
}
1182511833

11826-
Element get first {
11834+
Element get first => _first(_element);
11835+
11836+
@pragma('dart2js:noInline')
11837+
static Element _first(Element _element) {
1182711838
Element? result = _element._firstElementChild;
1182811839
if (result == null) throw new StateError("No elements");
1182911840
return result;
@@ -12965,6 +12976,16 @@ class Element extends Node
1296512976
*/
1296612977
List<Element> get children => new _ChildrenElementList._wrap(this);
1296712978

12979+
List<Node> get _children =>
12980+
// Element.children always returns the same list-like object which is a
12981+
// live view on the underlying DOM tree. So we can GVN it and remove it if
12982+
// unused.
12983+
JS(
12984+
'returns:HtmlCollection;creates:HtmlCollection;'
12985+
'depends:none;effects:none;gvn:true',
12986+
'#.children',
12987+
this);
12988+
1296812989
set children(List<Element> value) {
1296912990
// Copy list first since we don't want liveness during iteration.
1297012991
var copy = value.toList();
@@ -14824,11 +14845,6 @@ class Element extends Node
1482414845
@JSName('childElementCount')
1482514846
int get _childElementCount native;
1482614847

14827-
@JSName('children')
14828-
@Returns('HtmlCollection')
14829-
@Creates('HtmlCollection')
14830-
List<Node> get _children native;
14831-
1483214848
@JSName('firstElementChild')
1483314849
Element? get _firstElementChild native;
1483414850

tools/dom/scripts/htmlrenamer.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,6 @@ def GetDDC_Extension(interface, operationName):
387387
# Not prefixed but requires custom implementation for cross-browser compatibility.
388388
'Document.visibilityState',
389389
'Element.animate',
390-
'Element.children',
391390
'Element.childElementCount',
392391
'Element.firstElementChild',
393392
'Element.getClientRects',
@@ -791,6 +790,7 @@ def convertedFutureMembers(member):
791790
'DOMException.WRONG_DOCUMENT_ERR',
792791
'Element.accessKey',
793792
'Element.append',
793+
'Element.children',
794794
'Element.dataset',
795795
'Element.get:classList',
796796
'Element.getAttributeNode',

tools/dom/templates/html/impl/impl_Element.darttemplate

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ class _ChildrenElementList extends ListBase<Element>
4646
Iterator<Element> get iterator => toList().iterator;
4747

4848
void addAll(Iterable<Element> iterable) {
49+
_addAll(_element, iterable);
50+
}
51+
52+
static void _addAll(Element _element, Iterable<Element> iterable) {
4953
if (iterable is _ChildNodeListLazy) {
5054
iterable = new List.from(iterable);
5155
}
@@ -99,6 +103,10 @@ class _ChildrenElementList extends ListBase<Element>
99103
}
100104

101105
bool remove(Object$NULLABLE object) {
106+
return _remove(_element, object);
107+
}
108+
109+
static bool _remove(Element _element, Object$NULLABLE object) {
102110
if (object is Element) {
103111
Element element = object;
104112
if (identical(element.parentNode, _element)) {
@@ -149,7 +157,10 @@ class _ChildrenElementList extends ListBase<Element>
149157
return result;
150158
}
151159

152-
Element get first {
160+
Element get first => _first(_element);
161+
162+
@pragma('dart2js:noInline')
163+
static Element _first(Element _element) {
153164
Element$NULLABLE result = _element._firstElementChild;
154165
if (result == null) throw new StateError("No elements");
155166
return result;
@@ -667,6 +678,15 @@ $endif
667678
*/
668679
List<Element> get children => new _ChildrenElementList._wrap(this);
669680

681+
List<Node> get _children =>
682+
// Element.children always returns the same list-like object which is a
683+
// live view on the underlying DOM tree. So we can GVN it and remove it if
684+
// unused.
685+
JS('returns:HtmlCollection;creates:HtmlCollection;'
686+
'depends:none;effects:none;gvn:true',
687+
'#.children',
688+
this);
689+
670690
set children(List<Element> value) {
671691
// Copy list first since we don't want liveness during iteration.
672692
var copy = value.toList();

0 commit comments

Comments
 (0)