Bug 233628

Created attachment 445404 [details] Patch

The testing results in my Mac: Enabled CSS contain: :Runs -> [14.749262536873152, 13.080444735120986, 14.534883720930232, 14.814814814814815, 15.071590052750565, 12.987012987012987, 14.947683109118087, 14.771048744460856, 15.003750937734454, 15.128593040847223, 15.186028853454822, 14.936519790888722, 15.082956259426888, 14.85884101040119, 14.9812734082397, 14.947683109118087, 14.936519790888683, 14.716703458425313, 14.903129657227977, 15.015015015015015] runs/s mean: 14.732687751637487 runs/s median: 14.936519790888703 runs/s stdev: 0.6008961401762974 runs/s min: 12.987012987012987 runs/s max: 15.186028853454822 runs/s Disabled CSS contain: :Runs -> [5.980861244019139, 5.847953216374269, 5.917159763313613, 6.016847172081829, 5.959475566150179, 6.053268765133165, 6.067961165048544, 6.038647342995169, 6.105006105006091, 6.112469437652812, 6.067961165048544, 6.097560975609756, 6.112469437652812, 5.903187721369539, 6.097560975609756, 6.112469437652812, 5.973715651135006, 6.249999999999972, 6.165228113440198, 6.242197253433237] runs/s mean: 6.056100025436322 runs/s median: 6.067961165048544 runs/s stdev: 0.10498908615013607 runs/s min: 5.847953216374269 runs/s max: 6.249999999999972 runs/s

Comment on attachment 445404 [details] Patch I think this patch is ready for review now:)

Comment on attachment 445404 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445404&action=review > PerformanceTests/Layout/large-grid.html:148 > + description: "Measures performance of getting offsetHeight of a large grid container.", I think the focus is more on performance of relayout rather than offsetHeight. Also getting offsetHeight from document element rather than grid container should be about the same time right? So maybe it is better to use that?

Comment on attachment 445404 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445404&action=review >> PerformanceTests/Layout/large-grid.html:148 >> + description: "Measures performance of getting offsetHeight of a large grid container.", > > I think the focus is more on performance of relayout rather than offsetHeight. Also getting offsetHeight from document element rather than grid container should be about the same time right? So maybe it is better to use that? You mean `document.body.offsetHeight`? It's same to gridContainer.offsetHeight, doesn't relayout, if the element is outside the relayout boundary. The following interfaces are same: offsetWidth / offsetHeight, scrollWidth / scrollHeight, clientWidth / clientHeight of element and innerWidth / innerHeight of window. As to the performance of relayout(like getBoundingClientRect), containment doesn't change the performance much in this scenario. Layout containment generates a relayout boundary. It is very efficient to changes of a small part inside a complex document. But in this scenario, it changes a large part of the page, so the performance is not improved. On the other way, for interfaces like offsetHeight, the performance is improved. The relayout boundary makes it's possible to perform the relayout requirement asynchronously.

Comment on attachment 445404 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445404&action=review > PerformanceTests/ChangeLog:14 > + relayout. Then it can schedule the relayout requirement of substree asynchronously. I think the last sentence may be confusing or not relevant, so probably better to remove it. >>> PerformanceTests/Layout/large-grid.html:148 >>> + description: "Measures performance of getting offsetHeight of a large grid container.", >> >> I think the focus is more on performance of relayout rather than offsetHeight. Also getting offsetHeight from document element rather than grid container should be about the same time right? So maybe it is better to use that? > > You mean `document.body.offsetHeight`? It's same to gridContainer.offsetHeight, doesn't relayout, if the element is outside the relayout boundary. > The following interfaces are same: offsetWidth / offsetHeight, scrollWidth / scrollHeight, clientWidth / clientHeight of element and innerWidth / innerHeight of window. > > As to the performance of relayout(like getBoundingClientRect), containment doesn't change the performance much in this scenario. Layout containment generates a relayout boundary. It is very efficient to changes of a small part inside a complex document. > But in this scenario, it changes a large part of the page, so the performance is not improved. On the other way, for interfaces like offsetHeight, the performance is improved. > The relayout boundary makes it's possible to perform the relayout requirement asynchronously. Ok thanks, that clears things up.

<rdar://problem/86147146>

Comment on attachment 445404 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445404&action=review >> PerformanceTests/ChangeLog:14 >> + relayout. Then it can schedule the relayout requirement of substree asynchronously. > > I think the last sentence may be confusing or not relevant, so probably better to remove it. Done, thanks!

Created attachment 446144 [details] Patch

Committed r286595 (?): <https://commits.webkit.org/r286595> All reviewed patches have been landed. Closing bug and clearing flags on attachment 446144 [details].