Test out performance when adding whitespace to the DOM

Test out performance when adding whitespace to the DOM

BUG=484344

[rhogan, 2015-05-04 20:22:05]

robhogan@gmail.com changed reviewers:
+ leviw@chromium.org

[leviw_travelin_and_unemployed, 2015-05-04 20:55:24]

https://codereview.chromium.org/1126673002/diff/1/PerformanceTests/Layout/Add...
File PerformanceTests/Layout/AddWhitespace.html (right):

https://codereview.chromium.org/1126673002/diff/1/PerformanceTests/Layout/Add...
PerformanceTests/Layout/AddWhitespace.html:11: <div style="display:
inline-block;"></div>
Nit: your whitespace test has too much whitespace ;)

https://codereview.chromium.org/1126673002/diff/1/PerformanceTests/Layout/Add...
PerformanceTests/Layout/AddWhitespace.html:12: <div>
</div>?

https://codereview.chromium.org/1126673002/diff/1/PerformanceTests/Layout/Add...
PerformanceTests/Layout/AddWhitespace.html:24: description: "Measures
performance of layout when adding whitespace to a div.",
This isn't the greatest description. Right now, you're measuring the performance
of parsing and appending whitespace nodes and collapsed inlines.

You may want to use DOM apis instead of innerHTML? What about testing
pre-pending?

[leviw_travelin_and_unemployed, 2015-05-04 20:55:38]

leviw@chromium.org changed reviewers:
+ esprehn@chromium.org

[esprehn, 2015-05-04 21:22:40]

This doesn't really benchmark whitespace, you're creating and destroying the DOM
in each iteration of the loop and also forcing a compositing update.

https://codereview.chromium.org/1126673002/diff/1/PerformanceTests/Layout/Add...
File PerformanceTests/Layout/AddWhitespace.html (right):

https://codereview.chromium.org/1126673002/diff/1/PerformanceTests/Layout/Add...
PerformanceTests/Layout/AddWhitespace.html:17: target.innerHTML += '   <div
style="display: inline-block;"></div>   ';
This is serializing the DOM into a big string, destroying all the existing
nodes, recreating a whole new DOM with one extra div, then appending all those
new nodes.

https://codereview.chromium.org/1126673002/diff/1/PerformanceTests/Layout/Add...
PerformanceTests/Layout/AddWhitespace.html:18:
PerfTestRunner.forceLayoutOrFullFrame();
This is forcing a full frame update, that means compositing update and other
stuff too. Since you're also destroying and recreating the entire page that's
all you're really benchmarking.

https://codereview.chromium.org/1126673002/diff/1/PerformanceTests/Layout/Add...
PerformanceTests/Layout/AddWhitespace.html:20: target.innerHTML = '<div
style="display: inline-block;"></div>';
This is going to destroy all the nodes you created and recalced in the previous
step.

https://codereview.chromium.org/1126673002/diff/1/PerformanceTests/Layout/Add...
File PerformanceTests/Layout/AddWhitespaceToAnonymousTable.html (right):

https://codereview.chromium.org/1126673002/diff/1/PerformanceTests/Layout/Add...
PerformanceTests/Layout/AddWhitespaceToAnonymousTable.html:20: target.innerHTML
= '<div style="display: inline-block;"></div>';
ditto, this doesn't benchmark anything related to whitespace, it benchmarks
innerHTML, creating and destroying a moderately sized (100 element) DOM over and
over and then doing the full frame update. The whitespace isn't really
important.

[rhogan, 2015-05-04 21:45:14]

On 2015/05/04 at 21:22:40, esprehn wrote:
> This doesn't really benchmark whitespace, you're creating and destroying the
DOM in each iteration of the loop and also forcing a compositing update.

Right, I do understand that there's a lot more going on in the test than just
adding whitespace but my object is to create a test that's sensitive to work
done when adding whitespace - and these two test definitely detect when more
work starts happening in canHaveWhitespaceChildren. I'm creating them as a
baseline for https://codereview.chromium.org/1065563005 and used them to detect
the performance for the table case deteriorating.

[esprehn, 2015-05-04 22:03:40]

On 2015/05/04 at 21:45:14, robhogan wrote:
> On 2015/05/04 at 21:22:40, esprehn wrote:
> > This doesn't really benchmark whitespace, you're creating and destroying the
DOM in each iteration of the loop and also forcing a compositing update.
> 
> Right, I do understand that there's a lot more going on in the test than just
adding whitespace but my object is to create a test that's sensitive to work
done when adding whitespace - and these two test definitely detect when more
work starts happening in canHaveWhitespaceChildren. I'm creating them as a
baseline for https://codereview.chromium.org/1065563005 and used them to detect
the performance for the table case deteriorating.

This test is primarily about how fast we can create and destroy a tree using
innerHTML. We already have Dromaeo, html5-full-render and others for that. This
test benchmarks very little about whitespace. You're also making the GC thrash
pretty hard.

[rhogan, 2015-05-04 22:09:53]

On 2015/05/04 at 22:03:40, esprehn wrote:
> On 2015/05/04 at 21:45:14, robhogan wrote:
> > On 2015/05/04 at 21:22:40, esprehn wrote:
> > > This doesn't really benchmark whitespace, you're creating and destroying
the DOM in each iteration of the loop and also forcing a compositing update.
> > 
> > Right, I do understand that there's a lot more going on in the test than
just adding whitespace but my object is to create a test that's sensitive to
work done when adding whitespace - and these two test definitely detect when
more work starts happening in canHaveWhitespaceChildren. I'm creating them as a
baseline for https://codereview.chromium.org/1065563005 and used them to detect
the performance for the table case deteriorating.
> 
> This test is primarily about how fast we can create and destroy a tree using
innerHTML. We already have Dromaeo, html5-full-render and others for that. This
test benchmarks very little about whitespace. You're also making the GC thrash
pretty hard.

OK, I think I have a better attempt up there now.

[esprehn, 2015-05-04 22:14:39]

https://codereview.chromium.org/1126673002/diff/40001/PerformanceTests/Layout...
File PerformanceTests/Layout/AddWhitespace.html (right):

https://codereview.chromium.org/1126673002/diff/40001/PerformanceTests/Layout...
PerformanceTests/Layout/AddWhitespace.html:3: <head>
Leave out html, head, body and title.

https://codereview.chromium.org/1126673002/diff/40001/PerformanceTests/Layout...
PerformanceTests/Layout/AddWhitespace.html:14: target.innerHTML = '<div
style="display: inline-block;"></div>';
Why inline block?

https://codereview.chromium.org/1126673002/diff/40001/PerformanceTests/Layout...
PerformanceTests/Layout/AddWhitespace.html:16: var testElement =
document.createTextNode("   ");
This is not very realistic, pages don't have huge sequences of whitespace nodes.
I guess this does test something related to whitespace though.

https://codereview.chromium.org/1126673002/diff/40001/PerformanceTests/Layout...
File PerformanceTests/Layout/AddWhitespaceToAnonymousTable.html (right):

https://codereview.chromium.org/1126673002/diff/40001/PerformanceTests/Layout...
PerformanceTests/Layout/AddWhitespaceToAnonymousTable.html:3: <head>
ditto.

https://codereview.chromium.org/1126673002/diff/40001/PerformanceTests/Layout...
PerformanceTests/Layout/AddWhitespaceToAnonymousTable.html:14: target.innerHTML
= '<div style="display: inline-block;"></div>';
Why inline blocks? This just makes us create an anonymous table section.