Cache ViewportSize in Document. [chromium/src : main]
chromeperf@appspot.gserviceaccount.com (Gerrit)
Attention is currently required from: Steinar H Gunderson.
📍 Job complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/15f4e863a60000
To view, visit change 4604629. To unsubscribe, or for help writing mail filters, visit settings.
chromeperf@appspot.gserviceaccount.com (Gerrit)
Attention is currently required from: Steinar H Gunderson.
📍 Job complete.
See results at: https://pinpoint-dot-chromeperf.appspot.com/job/1263b9c7a60000
Steinar H Gunderson (Gerrit)
Attention is currently required from: Anders Hartvoll Ruud, Steinar H Gunderson.
Steinar H Gunderson uploaded patch set #3 to this change.
Cache ViewportSize in Document.
This never changes during a style recalc, and recently got much more
expensive to compute due to cast hardening. We can cache it once
per style recalc and be done with it (the layout sizes cannot change
during one).
Style perfest (Zen 2, LTO but no PGO):
Initial style (µs) Before After Perf 95% CI (BCa)
=================== ========= ========= ======= =================
ECommerce 6445 6070 +6.2% [ +5.3%, +7.0%]
Encyclopedia 55658 51903 +7.2% [ +6.3%, +8.1%]
Extension 89764 83818 +7.1% [ +6.4%, +7.9%]
News 28886 27524 +5.0% [ +4.2%, +5.8%]
Search 8145 7788 +4.6% [ +3.8%, +5.5%]
Social1 17327 16372 +5.8% [ +4.9%, +6.7%]
Social2 11410 10823 +5.4% [ +4.6%, +6.3%]
Sports 30362 29195 +4.0% [ +3.2%, +5.2%]
Video 23890 22926 +4.2% [ +3.5%, +5.0%]
Geometric mean +5.5% [ +5.1%, +5.9%]
Recalc style (µs) Before After Perf 95% CI (BCa)
=================== ========= ========= ======= =================
ECommerce 7910 7203 +9.8% [ +9.0%, +10.6%]
Encyclopedia 43488 39617 +9.8% [ +8.7%, +10.7%]
Extension 86038 79327 +8.5% [ +7.7%, +9.2%]
News 22384 20939 +6.9% [ +6.0%, +7.7%]
Search 4339 3963 +9.5% [ +8.8%, +10.4%]
Social1 12917 11924 +8.3% [ +7.4%, +9.2%]
Social2 9409 8687 +8.3% [ +7.4%, +9.2%]
Sports 16631 15683 +6.0% [ +5.3%, +6.9%]
Video 16538 15453 +7.0% [ +6.3%, +7.8%]
Geometric mean +8.2% [ +7.7%, +8.8%]
MotionMark is more cautious, with only a single tiny improvement on M1
and somewhat modest improvements on x86.
Pinpoint mac-m1_mini_2020-perf, no PGO, 95% CI:
Canvas Arcs [ -0.1%, +0.2%]
Canvas Lines [ -0.2%, +0.1%]
Design [ -1.8%, +4.0%]
Images [ -2.5%, +1.7%]
Leaves [ +0.5%, +0.9%]
Multiply [ -1.8%, +0.8%]
Paths [ -0.2%, +0.3%]
Suits [ -0.1%, +0.5%]
motionmark_ramp_composite [ -0.3%, +0.6%]
Pinpoint win-10-perf, no PGO, 95% CI:
Canvas Arcs [ -0.5%, +0.8%]
Canvas Lines [ -0.6%, +0.1%]
Design [ +1.1%, +3.1%]
Images [ -0.5%, +0.4%]
Leaves [ +0.6%, +1.4%]
Multiply [ +1.0%, +1.9%]
Paths [ -0.4%, +0.4%]
Suits [ -1.0%, -0.0%]
motionmark_ramp_composite [ +0.3%, +0.7%]
Change-Id: Ia8d062edd5b68c20121b6ff1929ddc7bc7051598
---
M third_party/blink/renderer/core/animation/timeline_offset.cc
M third_party/blink/renderer/core/animation/view_timeline.cc
M third_party/blink/renderer/core/css/css_gradient_value.cc
M third_party/blink/renderer/core/css/css_math_expression_node_test.cc
M third_party/blink/renderer/core/css/css_to_length_conversion_data.h
M third_party/blink/renderer/core/css/css_to_length_conversion_data_test.cc
M third_party/blink/renderer/core/css/resolver/style_resolver_state.cc
M third_party/blink/renderer/core/css/style_engine.cc
M third_party/blink/renderer/core/dom/document.cc
M third_party/blink/renderer/core/dom/document.h
M third_party/blink/renderer/core/svg/svg_length_context.cc
11 files changed, 34 insertions(+), 12 deletions(-)
To view, visit change 4604629. To unsubscribe, or for help writing mail filters, visit settings.
Steinar H Gunderson (Gerrit)
Attention is currently required from: Anders Hartvoll Ruud.
Steinar H Gunderson has uploaded this change for review.
Peter Boström (Gerrit)
Attention is currently required from: Anders Hartvoll Ruud, Steinar H Gunderson.
2 comments:
Patchset:
drive-by comment
File third_party/blink/renderer/core/dom/document.h:
Patch Set #3, Line 419:
const CSSToLengthConversionData::ViewportSize& GetViewportSize() const {Would you like to DCHECK that we always fetch the correct version (DCHECK that the calculated version == viewport_size_) ?
To view, visit change 4604629. To unsubscribe, or for help writing mail filters, visit settings.
Anders Hartvoll Ruud (Gerrit)
Attention is currently required from: Steinar H Gunderson.
3 comments:
Commit Message:
Patch Set #3, Line 40:
Geometric mean +8.2% [ +7.7%, +8.8%]👌
File third_party/blink/renderer/core/css/resolver/style_resolver_state.cc:
Patch Set #3, Line 121:
// Called from ApplyInheritance(), presumably because ParentStyle()?Probably to satisfy the crazy requirements related to unholy combinations of lh/em/font-size/line-height.
File third_party/blink/renderer/core/dom/document.h:
// You must call UpdateViewportSize() once before resolving style.
CSSToLengthConversionData::ViewportSize viewport_size_;`StyleRecalcContext` seems like a better fit for this if we anyway freshen this value for every recalc.
That would allow you to smuggle the `ViewportSize` "on stack" all the way to the `StyleResolverState` ctor, which could initialize its `css_to_length_conversion_data_` using that `ViewportSize`. Later calls to `UpdateLengthConversionData` could then effectively just update everything except the viewport sizes.
To view, visit change 4604629. To unsubscribe, or for help writing mail filters, visit settings.
Anders Hartvoll Ruud (Gerrit)
Attention is currently required from: Peter Boström, Steinar H Gunderson.
2 comments:
File third_party/blink/renderer/core/dom/document.h:
Patch Set #3, Line 419:
const CSSToLengthConversionData::ViewportSize& GetViewportSize() const {Would you like to DCHECK that we always fetch the correct version (DCHECK that the calculated versio […]
If we're sticking with the PS3 approach, this sounds like a good idea.
// You must call UpdateViewportSize() once before resolving style.
CSSToLengthConversionData::ViewportSize viewport_size_;
`StyleRecalcContext` seems like a better fit for this if we anyway freshen this value for every reca […]
If we're sticking with the PS3 approach, it probably makes (slightly) more sense to store this on StyleEngine and not Document.
To view, visit change 4604629. To unsubscribe, or for help writing mail filters, visit settings.