New issue
Advanced search Search tips

Issue 647192 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug



Sign in to add a comment

Incremental linking caused 1.5x slowdown of some layout tests on debug

Project Member Reported by yutak@chromium.org, Sep 15 2016

Issue description

So this is an interesting one...

The layout test history indicates enabling incremental linking caused
slow down of more than 3-6 seconds. The original test took 12-15 seconds,
and this slow down let the test exceed the timeout (18 seconds).

Test results:
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=fast%2Fcss3-text%2Fcss3-word-break%2Fword-break-all-ascii.html

The suspected change:
https://chromium.googlesource.com/chromium/src/+/f786d3a73671671237e8ab61d0d97d85a8db7d3c

However, this is not a critical issue because:

(1) We can mark this test as SLOW (and I think we should have done so)
    to address the timeouts,
(2) The timeouts are only observed on debug bots.

So this is basically just an FYI. Feel free to close if you think it's
okay to leave this as is.
 
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 15 2016

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

commit 817f5d2cd3ca0b0a64459a9c88f914340924f2ce
Author: yutak <yutak@chromium.org>
Date: Thu Sep 15 10:58:53 2016

Mark word-break-all-ascii.html as Slow.

This test consistently takes 2 seconds on Release and 10 seconds on
Debug, thus it meets the bar of Slow tests.

TBR=brucedawson@chromium.org
BUG= 647192 
NOTRY=true

Review-Url: https://codereview.chromium.org/2342043002
Cr-Commit-Position: refs/heads/master@{#418830}

[modify] https://crrev.com/817f5d2cd3ca0b0a64459a9c88f914340924f2ce/third_party/WebKit/LayoutTests/SlowTests

Comment 3 by yutak@chromium.org, Sep 15 2016

Note that the similar slowdown might be happening globally, i.e. all other
layout tests may be experiencing at most ~1.5x slowdown on Win Debug. These two
were caught just because they happened to exceed the time limit by the
incremental linking.

Comment 4 by yutak@chromium.org, Sep 15 2016

Summary: Incremental linking caused 1.5x slowdown of some layout tests on debug (was: Incremental linking caused slowdown of one layout test)
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 15 2016

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

commit 89809f9399f14eb55f823572e010cd82d287d490
Author: yutak <yutak@chromium.org>
Date: Thu Sep 15 11:10:28 2016

Mark global-interface-listing.html as Slow.

It takes 1-3 seconds on Release and ~10 seconds on Debug.

TBR=brucedawson@chromium.org
BUG= 647192 
NOTRY=true

Review-Url: https://codereview.chromium.org/2342803002
Cr-Commit-Position: refs/heads/master@{#418832}

[modify] https://crrev.com/89809f9399f14eb55f823572e010cd82d287d490/third_party/WebKit/LayoutTests/SlowTests

Note that I also changed release component builds on Windows to use incremental linking. This should improve incremental build times (sometimes by an enormous amount) but will cost some performance. If the increase in test time is seen to be significant we can revisit this or make it optional, but I think that having incremental linking defaulting to on for component builds is such a huge performance win that we should leave it on if possible.

Comment 7 by yutak@chromium.org, Sep 16 2016

If the slowdown of this degree is expected, it's perfectly fine to close this
bug. I already took care of all affected tests, so I think we are good.
Status: Fixed (was: Untriaged)
I think this is working-as-intended - a run-time slowdown in return for greatly improved build times. Closing as fixed (by crrev.com/2342803002)
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 20 2016

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

commit 05b40466bf7d29ed801f019dbcb9fd1aa64cfc88
Author: pkasting <pkasting@chromium.org>
Date: Thu Oct 20 19:44:30 2016

Update expectations for tests that are slow due to incremental linking.

This adds a missing test and also restricts the SLOW expectation on the existing
tests to only apply to Win Debug.

BUG= 647192 
TEST=none
TBR=mathp

Review-Url: https://chromiumcodereview.appspot.com/2437143003
Cr-Commit-Position: refs/heads/master@{#426566}

[modify] https://crrev.com/05b40466bf7d29ed801f019dbcb9fd1aa64cfc88/third_party/WebKit/LayoutTests/SlowTests

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 14 2017

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

commit 30e1e88c37b843bde2d1623b46a772f0c99b865d
Author: alexmos <alexmos@chromium.org>
Date: Fri Apr 14 19:00:54 2017

Add another variant of webexposed/global-interface-listing.html to SlowTests.

Three other variants of this test are already marked as Slow, but this one
is not, causing redness on the WebKit Win7 (dbg) bot.

BUG= 647192 
NOTRY=true
TBR=brucedawson@chromium.org

Review-Url: https://codereview.chromium.org/2817253003
Cr-Commit-Position: refs/heads/master@{#464767}

[modify] https://crrev.com/30e1e88c37b843bde2d1623b46a772f0c99b865d/third_party/WebKit/LayoutTests/SlowTests

Sign in to add a comment