New issue
Advanced search Search tips

Issue 872675 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 4
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Regression



Sign in to add a comment

3.7%-5.2% regression in rendering.mobile at 581613:581665

Project Member Reported by toyoshim@chromium.org, Aug 9

Issue description

See the link to graphs below.
 
All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=872675

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


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

Android Nexus6 WebView Perf
Cc: digit@google.com kojii@chromium.org
Owner: kojii@chromium.org
Status: Assigned (was: Untriaged)
📍 Found significant differences after each of 2 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/128ce008640000

android: Disable thinLTO for 32-bit ARM builds. by digit@google.com
https://chromium.googlesource.com/chromium/src/+/069a1be166b09bd5e640d9eebda50bb4a9b4337e
43.12 → 45.05 (+1.931)

Fix ShapeResult kMaxCharacterIndex by kojii@chromium.org
https://chromium.googlesource.com/chromium/src/+/7e19e900c9d1b48baca21a11b750d53b380da492
44.93 → 44.48 (-0.4515)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions
Owner: digit@google.com
If I read the pinpoint in #3 correctly:
* 581627 made it a bit worse
* 581631 made it a bit better
Does my reading look correct?

I have a mixed feeling in my CL; it is supposed to make the normal case faster while sacrifice an edge case, and do more correct job, so wondering, is the smaller number better for this test, or worse. From the graph in #1, I believe the smaller is the better.

digit@, if your change is supposed to make things faster and my reading seems to be opposite, please let me know, I will revert my patch.

Unfortunately, toyoshim@, the test owner, is OOO until 14th.
For the record, my change was required to fix a startup runtime crash (see http://crbug.com/871722 for details) on older Android devices. It is a work-around for a clang linker bug, it will be reverted when we roll a newer clang. See  http://crbug.com/872611 .

Status: Fixed (was: Assigned)
ThinLTO has been re-enabled after a new clang roll (see  http://crbug.com/872611 ).
Hence, this should be fixed by now.

Sign in to add a comment