New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 668164 link

Starred by 2 users

Issue metadata

Status: Fixed
Merged: issue 667811
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: ----

Blocking:
issue 468240



Sign in to add a comment

dromaeo.jslibmodifyjquery failing on 4 builders

Project Member Reported by fmea...@chromium.org, Nov 23 2016

Issue description

Project Member

Comment 2 by 42576172...@developer.gserviceaccount.com, Nov 23 2016

Mergedinto: 667811
Status: Duplicate (was: Available)

===== BISECT JOB RESULTS =====
Status: completed


===== SUSPECTED CL(s) =====
Subject : Reland "[wrapper-tracing] Enable flag per default"
Author  : mlippautz
Commit description:
  
BUG= chromium:468240 

Review-Url: https://codereview.chromium.org/2503043002
Cr-Commit-Position: refs/heads/master@{#433823}
Commit  : e23de6962be8b4967127ae95040f32bbb533b4d1
Date    : Tue Nov 22 09:28:34 2016


===== TESTED REVISIONS =====
Revision         Exit Code  Std Dev  N  Good?
chromium@433819  0          N/A      1  good
chromium@433821  0          N/A      1  good
chromium@433822  0          N/A      1  good
chromium@433823  1          N/A      1  bad    <--
chromium@433827  1          N/A      1  bad
chromium@433834  1          N/A      1  bad
chromium@433848  1          N/A      1  bad

Bisect job ran on: winx64_10_perf_bisect
Bug ID: 668164

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests dromaeo.jslibmodifyjquery
Test Metric: jslib/jslib
Relative Change: 0.00%

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64_10_perf_bisect/builds/784
Job details: https://chromeperf.appspot.com/buildbucket_job_status/8995182747579456528


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=6363531111301120

| O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq
|  X  | for more information addressing perf regression bugs. For feedback,
| / \ | file a bug with component Tests>AutoBisect.  Thank you!
Owner: mlippautz@chromium.org
Status: Assigned (was: Duplicate)
mlippautz: Sorry that the bisect comment isn't clear, but it's saying that https://codereview.chromium.org/2503043002 broke the dromaeo.jslibmodifyjquery tests on Windows. The logs are a bit hard to read, but it looks like this causes a browser crash: https://uberchromegw.corp.google.com/i/chromium.perf/builders/Win%2010%20Perf%20(3)/builds/4425/steps/dromaeo.jslibmodifyjquery/logs/stdio

Is it okay to revert?
Well, yes. Ideally we would get one more Canary since I added some more debugging checks which will only make it into the next Canary.

If you feel it's urgent, just revert.

Can you help me with reproducing this locally?
Fadi, are you okay with disabling the test on Windows while mlippautz waits for a Canary?

To reproduce, from a chromium checkout run the "Test Command" from the bisect bot in #2:
src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests dromaeo.jslibmodifyjquery


Looks like this is failing in 64 bit windows 10 and windows 7, not sure about other windows flavors.
Yes. Disabling on all windows for now.
Labels: OS-Windows
Cc: haraken@chromium.org jochen@chromium.org hlopko@chromium.org
Alright, this might actually help if it's reproducing the problems we also see on Canary, which would be awesome. I will try this tomorrow.
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 23 2016

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

commit 98679f8f94d5beb4b7a0715edef85f8ba250f34b
Author: fmeawad <fmeawad@chromium.org>
Date: Wed Nov 23 22:34:33 2016

Disable dromaeo.jslibmodifyjquery on Windows

R=sullivan@chromium.org
BUG= 668164 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq

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

[modify] https://crrev.com/98679f8f94d5beb4b7a0715edef85f8ba250f34b/tools/perf/benchmarks/dromaeo.py

fyi: Couldn't reproduce it on my win7 machine yet.
maybe it just ran out of memory (and your local win7 machine has more memory than the bots)
Status: Started (was: Assigned)
Couldn't repro (likely because of what Jochen said). Issue 668059 might be related and a fix for that is already in flight.
Blocking: 468240
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 25 2016

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

commit 03e5b7ab69b0d8f4dce70b930db57cc9bdddfb8d
Author: mlippautz <mlippautz@chromium.org>
Date: Fri Nov 25 09:28:23 2016

[wrapper-tracing] Untangle non-trivial mixin ctors

Refactor constructors to corresponding ::create methods of non-trivial Oilpan mixins
that could trigger V8 GCs.

BUG=chromium:668059, chromium:668164 

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

[modify] https://crrev.com/03e5b7ab69b0d8f4dce70b930db57cc9bdddfb8d/third_party/WebKit/Source/modules/fetch/Request.cpp
[modify] https://crrev.com/03e5b7ab69b0d8f4dce70b930db57cc9bdddfb8d/third_party/WebKit/Source/modules/fetch/Request.h

FYI: I am enabling the tests again (reverting the original one) and will watch the perf bots today.
Project Member

Comment 16 by bugdroid1@chromium.org, Nov 28 2016

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

commit d74a3000970f43efa51287a3f7f5931abdfa1437
Author: mlippautz <mlippautz@chromium.org>
Date: Mon Nov 28 18:42:25 2016

Revert of Disable dromaeo.jslibmodifyjquery on Windows (patchset #1 id:1 of https://codereview.chromium.org/2529673003/ )

Reason for revert:
Re-enable tests as we fixed a problem with wrapper tracing.

Original issue's description:
> Disable dromaeo.jslibmodifyjquery on Windows
>
> R=sullivan@chromium.org
> BUG= 668164 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
>
> Committed: https://crrev.com/98679f8f94d5beb4b7a0715edef85f8ba250f34b
> Cr-Commit-Position: refs/heads/master@{#434268}

TBR=sullivan@chromium.org,fmeawad@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 668164 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq

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

[modify] https://crrev.com/d74a3000970f43efa51287a3f7f5931abdfa1437/tools/perf/benchmarks/dromaeo.py

Project Member

Comment 17 by bugdroid1@chromium.org, Dec 7 2016

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

commit 6c485a2cf894fd6761fcd3e5ea4905ce773b96b5
Author: mlippautz <mlippautz@chromium.org>
Date: Wed Dec 07 11:51:32 2016

Reland of Disable dromaeo.jslibmodifyjquery on Windows (patchset #1 id:1 of https://codereview.chromium.org/2532653002/ )

Reason for revert:
Still failing with wrapper tracing. Investigating.

Original issue's description:
> Revert of Disable dromaeo.jslibmodifyjquery on Windows (patchset #1 id:1 of https://codereview.chromium.org/2529673003/ )
>
> Reason for revert:
> Re-enable tests as we fixed a problem with wrapper tracing.
>
> Original issue's description:
> > Disable dromaeo.jslibmodifyjquery on Windows
> >
> > R=sullivan@chromium.org
> > BUG= 668164 
> > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
> >
> > Committed: https://crrev.com/98679f8f94d5beb4b7a0715edef85f8ba250f34b
> > Cr-Commit-Position: refs/heads/master@{#434268}
>
> TBR=sullivan@chromium.org,fmeawad@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG= 668164 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
>
> Committed: https://crrev.com/d74a3000970f43efa51287a3f7f5931abdfa1437
> Cr-Commit-Position: refs/heads/master@{#434704}

TBR=sullivan@chromium.org,fmeawad@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 668164 

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

[modify] https://crrev.com/6c485a2cf894fd6761fcd3e5ea4905ce773b96b5/tools/perf/benchmarks/dromaeo.py

Cc: machenb...@chromium.org fmea...@chromium.org sullivan@chromium.org
+machenbach

Alright, still happening with the latest fixes when I turned on the flag. I temporarily disabled the test again.

I am not able to repro locally on my Windows 7 machine (NVIDIA). Since it is doing an official build, is it somehow possible to get a minidump, crash id? 

Or can we arrange a short session on the bots to repro this directly there?
Labels: Pri-1
Project Member

Comment 20 by bugdroid1@chromium.org, Dec 7 2016

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

commit 61aaba1d8be432e52687ea4ff207342189d5d15d
Author: mlippautz <mlippautz@chromium.org>
Date: Wed Dec 07 18:33:41 2016

Revert of Disable dromaeo.jslibmodifyjquery on Windows (patchset #1 id:1 of https://codereview.chromium.org/2557513006/ )

Reason for revert:
Another fix landed in V8 5.7.155. https://codereview.chromium.org/2551973005/

Trying again

Original issue's description:
> Reland of Disable dromaeo.jslibmodifyjquery on Windows (patchset #1 id:1 of https://codereview.chromium.org/2532653002/ )
>
> Reason for revert:
> Still failing with wrapper tracing. Investigating.
>
> Original issue's description:
> > Revert of Disable dromaeo.jslibmodifyjquery on Windows (patchset #1 id:1 of https://codereview.chromium.org/2529673003/ )
> >
> > Reason for revert:
> > Re-enable tests as we fixed a problem with wrapper tracing.
> >
> > Original issue's description:
> > > Disable dromaeo.jslibmodifyjquery on Windows
> > >
> > > R=sullivan@chromium.org
> > > BUG= 668164 
> > > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
> > >
> > > Committed: https://crrev.com/98679f8f94d5beb4b7a0715edef85f8ba250f34b
> > > Cr-Commit-Position: refs/heads/master@{#434268}
> >
> > TBR=sullivan@chromium.org,fmeawad@chromium.org
> > # Not skipping CQ checks because original CL landed more than 1 days ago.
> > BUG= 668164 
> > CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq
> >
> > Committed: https://crrev.com/d74a3000970f43efa51287a3f7f5931abdfa1437
> > Cr-Commit-Position: refs/heads/master@{#434704}
>
> TBR=sullivan@chromium.org,fmeawad@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG= 668164 
>
> Committed: https://crrev.com/6c485a2cf894fd6761fcd3e5ea4905ce773b96b5
> Cr-Commit-Position: refs/heads/master@{#436918}

TBR=sullivan@chromium.org,fmeawad@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 668164 

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

[modify] https://crrev.com/61aaba1d8be432e52687ea4ff207342189d5d15d/tools/perf/benchmarks/dromaeo.py

The failure is still present after relanding yesterday. 

Jochen mentioned that dromaeo tests are sometimes close to OOM. I landed some marking optimizations today in 4f161eac0b1f1b76ef9cd36de5a6dce3c9002a71 which could improve our situation there.

Keeping this on my radar.
Status: Fixed (was: Started)
Just checked the most recent builds of the Windows perf bots and the test is passing now, so I am assuming that it was indeed related to being close to OOM.

Please reopen if you feel this is not resolved.
Cc: yukishiino@chromium.org benhenry@chromium.org mlippautz@chromium.org
 Issue 676366  has been merged into this issue.
Status: Available (was: Fixed)
This is still failing. See https://luci-milo.appspot.com/buildbot/chromium.perf/Win%207%20Intel%20GPU%20Perf/123 for an example build.
I already disabled the test in https://codereview.chromium.org/2596943002
I identified a problem in the dromaeo benchmarks that could also happen here where we have an endless stream of incremental marking events, alternating v8 marking with wrapper tracing marking. We cannot find the fixpoint because after each marking step we allocate new wrappers that require tracing in both "worlds".

I am working on a fix on the v8 side where we force finalization after a certain amount of steps.
Cc: bashi@chromium.org
 Issue 676455  has been merged into this issue.
Project Member

Comment 28 by bugdroid1@chromium.org, Dec 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/a47417b89373c615f9256800cfc803d84ba58378

commit a47417b89373c615f9256800cfc803d84ba58378
Author: mlippautz <mlippautz@chromium.org>
Date: Thu Dec 22 14:50:21 2016

[heap] Ensure progress when incrementally marking wrappers

The problem here is estimating the marking step size for wrapper tracing. If the
steps are too small, we cannot keep up with the mutator creating new wrappers.
The result is an endless stream of incremental marking steps, alternating v8 and
wrappers tracing, without ever finalizing in a GC.

The mitigation here is to abort finding the fix point after 10 incremental
iterations.

A proper solution would track newly created wrappers on the blink side during
wrapper tracing. Will give this more thought after the holidays.

BUG= chromium:668164 ,  chromium:468240 

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

[modify] https://crrev.com/a47417b89373c615f9256800cfc803d84ba58378/src/heap/embedder-tracing.cc
[modify] https://crrev.com/a47417b89373c615f9256800cfc803d84ba58378/src/heap/embedder-tracing.h
[modify] https://crrev.com/a47417b89373c615f9256800cfc803d84ba58378/src/heap/incremental-marking.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Dec 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/73036fb6916e5b2d75db766d695ae7a45af138a4

commit 73036fb6916e5b2d75db766d695ae7a45af138a4
Author: mlippautz <mlippautz@chromium.org>
Date: Thu Dec 22 15:07:36 2016

Revert of [heap] Ensure progress when incrementally marking wrappers (patchset #3 id:60001 of https://codereview.chromium.org/2592403002/ )

Reason for revert:
This won't work because the finalization still checks whether both marking deques are empty, also calling into blink. So we never proceed there.

Original issue's description:
> [heap] Ensure progress when incrementally marking wrappers
>
> The problem here is estimating the marking step size for wrapper tracing. If the
> steps are too small, we cannot keep up with the mutator creating new wrappers.
> The result is an endless stream of incremental marking steps, alternating v8 and
> wrappers tracing, without ever finalizing in a GC.
>
> The mitigation here is to abort finding the fix point after 10 incremental
> iterations.
>
> A proper solution would track newly created wrappers on the blink side during
> wrapper tracing. Will give this more thought after the holidays.
>
> BUG= chromium:668164 ,  chromium:468240 
>
> Review-Url: https://codereview.chromium.org/2592403002
> Cr-Commit-Position: refs/heads/master@{#41923}
> Committed: https://chromium.googlesource.com/v8/v8/+/a47417b89373c615f9256800cfc803d84ba58378

TBR=ulan@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= chromium:668164 ,  chromium:468240 

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

[modify] https://crrev.com/73036fb6916e5b2d75db766d695ae7a45af138a4/src/heap/embedder-tracing.cc
[modify] https://crrev.com/73036fb6916e5b2d75db766d695ae7a45af138a4/src/heap/embedder-tracing.h
[modify] https://crrev.com/73036fb6916e5b2d75db766d695ae7a45af138a4/src/heap/incremental-marking.cc

61a55548c50e01d84ed4aefa396324cbb4039b51 is supposed to fix this. Will take some more hours to roll though.
Project Member

Comment 31 by bugdroid1@chromium.org, Dec 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/fb8b3bcc821cf11c22593d2773c5f06823acd53f

commit fb8b3bcc821cf11c22593d2773c5f06823acd53f
Author: mlippautz <mlippautz@chromium.org>
Date: Fri Dec 23 12:25:38 2016

Reland "[heap] Ensure progress when incrementally marking wrappers"

1) Alternate between processing v8 and wrappers
2) Once v8 is empty, try 3 rounds of finding the fixpoint between v8 and wrappers
3) After that, finalize once v8 marking deque is empty again

Reland fixed: Toggle needs to be IncrementalMarking global as we need to properly
alternate tracing v8 and wrappers.

BUG= chromium:468240 ,  chromium:668164 

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

[modify] https://crrev.com/fb8b3bcc821cf11c22593d2773c5f06823acd53f/src/heap/embedder-tracing.cc
[modify] https://crrev.com/fb8b3bcc821cf11c22593d2773c5f06823acd53f/src/heap/embedder-tracing.h
[modify] https://crrev.com/fb8b3bcc821cf11c22593d2773c5f06823acd53f/src/heap/heap.cc
[modify] https://crrev.com/fb8b3bcc821cf11c22593d2773c5f06823acd53f/src/heap/heap.h
[modify] https://crrev.com/fb8b3bcc821cf11c22593d2773c5f06823acd53f/src/heap/incremental-marking.cc
[modify] https://crrev.com/fb8b3bcc821cf11c22593d2773c5f06823acd53f/src/heap/incremental-marking.h
[modify] https://crrev.com/fb8b3bcc821cf11c22593d2773c5f06823acd53f/src/heap/mark-compact.cc

The should be fixed now, will re-enable the test.
Components: Blink>JavaScript>GC Blink>Bindings
Status: Started (was: Available)
Project Member

Comment 34 by bugdroid1@chromium.org, Jan 2 2017

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

commit ec2ca57b674da058aa05b3ac04806b9e9bd2f7bf
Author: mlippautz <mlippautz@chromium.org>
Date: Mon Jan 02 11:35:21 2017

Enable dromaeo.jslibmodifyjquery on Windows

Fix landed in fb8b3bcc821cf11c22593d2773c5f06823acd53f.

BUG= chromium:668164 
TBR=sullivan@chromium.org,fmeawad@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.perf:linux_perf_cq;master.tryserver.chromium.perf:mac_retina_perf_cq;master.tryserver.chromium.perf:winx64_10_perf_cq

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

[modify] https://crrev.com/ec2ca57b674da058aa05b3ac04806b9e9bd2f7bf/tools/perf/benchmarks/dromaeo.py

Status: Fixed (was: Started)

Sign in to add a comment