All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=845944

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=5fdac69f3e69466c93cdaaf05c449bff964ba4b18881404b62ad8fbee5648cca


Bot(s) for this bug's original alert(s):

linux-perf

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/14b1f2a2240000

📍 Found significant differences after each of 3 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/14b1f2a2240000

Mojo: Invitation API by rockot@chromium.org
https://chromium.googlesource.com/chromium/src/+/a523cf48c629d7327cde4a5f36098f47ad50a696

Revert "Mojo: Invitation API" by rockot@chromium.org
https://chromium.googlesource.com/chromium/src/+/6958662bf45564a48de067bf6c906fb8f890a386

[cssom] Make CSSOM APIs always append to the declaration block. by emilio@chromium.org
https://chromium.googlesource.com/chromium/src/+/9d45b94d610d27094ccfd07d20b7283813fa0bf6

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

I'm happy to take a stab at debugging this, but I can't access the trace:

  https://storage.cloud.google.com/chrome-telemetry-output/twitter_2018-05-19_18-46-16_42774.html

Returns a 403 for me. Am I missing something? Is that trace for googlers only? I'll get a build during the week and debug this otherwise I suppose.

Also, not sure whether it's acceptable to take this, my patch is a correctness fix and the regression is a 3ms one in a single page I suspect.

I guess I can try to skip style recalc if the property is logical or the longhand contains logical properties and the value didn't change, or so...

I tried to run the test-case locally, but I can't because of access:

  AccessDeniedException: 403 emilio@chromium.org does not have storage.objects.list access to chrome-partner-telemetry.

I'd like to see what the actual test-case is doing, because if the properties that Twitter happens to set redundantly are logical or have logical aliases then the fix would be more complex... This is all assuming the problem is what I think it is.

For reference, I think the problem is that Twitter is setting declarations that used to be redundant and now cause style recalc (disclaimer: I may very well be wrong, since I don't have access to the test-case).

The first fix I can think of knowing whether the property is either logical or there are logical properties that map to that property, and if this is _not_ the case, pass enough data around to avoid style recalc. That's probably a bit of work, but maybe not too terrible.

However, that wouldn't help if the properties they're setting are indeed logical / have logical properties mapping to it (which is not that uncommon). In that case, if we do want to keep such an optimization to optimize recalc on redundant setters, we'd need to have a mapping from a property to all the related logical / physical properties that may collide, and a fast (O(1)) way to check if a property is in a MutableCSSPropertyValueSet. Right now there's no way to do this easily with the current data structures. The way to do this would be storing a bitfield with storage for all the possible longhands and keep it up-to-date (or something like that). But that adds memory overhead which may or may not be worth it. Assuming that could be done, you could check whether any conflicting longhands exist in the declaration block, and keep doing the optimization to avoid recalc if none exist. Implementing that would be a fairly non-trivial amount of work which I'm unsure I can commit to.

The last option of course would be taking the regression. I still think looking at the absolute numbers which is not huge (assuming it's this single test-case).

In any case, without any kind of look at the test-case, there's not much I can do here. Unassigning for now since I don't think I can work on this without knowing what's going on, but feel free to re-assign if there's a test-case to look at or what not.

Other potential source for slowdown is if we're memmoving much more in declaration blocks, but I'd think that's fairly unlikely to be the root cause of this.

 Issue 846840  has been merged into this issue.

+sullivan may know about the chrome-telemetry-output access controls. I do think they are Googler only.

 Issue 846829  has been merged into this issue.

 Issue 845945  has been merged into this issue.

Well, bad news is that bots are somewhat angry at my change. Good news is that it's not a regression across the board, but only on two test-cases (twitter / wordpress).

Not sure if a revert is the most adequate thing or not here (I think the regression is not that massive but I'm probably not the right person to make a call here). In any case without having any access to the test-cases, even if we revert I wouldn't be able to work on this unfortunately :/.

This issue is marked as a release blocker with no OS labels associated. Please add an appropriate OS label.

All release blocking issues should have OS labels associated to it, so that the issue can tracked and promptly verified, once it gets fixed.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Pretty sure this is platform-independent, updated to reflect that.

For test case access, you can mail telemetry@chromium.org.

I can try to look at this.

Thanks Rune!

For the record, I mailed telemetry@ but got no reply yet.

I've looked at the tracing data from the pinpoint job and I'm not able to spot the difference.

But, this is about the _mean_ value for input_event_latency events in those traces (mean_input_event_latency/twitter), and I think the input event sources are different and not necessarily comparable.

We'd probably have to extract all the latency parts from the traces and compare more than the mean value to see how impactful this is.

The trace shows the average latency is higher too.

Attached input latency graphs. Notice the higher latency from the beginning for the tracing done with the regression.

Deleted: latency-baseline.png 53.7 KB

	latency-baseline.png 53.7 KB View Download

Deleted: latency-regression.png 50.2 KB

	latency-regression.png 50.2 KB View Download

I don't know why you get that sawtooth pattern or why it starts low without the regression.

The update style and layout tree calls are ten times more expensive with the patch: ~0.08ms instead of ~0.008ms.

We lost the optimization for setting the same value over and over again. The example below triggers a style recalc every frame:

<!DOCTYPE html>
<div id="elm"></div>
<script>
  function rAF() {
    elm.style.color = "blue";
    requestAnimationFrame(rAF);
  }
  rAF();
</script>

We should re-introduce that optimization and see if that fixes it.

This might fix it:

https://chromium-review.googlesource.com/c/chromium/src/+/1088615

Reported  issue 850017  for the regression mentioned in #23.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85f8362c2b95986a4d7e574628ee8a4db33d62c1

commit 85f8362c2b95986a4d7e574628ee8a4db33d62c1
Author: Rune Lillesveen <futhark@chromium.org>
Date: Thu Jun 07 15:48:32 2018

Revert "[cssom] Make CSSOM APIs always append to the declaration block."

Revert "Cleanup no longer used argument in MutableCSSPropertyValueSet::SetProperty."

This reverts commit 710b5cc7dbc9d27d33fd4aa363b0b613c8a0803f.
This reverts commit 9d45b94d610d27094ccfd07d20b7283813fa0bf6.

Reason for reverts: suspected performance regression in 845944.

Combined reverts to make it easier to merge to M68.

Bug:  845944 
Change-Id: Id01bb23981e82e25c26ff54a3970150f082d6878
Reviewed-on: https://chromium-review.googlesource.com/1090835
Reviewed-by: Emilio Cobos Álvarez <emilio@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565280}
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/WebKit/LayoutTests/editing/pasteboard/dragstart-contains-default-content-expected.txt
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/WebKit/LayoutTests/editing/pasteboard/onpaste-text-html-expected.txt
[add] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/WebKit/LayoutTests/external/wpt/css/cssom/cssstyledeclaration-setter-order-expected.txt
[add] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/WebKit/LayoutTests/fast/css/no-recalc-on-no-op-inline-style-changes.html
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/WebKit/LayoutTests/fast/dom/MutationObserver/observe-attributes-expected.txt
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/WebKit/LayoutTests/fast/dom/MutationObserver/observe-attributes.html
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/WebKit/LayoutTests/fast/dom/css-set-property-exception-expected.txt
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/WebKit/LayoutTests/fast/events/ondrop-text-html-expected.txt
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/blink/renderer/core/css/css_property_value_set.cc
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/blink/renderer/core/css/css_property_value_set.h
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/blink/renderer/core/css/dom_window_css.cc
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/blink/renderer/core/css/parser/css_parser.cc
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/blink/renderer/core/css/parser/css_parser.h
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/blink/renderer/core/css/parser/css_parser_impl.cc
[modify] https://crrev.com/85f8362c2b95986a4d7e574628ee8a4db33d62c1/third_party/blink/renderer/core/css/parser/css_parser_impl.h

[Auto-generated comment by a script] We noticed that this issue is targeted for M-68; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-68 label, otherwise remove Merge-TBD label. Thanks.

This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), kariahda@(iOS), bhthompson@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Confirmed that twitter input latency test recovered on linux-perf after revert.

Approved. branch:3440

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/677d94d67ff51534e9e7d4ec913fd2392f0ace10

commit 677d94d67ff51534e9e7d4ec913fd2392f0ace10
Author: Rune Lillesveen <futhark@chromium.org>
Date: Mon Jun 11 06:42:13 2018

Revert "[cssom] Make CSSOM APIs always append to the declaration block."

Revert "Cleanup no longer used argument in MutableCSSPropertyValueSet::SetProperty."

This reverts commit 710b5cc7dbc9d27d33fd4aa363b0b613c8a0803f.
This reverts commit 9d45b94d610d27094ccfd07d20b7283813fa0bf6.

Reason for reverts: suspected performance regression in 845944.

Combined reverts to make it easier to merge to M68.

Bug:  845944 
Change-Id: Id01bb23981e82e25c26ff54a3970150f082d6878
Reviewed-on: https://chromium-review.googlesource.com/1090835
Reviewed-by: Emilio Cobos Álvarez <emilio@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#565280}(cherry picked from commit 85f8362c2b95986a4d7e574628ee8a4db33d62c1)
Reviewed-on: https://chromium-review.googlesource.com/1095014
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/branch-heads/3440@{#270}
Cr-Branched-From: 010ddcfda246975d194964ccf20038ebbdec6084-refs/heads/master@{#561733}
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/WebKit/LayoutTests/editing/pasteboard/dragstart-contains-default-content-expected.txt
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/WebKit/LayoutTests/editing/pasteboard/onpaste-text-html-expected.txt
[add] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/WebKit/LayoutTests/external/wpt/css/cssom/cssstyledeclaration-setter-order-expected.txt
[add] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/WebKit/LayoutTests/fast/css/no-recalc-on-no-op-inline-style-changes.html
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/WebKit/LayoutTests/fast/dom/MutationObserver/observe-attributes-expected.txt
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/WebKit/LayoutTests/fast/dom/MutationObserver/observe-attributes.html
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/WebKit/LayoutTests/fast/dom/css-set-property-exception-expected.txt
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/WebKit/LayoutTests/fast/events/ondrop-text-html-expected.txt
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/blink/renderer/core/css/css_property_value_set.cc
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/blink/renderer/core/css/css_property_value_set.h
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/blink/renderer/core/css/dom_window_css.cc
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/blink/renderer/core/css/parser/css_parser.cc
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/blink/renderer/core/css/parser/css_parser.h
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/blink/renderer/core/css/parser/css_parser_impl.cc
[modify] https://crrev.com/677d94d67ff51534e9e7d4ec913fd2392f0ace10/third_party/blink/renderer/core/css/parser/css_parser_impl.h