Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1995203002:
Rewrite Shadow DOM distribution engine to support partial synchronous distribution for v1 (Closed)
|
Created:
4 years, 7 months ago by hayato Modified:
4 years, 6 months ago CC:
blink-reviews, blink-reviews-dom_chromium.org, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink, eae+blinkwatch, rwlbuis, sof, webcomponents-bugzilla_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionRewrite Shadow DOM distribution engine to support partial synchronous distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slot| HashMap, via DocumentOrderedMap.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes, m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 1.8x faster
Time:
Before: avg 3190.3 runs/s
After: avg 5885.6 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.0x faster
Time:
Before: avg 51.6 runs/s
After: avg 306.4 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.4x faster
Time:
Before: avg 1647.4 runs/s
After: avg 5645.0 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107
- This CL also fixes the following bugs. These tests no longer fail.
- crbug.com/610588
- imported/wpt/shadow-dom/HTMLSlotElement-interface.html
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedMap might not be optimized for our purpose.
e.g. DocumentOrderedMap has to travers DOM Tree if conflicts happen.
Just using Document::compareDocumentPosition() might be better, given that
conflicts is unlikely to happen.
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990, 595287, 610588, 610961, 579107
Committed: https://crrev.com/6310c024c916f3f7ea89eb3ffabbc94e59a3c29a
Cr-Commit-Position: refs/heads/master@{#396443}
Patch Set 1 #Patch Set 2 : wip #
Total comments: 59
Patch Set 3 : Use DocumentOrderedMap #Patch Set 4 : clean up #
Total comments: 8
Patch Set 5 : fixed #Patch Set 6 : Update HTMLSlotElement::getDistributedNodes() for a document tree #Patch Set 7 : No longer FAIL: imported/wpt/shadow-dom/HTMLSlotElement-interface.html #Messages
Total messages: 74 (38 generated)
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1995203002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1995203002/1
Description was changed from
==========
Rewrite Shadow DOM distriubtion engine
BUG=
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Got a significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree.
The HashMap is updated in each insertion/removal/renaming of slots.
- DOM Standard [2] requires synchronous calculation of slot assignments in
each DOM mutation to fire a slotchagne event,
however, as an optimization, we do not calculate it synchronously if we can
be sure to delay it.
Minimum check is done for detecting slotchange, using the characteristics of
slot assignments.
- blink::SlotAssignment class no longer needs member variables, such as
m_oldDistributedNdoes, m_oldAssingedNodes, m_fallbackNodes.
- SlotAssignment::resolveDistribution became much simpler because slotchange is
already detected at eary stage.
See the bug 610961 for basic ideas in detail.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being makred skipped. See crbug.com/579107.
This CL also fixed underwater bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive for slotchange event. e.g. A preceding slot is
inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
play a performance penalty to remove this kind of false-positive.
- Add more layout tests.
- Add W3C Web Platform tests and upstream it.
- Optimize the performance more and more.
e.g. We could optimize SlotAssignment::aaa, which is not caluculatee
synchrously, to detect slotchange exactly.
- Support SlotAssignment even it is non-shadow tree. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate a distribution flag for v0 and v1. Currently, the same flag is used in
both kinds of shadow trees, v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose. e.g. it used
compareDocumentPosition, which
Make our own version or make it template it. e.g. DocumentOrderedList<T>,
specialize it for our purpose.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Got a significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree.
The HashMap is updated in each insertion/removal/renaming of slots.
- DOM Standard [2] requires synchronous calculation of slot assignments in
each DOM mutation to fire a slotchagne event,
however, as an optimization, we do not calculate it synchronously if we can
be sure to delay it.
Minimum check is done for detecting slotchange, using the characteristics of
slot assignments.
- blink::SlotAssignment class no longer needs member variables, such as
m_oldDistributedNdoes, m_oldAssingedNodes, m_fallbackNodes.
- SlotAssignment::resolveDistribution became much simpler because slotchange is
already detected at eary stage.
See the bug 610961 for basic ideas in detail.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being makred skipped. See crbug.com/579107.
This CL also fixed underwater bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive for slotchange event. e.g. A preceding slot is
inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
play a performance penalty to remove this kind of false-positive.
- Add more layout tests.
- Add W3C Web Platform tests and upstream it.
- Optimize the performance more and more.
e.g. We could optimize SlotAssignment::aaa, which is not caluculatee
synchrously, to detect slotchange exactly.
- Support SlotAssignment even it is non-shadow tree. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate a distribution flag for v0 and v1. Currently, the same flag is used in
both kinds of shadow trees, v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose. e.g. it used
compareDocumentPosition, which
Make our own version or make it template it. e.g. DocumentOrderedList<T>,
specialize it for our purpose.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Got a significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree.
The HashMap is updated in each insertion/removal/renaming of slots.
- DOM Standard [2] requires synchronous calculation of slot assignments in
each DOM mutation to fire a slotchagne event,
however, as an optimization, we do not calculate it synchronously if we can
be sure to delay it.
Minimum check is done for detecting slotchange, using the characteristics of
slot assignments.
- HTMLSlotElement becomes much smaller footprint element.
- The following member variables are gone:
m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
- SlotAssignment::resolveDistribution became much simpler because slotchange is
already detected at early stage.
See the bug 610961 for basic ideas in detail.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
This CL also fixed underwater bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive for slotchange event. e.g. A preceding slot is
inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
play a performance penalty to remove this kind of false-positive.
- Add more layout tests.
- Add W3C Web Platform tests and upstream it.
- Optimize the performance more and more.
e.g. We could optimize SlotAssignment::aaa, which is calculated synchronously,
to detect slotchange exactly.
- Support SlotAssignment even it is non-shadow tree. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate a distribution flag for v0 and v1. Currently, the same flag is used in
both kinds of shadow trees, v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose. e.g. it used
compareDocumentPosition, which
Make our own version or make it template it. e.g. DocumentOrderedList<T>,
specialize it for our purpose.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Got a significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree.
The HashMap is updated in each insertion/removal/renaming of slots.
- DOM Standard [2] requires synchronous calculation of slot assignments in
each DOM mutation to fire a slotchagne event,
however, as an optimization, we do not calculate it synchronously if we can
be sure to delay it.
Minimum check is done for detecting slotchange, using the characteristics of
slot assignments.
- HTMLSlotElement becomes much smaller footprint element.
- The following member variables are gone:
m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
- SlotAssignment::resolveDistribution became much simpler because slotchange is
already detected at early stage.
See the bug 610961 for basic ideas in detail.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
This CL also fixed underwater bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive for slotchange event. e.g. A preceding slot is
inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
play a performance penalty to remove this kind of false-positive.
- Add more layout tests.
- Add W3C Web Platform tests and upstream it.
- Optimize the performance more and more.
e.g. We could optimize SlotAssignment::aaa, which is calculated synchronously,
to detect slotchange exactly.
- Support SlotAssignment even it is non-shadow tree. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate a distribution flag for v0 and v1. Currently, the same flag is used in
both kinds of shadow trees, v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose. e.g. it used
compareDocumentPosition, which
Make our own version or make it template it. e.g. DocumentOrderedList<T>,
specialize it for our purpose.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Got significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree.
The HashMap is updated in each insertion/removal/renaming of slots.
- DOM Standard [2] requires synchronous calculation of slot assignments in
each DOM mutation to fire a slotchagne event,
however, as an optimization, we do not calculate it synchronously if we can
be sure to delay it.
Minimum check is done for detecting slotchange, using the characteristics of
slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
- HTMLSlotElement becomes much smaller footprint element.
- The following member variables are gone:
m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
- SlotAssignment::resolveDistribution became much simpler because slotchange is
already detected at early stage.
See the bug 610961 for basic ideas in detail.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
This CL also fixed underwater bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive for slotchange event. e.g. A preceding slot is
inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
play a performance penalty to remove this kind of false-positive.
- Add more layout tests.
- Add W3C Web Platform tests and upstream it.
- Optimize the performance more and more.
e.g. We could optimize SlotAssignment::aaa, which is calculated synchronously,
to detect slotchange exactly.
- Support SlotAssignment even it is non-shadow tree. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate a distribution flag for v0 and v1. Currently, the same flag is used in
both kinds of shadow trees, v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose. e.g. it used
compareDocumentPosition, which
Make our own version or make it template it. e.g. DocumentOrderedList<T>,
specialize it for our purpose.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Got significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree.
The HashMap is updated in each insertion/removal/renaming of slots.
- DOM Standard [2] requires synchronous calculation of slot assignments in
each DOM mutation to fire a slotchagne event,
however, as an optimization, we do not calculate it synchronously if we can
be sure to delay it.
Minimum check is done for detecting slotchange, using the characteristics of
slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
- HTMLSlotElement becomes much smaller footprint element.
- The following member variables are gone:
m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
- SlotAssignment::resolveDistribution became much simpler because slotchange is
already detected at early stage.
See the bug 610961 for basic ideas in detail.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
This CL also fixed underwater bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive for slotchange event. e.g. A preceding slot is
inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
play a performance penalty to remove this kind of false-positive.
- Add more layout tests.
- Add W3C Web Platform tests and upstream it.
- Optimize the performance more and more.
e.g. We could optimize SlotAssignment::aaa, which is calculated synchronously,
to detect slotchange exactly.
- Support SlotAssignment even it is non-shadow tree. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate a distribution flag for v0 and v1. Currently, the same flag is used in
both kinds of shadow trees, v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose. e.g. it used
compareDocumentPosition, which
Make our own version or make it template it. e.g. DocumentOrderedList<T>,
specialize it for our purpose.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Got significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree.
The HashMap is updated in each insertion/removal/renaming of slots.
- DOM Standard [2] requires synchronous calculation of slot assignments in
each DOM mutation to fire a slotchagne event,
however, as an optimization, we do not calculate it synchronously if we can
be sure to delay it.
Minimum check is done for detecting slotchange, using the characteristics of
slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
- HTMLSlotElement becomes much smaller footprint element.
- The following member variables are gone:
m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
- SlotAssignment::resolveDistribution became much simpler because slotchange is
already detected at early stage.
See the details in bug 610961 for basic ideas behind the scenes.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
This CL also fixed underwater bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive for slotchange event. e.g. A preceding slot is
inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
play a performance penalty to remove this kind of false-positive.
- Add more layout tests.
- Add W3C Web Platform tests and upstream it.
- Optimize the performance more and more.
e.g. We could optimize SlotAssignment::aaa, which is calculated synchronously,
to detect slotchange exactly.
- Support SlotAssignment even it is non-shadow tree. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate a distribution flag for v0 and v1. Currently, the same flag is used in
both kinds of shadow trees, v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose. e.g. it used
compareDocumentPosition, which
Make our own version or make it template it. e.g. DocumentOrderedList<T>,
specialize it for our purpose.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Got significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree.
The HashMap is updated in each insertion/removal/renaming of slots.
- DOM Standard [2] requires synchronous calculation of slot assignments in
each DOM mutation to fire a slotchagne event,
however, as an optimization, we do not calculate it synchronously if we can
be sure to delay it.
Minimum check is done for detecting slotchange, using the characteristics of
slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
- HTMLSlotElement becomes much smaller footprint element.
- The following member variables are gone:
m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
- SlotAssignment::resolveDistribution became much simpler because slotchange is
already detected at early stage.
See the details in bug 610961 for basic ideas behind the scenes.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
This CL also fixed underwater bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive for slotchange event. e.g. A preceding slot is
inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
play a performance penalty to remove this kind of false-positive.
- Add more layout tests.
- Add W3C Web Platform tests and upstream it.
- Optimize the performance more and more.
e.g. We could optimize SlotAssignment::aaa, which is calculated synchronously,
to detect slotchange exactly.
- Support SlotAssignment even it is non-shadow tree. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate a distribution flag for v0 and v1. Currently, the same flag is used in
both kinds of shadow trees, v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose. e.g. it used
compareDocumentPosition, which
Make our own version or make it template it. e.g. DocumentOrderedList<T>,
specialize it for our purpose.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Get significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
- Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, the
only minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
- Make HTMLSlotElement a much smaller footprint element.
- The following member variables are removed:
- m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
- Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Get significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
- Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, the
only minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
- Make HTMLSlotElement a much smaller footprint element.
- The following member variables are removed:
- m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
- Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Get significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
- Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, the
only minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
- Make HTMLSlotElement a much smaller footprint element.
- The following member variables are removed:
- m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
- Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
- Make Blink be more spec compliant, regarding slotchange event.
- Get significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
- Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, the
only minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
- Make HTMLSlotElement a much smaller footprint element.
- The following member variables are removed:
- m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
- Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
- Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, the
only minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
- The following member variables are removed:
- m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
wip
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1995203002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1995203002/20001
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
- Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, the
only minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
- The following member variables are removed:
- m_oldAssingedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentSate
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
- Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, the
only minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
- The following member variables are removed:
- m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentState
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
- Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
- Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, the
only minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
- Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
- The following member variables are removed:
- m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentState
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming of
slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate it
synchronously,
as an important optimization, if we can be sure to delay it. Instead, the only
minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentState
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming of
slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate it
synchronously,
as an important optimization, if we can be sure to delay it. Instead, the only
minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentState
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, the
only minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentState
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, the
only minimum check is done to detect
a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentState
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, we do a
minimum necessary check to detect
a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentState
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order
in a shadow tree. The HashMap is updated in each insertion/removal/renaming
of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to
fire a slotchagne event at the end of microtask, this CL does not calculate
it synchronously,
as an important optimization, if we can be sure to delay it. Instead, we do a
minimum necessary check to detect
a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes, m_distributionState, m_assignmentState
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
- There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot.
This would not be a significant issue, but should be fixed. As of now, we must
pay a performance penalty
to remove this kind of false-positive.
- Add more layout tests and add W3C Web Platform tests to be upstreamed.
- Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
- Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
- Isolate NeedsRecalDistribution flag for v0 and v1. Currently, the same flag is
used in both kinds of shadow trees,
v0 and v1, to support both together in one page.
- DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general.
It might make sense to make our own version.
Though, this is a low-priority task because it is unlikely that there are more
than one slots which have the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree. The HashMap is
updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly and
more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree. The HashMap is
updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree. The HashMap is
updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree. The HashMap is
updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree. The HashMap is
updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
hayato@chromium.org changed reviewers: + esprehn@chromium.org, kochi@chromium.org, tkent@chromium.org
I think this CL is ready to be reviewed. PTAL.
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange events [1] more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree. The HashMap is
updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree. The HashMap is
updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment now slotname-to-slots HashMap, where slots are
DocumentOrderedList to maintain tree order in a shadow tree. The HashMap is
updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slots| HashMap, where |slots| are
DocumentOrderedList so that we maintain tree order in a shadow tree.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slots| HashMap, where |slots| are
DocumentOrderedList so that we maintain tree order in a shadow tree.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slots| HashMap, where |slots| are
DocumentOrderedList so that we maintain tree order in a shadow tree.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slots| HashMap, where |slots| are
DocumentOrderedList so that we maintain tree order in a shadow tree.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of a micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slots| HashMap, where |slots| are
DocumentOrderedList so that we maintain tree order in a shadow tree.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slots| HashMap, where |slots| are
DocumentOrderedList so that we maintain tree order in a shadow tree.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slots| HashMap, where |slots| are
DocumentOrderedList so that we maintain tree order in a shadow tree.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961,579107
==========
On 2016/05/20 08:06:35, hayato wrote: > I think this CL is ready to be reviewed. PTAL. Let me take a look. (takes a while)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Comments for SlotAssignment class. I'm looking into the rest now. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:37: // |first| is preceding |slot|. Maybe you can add a test case for this case, and now marking it failing, and add TODO in the test to fix this? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:45: if (first && first != &slot) { Instead of |first != &slot|, you should check |oldFirst == slot|? If there are 3 slots, if removed slot was second or third, no slotchange event will occur, but if removed slot was oldFirst, then send slotchange event to the second? At this point, first can't be |slot| anyway because it is already removed from |m_slots| on line 43. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:89: // TODO(hayato): Avoid traversing children every time. Can we shortcut the traverse if any slot with |slotName| has already assigned nodes? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:104: entry(oldSlotName).remove(slot); Shall we remove the entry from |m_slotEntryMap| when the entry becomes empty? (if script makes random slot name and change it many times, SlotAssignment gets a lot of garbage entries.) https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:49: // 1. https://dom.spec.whatwg.org/#assignTo-slotables Can't navigate to this URL hash. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:51: // 2. https://dom.spec.whatwg.org/#assignTo-a-slot Ditto. (the 2 merged to https://dom.spec.whatwg.org/#assigning-slotables-and-slots ?) https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:54: // Instead, provide alternative, HTMLSlotElement::isAssignedNodesEmptySynchronoulsy nit: s/Synchronoulsy/Synchronously/
This is a big patch, it does seem like a big improvement though. I didn't see the reviews for the v1 stuff before, but not keeping around all of those data members is way better. :) I'll have to look this over again tomorrow. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Perf... File third_party/WebKit/PerformanceTests/Skipped (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Perf... third_party/WebKit/PerformanceTests/Skipped:61: ShadowDOM/v1-host-child-apped.html typo, did you mean append? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/Document.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/Document.cpp:3844: for (Node& n : NodeTraversal::childrenOf(container)) this traverses the whole subtree again whenever you remove a sutbree. ex. doing document.body.removeChild() will get traversed again inside here. This should be done inside ::removedFrom or ::childrenChanged or inside the notifyRemoved walk. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/Node.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/Node.cpp:2389: // Although DOM Standard requires "assign a slot for node / run assing slotables" at this timing, typo, assign? you probably want to spell check your comments. :) https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:45: if (first && first != &slot) { No need for the &, you can compare references and ptrs to nodes https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:61: return size() ? toHTMLSlotElement(*m_slots.begin()) : nullptr; You want .first() https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:75: entry(slot.name()).add(slot); findEntryByName ? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:199: return entry(slotName).firstSlot(); Hmm, we should just reuse the same map machinery we use for ids. As long as you don't have conflicting names it's fast and there's no traversing. You want to add getElementBySlotName to DocumentOrderedMap and use the add() version that takes a string. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:208: for (HTMLSlotElement& slot : Traversal<HTMLSlotElement>::descendantsOf(*m_owner)) { we can just use HTMLSlotElement::insertedInto and a check for duplicate slot names that causes the slow traversal. We should just use the same collection machinery we use for ids. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:33: DECLARE_TRACE(); I don't think traces are usually private? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:45: HTMLSlotElement* findSlot(const Node&); what does the first one do? How does it find a slot given a node? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:70: SlotAssignment(ShadowRoot& owner); explicit, even private constructors need it or your class can invoke it from internally by mistake https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:83: unsigned m_slotCount = 0; we could combine these together to make this thing be only one unsigned large https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLSlotElement.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:60: return (name.isNull() || name.isEmpty()) ? emptyAtom : name; Why does your code care if the name is null or empty? This method should not be needed. Just add the .isEmpty() checks to the right spots. In general your code should avoid caring about null or empty strings https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:82: #if DCHECK_IS_ON why is this needed now? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:98: m_distributedNodes.appendVector(toHTMLSlotElement(child).getDistributedNodes()); This code doesn't make sense, if the slot itself is not in a shadow tree then none of the children of it can be in a shadow tree either. This is then iterating every child and putting them into the vector in tree order, and then it's putting every child of every <slot> that's a child of this slot in there too. Why does the spec want us to put every descendant of the slot element into a vector? That's very weird. ex. <slot> <slot> <div /> <slot> <span /> </slot> </slot> <section /> </slot> This method walks that entire subtree and returns [div, span, section] but why?? If that's what the spec really wants then all of this recursion isn't needed, just use descendantsOf() instead. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:137: m_slotchangeEventEnqueued = false; you need to set this to false before you dispatch the event, otherwise a DOM mutation inside the event listener will fail to cause another slot change event to happen since it'll think the event is still pending. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:190: root->ensureSlotAssignment().slotRenamed(normalizeSlotName(oldValue), *this); slotRenamed shouldn't care if the string is empty or null https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:282: Microtask::enqueueMicrotask(WTF::bind(&HTMLSlotElement::dispatchSlotChangeEvent, this)); Does a Persistent<> get magically created for |this| here? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLSlotElement.h (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.h:70: bool hasAssignedNodesSynchronously() const; what does the sync mean on this? Can you add a comment?
https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Perf... File third_party/WebKit/PerformanceTests/Skipped (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Perf... third_party/WebKit/PerformanceTests/Skipped:58: # Times out on reference build - crbug.com/579107 What's the plan for enabling these tests? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:32: if (first.findHostChildWithSameSlotName()) Can this be simply "if (findHostChildBySlotName(first.name())"? (ditto for line 47) https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLSlotElement.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:202: return root1; The caller of this is only insertedInto() below (as of now), and this is called within context that slot.containingShadowRoot() exists - so root1 should be always non-null, and only relevant check here seems if (root1 == root2) return false; which means "if insertion point is under the same shadow root, return false". Then it sounds logically inverted from what the function name suggests to me. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:215: root->ensureSlotAssignment().slotAdded(*this); I'm confused at following the logic here - could you explain the intention here? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:236: // 1. If this slot is still in a tree scope, it means the slot has been in a shadow tree. An inclusive inclusive shadow-including ancestor of the shadow host was originally removed from its parent. nit: s/inclusive inclusive/inclusive/
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Use DocumentOrderedMap
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1995203002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1995203002/40001
Thank you for the reviews. I am afraid that I have not answered every review comments yet, but let me upload the next patch now because I started to use DocumentOrderedMap. Some of my comments might be outdated due to DocumentOrderedMap. Please review to the latest patch. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Perf... File third_party/WebKit/PerformanceTests/Skipped (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Perf... third_party/WebKit/PerformanceTests/Skipped:58: # Times out on reference build - crbug.com/579107 On 2016/05/24 at 10:02:56, kochi wrote: > What's the plan for enabling these tests? I want to enable these tests, of course. :) That can be tackled by another effort. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/Document.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/Document.cpp:3844: for (Node& n : NodeTraversal::childrenOf(container)) My intention to add this check code here, Document::nodeChildrenWillBeRemoved, is to get this called only *once* per removing a subtree. As far as I can read, Document::nodeChildrenWillBeRemoved is called from ContainerNode::removeChildren, which is called only from Node::setTextContent or something similar. Let me re-check later, but it looks this is not called recursively. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:37: // |first| is preceding |slot|. Yes, we should have a test. Let me add a test later. There might be other false-positive cases. Thus, let me investigate further as another task. I do not want to make this CL larger than the current one. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:45: if (first && first != &slot) { Done https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:61: return size() ? toHTMLSlotElement(*m_slots.begin()) : nullptr; Done https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:75: entry(slot.name()).add(slot); Done. Because this method creates and return a new entry if not found, I renamed it |entryByName| rather than |findEntryByName|. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:89: // TODO(hayato): Avoid traversing children every time. On 2016/05/23 at 06:08:42, kochi wrote: > Can we shortcut the traverse if any slot with |slotName| has already assigned nodes? No. Slot's assigned nodes are not updated at this timing. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:104: entry(oldSlotName).remove(slot); On 2016/05/23 at 06:08:42, kochi wrote: > Shall we remove the entry from |m_slotEntryMap| when the entry becomes empty? > > (if script makes random slot name and change it many times, SlotAssignment > gets a lot of garbage entries.) We can. However, it is very rare. I am afraid that we might not justify the cost of checking emptiness. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:199: return entry(slotName).firstSlot(); On 2016/05/23 at 06:41:00, esprehn wrote: > Hmm, we should just reuse the same map machinery we use for ids. As long as you don't have conflicting names it's fast and there's no traversing. You want to add getElementBySlotName to DocumentOrderedMap and use the add() version that takes a string. Ops. Thank you for letting me notice that. It looks I have almost re-invented DocumentOrderedMap, with a different strategy for conflicting. :( Done. Since DocumentOrderedMap does now allow an access to its internal MayEntry, I have to look up a map more than onc at several places, e.g. SlotAssignment::slotAdded(). Let me optimize this with a fast access later. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:208: for (HTMLSlotElement& slot : Traversal<HTMLSlotElement>::descendantsOf(*m_owner)) { On 2016/05/23 at 06:41:00, esprehn wrote: > we can just use HTMLSlotElement::insertedInto and a check for duplicate slot names that causes the slow traversal. We should just use the same collection machinery we use for ids. Yeah, but we need an ordered list of *all* slots, regardless of their name, in a tree order, which is necessary to calculate fallback contents correctly. e.g. <slot name=s1> <slot name=s2></slot> </slot> See SlotAssignment::resolveDistribution, where "for (auto slot = slots.rbegin(); slot != slots.rend(); ++slot)" is used. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:45: HTMLSlotElement* findSlot(const Node&); On 2016/05/23 at 06:41:00, esprehn wrote: > what does the first one do? How does it find a slot given a node? It uses the node's slot attribute. I chose this method name to make it be consistent with the DOM Standard. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:49: // 1. https://dom.spec.whatwg.org/#assignTo-slotables Fixed https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:51: // 2. https://dom.spec.whatwg.org/#assignTo-a-slot Fixed https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.h:83: unsigned m_slotCount = 0; Done https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLSlotElement.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:60: return (name.isNull() || name.isEmpty()) ? emptyAtom : name; |name| here came from the result of Element::fastGetAttribute(), which can be nullAtom (e.g. for <div></div>) or emptyAtom (e.g. for <div slot=""></div>) We have to treat both cases in the same way as per the Shadow DOM spec. I am assuming that nullAtom != emptyAtom if they are used as a key of HashMap. Normalizing a result of fastGetAttribute() in one place will eliminate several manual checks for (isEmpty() || isNull()). https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:82: #if DCHECK_IS_ON Done. Removed. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:98: m_distributedNodes.appendVector(toHTMLSlotElement(child).getDistributedNodes()); On 2016/05/23 at 06:41:00, esprehn wrote: > This code doesn't make sense, if the slot itself is not in a shadow tree then none of the children of it can be in a shadow tree either. This is then iterating every child and putting them into the vector in tree order, and then it's putting every child of every <slot> that's a child of this slot in there too. Why does the spec want us to put every descendant of the slot element into a vector? That's very weird. > > ex. > > <slot> > <slot> > <div /> > <slot> > <span /> > </slot> > </slot> > <section /> > </slot> > > This method walks that entire subtree and returns [div, span, section] but why?? That's an expected behavior. A slot itself can be a fallback content of other (parent) slot. We have to *flatten* a slot, recursively, even in a case of fallback contents. Maybe it might be easier to understand why we need this if each slot has a slot name, and suppose they are used in a shadow tree <slot name=s1> <slot name=s2> <div /> <slot name=s3> <span /> </slot> </slot> <section /> </slot> 1. If a host's children are [<div slot=s1>], slot s1's distributed nodes is [<div slot=s1>] 2. If a host's children are [<div slot=s2>], slot s1's distributed nodes is [<div slot=s2>, <section>] 3. If a host's children are [<div slot=s3>], slot s1's distributed nodes is [<div>, <div slot=s3> <section>, 4. If a host's children are none, slot s1's distributed nodes is [<div>, <span>, <section>], as you said. That's an expected behavior. > If that's what the spec really wants Yes. > then all of this recursion isn't needed, just use descendantsOf() instead. We can not. e.g. <slot name=s1> <slot name=s2> <div> <div id=foo> <slot name=s3> <span /> </slot> </div> </slot> <section /> </slot> Is this case, s1' distributed nodes will be [<div>, <section>] (where neither <div id=foo> nor <span> are included), if the host does not have a child. I think it is rare that slots are used in a document tree, thus, the support for slots in a document tree is very limited in the current implementation. That's one of TODO items. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:190: root->ensureSlotAssignment().slotRenamed(normalizeSlotName(oldValue), *this); On 2016/05/23 at 06:41:00, esprehn wrote: > slotRenamed shouldn't care if the string is empty or null That should be cared because a slot with name of "empty or null" can be matched to a host's child whose slot name is "empty or null". https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:202: return root1; No. It looks you are focusing on the node tree's *current* status, *after* the node was inserted. This is check for the status in the past, *before* the |slot| was inserted. If root1 == root2, that means |slot| was in a *disconnected* tree. Does it make sense? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:282: Microtask::enqueueMicrotask(WTF::bind(&HTMLSlotElement::dispatchSlotChangeEvent, this)); Good point! It looks it does not. Let me use Persistent<HTMLSlotElement>(this)) here. Is this okay here? https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLSlotElement.h (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.h:70: bool hasAssignedNodesSynchronously() const; On 2016/05/23 at 06:41:01, esprehn wrote: > what does the sync mean on this? Can you add a comment? Yeah, I agree this is a confusing name.. I renamed this to canHaveAssignedNodes(), with a comment. A caller should use this method when the slot's m_assignedNodes is not calculated yet, which is asynchronously updated later in resolveAssignment(). Thus, this method has to check the slot's assignedNodes *synchronously* without waiting for resolveAssignment().
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
clean up
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1995203002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1995203002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
replies only. I'm now looking at the new patch set. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Perf... File third_party/WebKit/PerformanceTests/Skipped (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Perf... third_party/WebKit/PerformanceTests/Skipped:58: # Times out on reference build - crbug.com/579107 On 2016/05/24 13:23:38, hayato wrote: > On 2016/05/24 at 10:02:56, kochi wrote: > > What's the plan for enabling these tests? > > I want to enable these tests, of course. :) That can be tackled by another > effort. Acknowledged. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:89: // TODO(hayato): Avoid traversing children every time. On 2016/05/24 13:23:38, hayato wrote: > On 2016/05/23 at 06:08:42, kochi wrote: > > Can we shortcut the traverse if any slot with |slotName| has already assigned > nodes? > > No. Slot's assigned nodes are not updated at this timing. Acknowledged. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:104: entry(oldSlotName).remove(slot); On 2016/05/24 13:23:39, hayato wrote: > On 2016/05/23 at 06:08:42, kochi wrote: > > Shall we remove the entry from |m_slotEntryMap| when the entry becomes empty? > > > > (if script makes random slot name and change it many times, SlotAssignment > > gets a lot of garbage entries.) > > We can. However, it is very rare. I am afraid that we might not justify the cost > of checking emptiness. Even it's rare, do we (kind of) leak empty slot entries? I don't think it's such heavy cost... https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLSlotElement.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:202: return root1; On 2016/05/24 13:23:39, hayato wrote: > No. It looks you are focusing on the node tree's *current* status, *after* the > node was inserted. > > This is check for the status in the past, *before* the |slot| was inserted. > > If root1 == root2, that means |slot| was in a *disconnected* tree. Does it make > sense? Acknowledged.
https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLSlotElement.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:98: m_distributedNodes.appendVector(toHTMLSlotElement(child).getDistributedNodes()); If you're not in a shadow tree then none of the elements you can reach through NodeTraversal::childrenOf can be either, which means the recursive step of toHTMLSlotElement(child).getDistributedNodes() on the child will go to: if (isInShadowTree()) return ...; but it'll be false, so then you'll come down to this loop and start iterating the kids of that child. You can instead just use a loop over NodeTraversal::descendantsOf and not have this be recursive at all.
lgtm from me, with some nits. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:104: entry(oldSlotName).remove(slot); On 2016/05/25 05:37:11, kochi wrote: > On 2016/05/24 13:23:39, hayato wrote: > > On 2016/05/23 at 06:08:42, kochi wrote: > > > Shall we remove the entry from |m_slotEntryMap| when the entry becomes > empty? > > > > > > (if script makes random slot name and change it many times, SlotAssignment > > > gets a lot of garbage entries.) > > > > We can. However, it is very rare. I am afraid that we might not justify the > cost > > of checking emptiness. > > Even it's rare, do we (kind of) leak empty slot entries? > I don't think it's such heavy cost... Ah, it is no longer necessary because now we use DocumentOrderedMap and SlotEntry is no longer used. https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp (right): https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:31: HTMLSlotElement& oldFirst = *findSlotByName(slot.name()); nit: maybe oldActive or oldActiveSlot is better? I'm okay with oldFirst, though. https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:34: if (findSlotByName(slot.name()) == &oldFirst) nit: No need for & https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:52: HTMLSlotElement* oldFirst = findSlotByName(slot.name()); nit: oldActive and newActive in this function? https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLSlotElement.cpp (right): https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:259: if (root && root->isV1() && root == &insertionPoint->treeScope().rootNode()) { nit: no need for &
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slots| HashMap, where |slots| are
DocumentOrderedList so that we maintain tree order in a shadow tree.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 2x faster
Time:
Before: avg 3190.362893289229 runs/s
After: avg 6078.959691839927 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.5x faster
Time:
Before: avg 51.60166182585167 runs/s
After: avg 333.3014393812497 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.3x faster
Time:
Before: avg 1647.4161901719608 runs/s
After: avg 5485.582186368757 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedList is not optimized for our purpose.
e.g. it uses Document::compareDocumentPosition, which is too general. It
might make sense to make our own version. Though, this is a low-priority
task because it is unlikely that there are more than one slots which have
the same name in a shadow tree
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961,579107
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slots| HashMap, where |slots| are
DocumentOrderedList so that we maintain tree order in a shadow tree.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 1.8x faster
Time:
Before: avg 3190.3 runs/s
After: avg 5885.6 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.0x faster
Time:
Before: avg 51.6 runs/s
After: avg 306.4 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.4x faster
Time:
Before: avg 1647.4 runs/s
After: avg 5645.0 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedMap might not be optimized for our purpose.
e.g. DocumentOrderedMap has to travers DOM Tree if conflicts happen.
Just using Document::compareDocumentPosition() might be better, given that
conflicts is unlikely to happen.
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961,579107
==========
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
fixed
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1995203002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1995203002/80001
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slots| HashMap, where |slots| are
DocumentOrderedList so that we maintain tree order in a shadow tree.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 1.8x faster
Time:
Before: avg 3190.3 runs/s
After: avg 5885.6 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.0x faster
Time:
Before: avg 51.6 runs/s
After: avg 306.4 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.4x faster
Time:
Before: avg 1647.4 runs/s
After: avg 5645.0 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedMap might not be optimized for our purpose.
e.g. DocumentOrderedMap has to travers DOM Tree if conflicts happen.
Just using Document::compareDocumentPosition() might be better, given that
conflicts is unlikely to happen.
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961,579107
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slot| HashMap, via DocumentOrderedMap.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 1.8x faster
Time:
Before: avg 3190.3 runs/s
After: avg 5885.6 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.0x faster
Time:
Before: avg 51.6 runs/s
After: avg 306.4 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.4x faster
Time:
Before: avg 1647.4 runs/s
After: avg 5645.0 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedMap might not be optimized for our purpose.
e.g. DocumentOrderedMap has to travers DOM Tree if conflicts happen.
Just using Document::compareDocumentPosition() might be better, given that
conflicts is unlikely to happen.
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961,579107
==========
Thank you for the review. I also updated the description of this CL. I re-ran the benchmarks. The result became slightly slower than the result of the previous patch which does not use DocumentOrderedMap. However, that could be acceptable. That's still much better than the current Blink's implementation. https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLSlotElement.cpp (right): https://codereview.chromium.org/1995203002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:98: m_distributedNodes.appendVector(toHTMLSlotElement(child).getDistributedNodes()); I am afraid that it is not easy with NodeTraversal::descendantsOf. Let me re-use the example: <slot name=s1> <slot name=s2> <div> <div id=foo> <slot name=s3> <span /> </slot> </div> </slot> <section /> </slot> For s1.getDistributedNodes(): 1. We have to pick up: [<div>, <section>] 2. We should not pick up: [<div id=foo>, <span>]. With NodeTraversal::descendantsOf, we need a `shouldPickup` function, like: for (Node& node: NodeTraversal::descendantsOf(slot) { if (shouldPickup(node) { distributedNodes.append(node) } } I do not see how we can implement `shouldPickup` easily here without be recursive. - shoudPickup should return true for <div> and <section> - shoudPickup should return false for <div id=foo> and <span> https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp (right): https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:31: HTMLSlotElement& oldFirst = *findSlotByName(slot.name()); Done https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:34: if (findSlotByName(slot.name()) == &oldFirst) Done https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/dom/shadow/SlotAssignment.cpp:52: HTMLSlotElement* oldFirst = findSlotByName(slot.name()); Done https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/html/HTMLSlotElement.cpp (right): https://codereview.chromium.org/1995203002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/html/HTMLSlotElement.cpp:259: if (root && root->isV1() && root == &insertionPoint->treeScope().rootNode()) { Done
You can use nextSkippingChildren and next then with ElementTraversal.
The code is this:
HTMLSlotElement::getDistributedNodes()
{
if in_shadow_tree:
return m_distributedNodes;
reset distribution.
# If we got here then in_shadow_tree is false for every child, and
# every descendant below child.
for each child of this element {
if child is not slotable:
continue.
if child is a <slot>
recursively call getDistributedNodes on child. <--- RECURSION
append the result to m_distributedNodes.
else
append child to m_distributedNodes.
}
return m_distributedNodes;
}
But that recursive step, and the calls to clearDistribution(),
resetDistribution(), etc. are just wasted at every level.
Instead I think you can write:
HTMLSlotElement::getDistributedNodes()
{
if in_shadow_tree:
return m_distributedNodes;
reset distribution.
Element* child = ElementTraversal::firstChild(*this);
while (child) {
if (!child->isSlotable()) {
child = ElementTraversal::nextSibling(*child);
continue;
}
if (isHTMLSlotElement(child)) {
child = ElementTraversal::next(*child, this);
} else {
m_distributedNodes.append(this);
child = ElementTraversal::nextSkippingChildren(*child, this);
}
}
return m_distributedNodes;
}
which iteratively walks down the tree collecting the children of this node, or
the children of any child <slot> recursively.
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Update HTMLSlotElement::getDistributedNodes() for a document tree
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1995203002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1995203002/100001
Yeah, we can achieve that with nextSkippingChildren. Great. I updated getDistributedNodes(), as you suggested, with two modifications: - Use NodeTraversal, instead of ElementTraversal, because TextNode is slotable. - Use nextSkippingChildren(), instead of nextSibling(*child), in the case of `if (!child->isSlotable())`, because we might be traversing a non-direct child here.
The new description looks good, one nit: - This CL also fixes bugs around slot assignment, e.g. bug 531990. Is this line meant to refer bug 610588? 531990 is the umbrella bug for V1.
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slot| HashMap, via DocumentOrderedMap.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 1.8x faster
Time:
Before: avg 3190.3 runs/s
After: avg 5885.6 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.0x faster
Time:
Before: avg 51.6 runs/s
After: avg 306.4 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.4x faster
Time:
Before: avg 1647.4 runs/s
After: avg 5645.0 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107.
- This CL also fixes bugs around slot assignment, e.g. bug 531990.
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedMap might not be optimized for our purpose.
e.g. DocumentOrderedMap has to travers DOM Tree if conflicts happen.
Just using Document::compareDocumentPosition() might be better, given that
conflicts is unlikely to happen.
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961,579107
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slot| HashMap, via DocumentOrderedMap.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 1.8x faster
Time:
Before: avg 3190.3 runs/s
After: avg 5885.6 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.0x faster
Time:
Before: avg 51.6 runs/s
After: avg 306.4 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.4x faster
Time:
Before: avg 1647.4 runs/s
After: avg 5645.0 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107
- This CL also fixes bugs around slot assignment, e.g. crbug.com/610588
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedMap might not be optimized for our purpose.
e.g. DocumentOrderedMap has to travers DOM Tree if conflicts happen.
Just using Document::compareDocumentPosition() might be better, given that
conflicts is unlikely to happen.
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961,579107
==========
On 2016/05/26 at 06:34:01, kochi wrote: > The new description looks good, one nit: > > - This CL also fixes bugs around slot assignment, e.g. bug 531990. > > Is this line meant to refer bug 610588? > 531990 is the umbrella bug for V1. Yes. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
No longer FAIL: imported/wpt/shadow-dom/HTMLSlotElement-interface.html
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1995203002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1995203002/120001
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slot| HashMap, via DocumentOrderedMap.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 1.8x faster
Time:
Before: avg 3190.3 runs/s
After: avg 5885.6 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.0x faster
Time:
Before: avg 51.6 runs/s
After: avg 306.4 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.4x faster
Time:
Before: avg 1647.4 runs/s
After: avg 5645.0 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107
- This CL also fixes bugs around slot assignment, e.g. crbug.com/610588
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedMap might not be optimized for our purpose.
e.g. DocumentOrderedMap has to travers DOM Tree if conflicts happen.
Just using Document::compareDocumentPosition() might be better, given that
conflicts is unlikely to happen.
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961,579107
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slot| HashMap, via DocumentOrderedMap.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 1.8x faster
Time:
Before: avg 3190.3 runs/s
After: avg 5885.6 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.0x faster
Time:
Before: avg 51.6 runs/s
After: avg 306.4 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.4x faster
Time:
Before: avg 1647.4 runs/s
After: avg 5645.0 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107
- This CL also fixes the following bugs. These tests no longer fail.
- crbug.com/610588
- imported/wpt/shadow-dom/HTMLSlotElement-interface.html
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedMap might not be optimized for our purpose.
e.g. DocumentOrderedMap has to travers DOM Tree if conflicts happen.
Just using Document::compareDocumentPosition() might be better, given that
conflicts is unlikely to happen.
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961,579107
==========
I have removed third_party/WebKit/LayoutTests/imported/wpt/shadow-dom/HTMLSlotElement-interface-expected.txt because this wpt test no longer fails.
lgtm
The CQ bit was unchecked by hayato@chromium.org
The CQ bit was checked by hayato@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1995203002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1995203002/120001
Message was sent while issue was closed.
Committed patchset #7 (id:120001)
Message was sent while issue was closed.
Description was changed from
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slot| HashMap, via DocumentOrderedMap.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 1.8x faster
Time:
Before: avg 3190.3 runs/s
After: avg 5885.6 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.0x faster
Time:
Before: avg 51.6 runs/s
After: avg 306.4 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.4x faster
Time:
Before: avg 1647.4 runs/s
After: avg 5645.0 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107
- This CL also fixes the following bugs. These tests no longer fail.
- crbug.com/610588
- imported/wpt/shadow-dom/HTMLSlotElement-interface.html
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedMap might not be optimized for our purpose.
e.g. DocumentOrderedMap has to travers DOM Tree if conflicts happen.
Just using Document::compareDocumentPosition() might be better, given that
conflicts is unlikely to happen.
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961,579107
==========
to
==========
Rewrite Shadow DOM distribution engine to support partial synchronous
distribution for v1
This is a huge engine rewrite to support slotchange [1] events more strictly
and more efficiently.
The summary of the changes is:
1. Make Blink be more spec compliant, regarding slotchange event.
2. Get significant performance improvements.
Now SlotAssignment has |slotname|-to-|slot| HashMap, via DocumentOrderedMap.
This HashMap is updated in each insertion/removal/renaming of slots.
Though DOM Standard [2] requires synchronous calculation of slot assignments
in each DOM mutation to fire a slotchagne event at the end of microtask,
this CL does not calculate it synchronously, as an important optimization,
if we can be sure to delay it. Instead, we do a minimum necessary check to
detect a slotchange, using the characteristics of slot assignments.
Especially, appending a child to a shadow host, which happens a lot in
Polymer, becomes much faster.
3. Make HTMLSlotElement a much smaller footprint element.
The following member variables are removed:
m_oldAssignedNodes, m_oldDistributedNodes, m_fallbackNodes,
m_oldFallbackNodes,
m_distributionState and m_assignmentState.
4. Make SlotAssignment::resolveDistribution much simpler because slotchange is
already detected at early stage.
See the bug 610961 for details to know the basic ideas behind the scene.
The results of micro benchmarks are:
- PerformanceTests/ShadowDOM/v1-distribution.html => About 1.8x faster
Time:
Before: avg 3190.3 runs/s
After: avg 5885.6 runs/s
- PerformanceTests/ShadowDOM/v1-host-child-append.html => About 6.0x faster
Time:
Before: avg 51.6 runs/s
After: avg 306.4 runs/s
- PerformanceTests/ShadowDOM/v1-slot-append.html => About 3.4x faster
Time:
Before: avg 1647.4 runs/s
After: avg 5645.0 runs/s
This CL also reduced the memory usage in micro benchmarks because a slot element
becomes a smaller footprint element.
- PerformanceTests/ShadowDOM/v1-distribution.html
JS Heap:
Before: avg 21357790.4 bytes
After: avg 3136606.4 bytes
- PerformanceTests/ShadowDOM/v1-host-child-append.html
JS Heap:
Before: avg 5860745.6 bytes
After: avg 4165614.4 bytes
- PerformanceTests/ShadowDOM/v1-slot-append.html
JS Heap:
Before: avg 13256016 bytes
After: avg 3540172.8 bytes
Notes:
- Reducing the memory usage is not the primary motivation of this CL.
- These performance tests are being marked skipped. See crbug.com/579107
- This CL also fixes the following bugs. These tests no longer fail.
- crbug.com/610588
- imported/wpt/shadow-dom/HTMLSlotElement-interface.html
The future works are:
1. There is a still false-positive case for slotchange event.
e.g. A preceding slot is inserted together with a following slot. This would
not be a significant issue, but should be fixed. As of now, we must pay a
performance penalty to remove this kind of false-positive.
2. Add more layout tests and add W3C Web Platform tests to be upstreamed.
3. Optimize the performance more and more.
e.g. It might be possible to optimize
HTMLSlotElement::hasAssignedNodesSynchronously by using yet another data
structure.
4. Support SlotAssignment for non shadow trees. e.g. a document tree.
This is a low-priority task because a document tree is unlikely to have a
slot.
5. Isolate NeedsRecalDistribution flag for v0 and v1.
Currently, the same flag is used in both kinds of shadow trees, v0 and v1,
to support both together in one page.
6. DocumentOrderedMap might not be optimized for our purpose.
e.g. DocumentOrderedMap has to travers DOM Tree if conflicts happen.
Just using Document::compareDocumentPosition() might be better, given that
conflicts is unlikely to happen.
Links:
[1] slotchange event: https://github.com/w3c/webcomponents/issues/288
[2] Relevant DOM Standard sections: https://dom.spec.whatwg.org/
4.2.2 Shadow tree 4.2.2.1 Slots
4.2.2.2 Slotables
4.2.2.3 Finding slots and slotables
4.2.2.4 Assigning slotables and slots
4.2.2.5 Signaling slot changes
4.2.3 Mutation algorithms
BUG=531990,595287,610588,610961,579107
Committed: https://crrev.com/6310c024c916f3f7ea89eb3ffabbc94e59a3c29a
Cr-Commit-Position: refs/heads/master@{#396443}
==========
Message was sent while issue was closed.
Patchset 7 (id:??) landed as https://crrev.com/6310c024c916f3f7ea89eb3ffabbc94e59a3c29a Cr-Commit-Position: refs/heads/master@{#396443} |
