autotest: profiler is broken |
||||||||||||
Issue description"background" was removed in https://chromium-review.googlesource.com/c/562586/, which broke the profiler. The profiler is needed for Feedback Guided Optimization. Without it there will be a 5-20% overall performance drop. The waterfalls have hard dependency on this. Unfortunately it was broken before the branch point. Canary / Dev (master-chromium-pfq) and Beta (samus-chrome-preflight-branch) will break around Aug. 1. UnhandledTestFail: Unhandled TypeError: run() got an unexpected keyword argument 'background' Traceback (most recent call last): File "/usr/local/autotest/client/common_lib/test.py", line 818, in _call_test_function return func(*args, **dargs) File "/usr/local/autotest/client/common_lib/test.py", line 471, in execute dargs) File "/usr/local/autotest/client/common_lib/test.py", line 348, in _call_run_once_with_retry postprocess_profiled_run, args, dargs) File "/usr/local/autotest/client/common_lib/test.py", line 376, in _call_run_once *args, **dargs) File "/usr/local/autotest/client/common_lib/test.py", line 487, in run_once_profiling profilers.before_start(self) File "/usr/local/autotest/server/profilers.py", line 214, in before_start self._run_clients(test, self._get_hosts(host)) File "/usr/local/autotest/server/profilers.py", line 191, in _run_clients at.run(control_script, background=True) TypeError: run() got an unexpected keyword argument 'background'
,
Jul 26 2017
Duh. I was pretty sure I looked around for all uses...
,
Jul 26 2017
There are plenty users (Ugh! Each one of them is WRONG): https://cs.corp.google.com/search/?q=background%3DTrue&m=100&sq=package:chromeos+file:src/third_party/autotest/files&type=cs How did I miss this? :'( Anyway, reverting first.
,
Jul 26 2017
,
Jul 26 2017
,
Jul 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/cb4f54963f755520254a01d9c8a6bcde65808853 commit cb4f54963f755520254a01d9c8a6bcde65808853 Author: Prathmesh Prabhu <pprabhu@chromium.org> Date: Fri Jul 28 00:55:42 2017 Revert "[autotest] Drop server-side support for multi-section client tests" This reverts commit 5d1c546c9611c4bd17c7ede9f9aa7e0e83097c4b. Reason for revert: On top of CL:562586, which needs to be reverted Original change's description: > [autotest] Drop server-side support for multi-section client tests > > This CL removes the server side autotest support for client tests with > multiple steps. Follow up CLs will remove the client side support. > > This is a dead feature that duplicates functionality intended to be used > via server-side tests. > > BUG= chromium:684311 > TEST=(1) unittests > (2) Run a client test locally. > > Change-Id: I7df7a1bfb2bdba8a26b45f6e7b341922f45804e4 > Reviewed-on: https://chromium-review.googlesource.com/562587 > Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org> > Tested-by: Prathmesh Prabhu <pprabhu@chromium.org> > Reviewed-by: Xixuan Wu <xixuan@chromium.org> BUG= chromium:684311 BUG= chromium:749198 Change-Id: Id777838151c9a26015fe9bd8c5ee6b7e35fd638b Reviewed-on: https://chromium-review.googlesource.com/586432 Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org> Tested-by: Prathmesh Prabhu <pprabhu@chromium.org> Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org> [modify] https://crrev.com/cb4f54963f755520254a01d9c8a6bcde65808853/server/autotest.py
,
Jul 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/beaee90065f9352067981d8cb1bc631d24c5a17f commit beaee90065f9352067981d8cb1bc631d24c5a17f Author: Prathmesh Prabhu <pprabhu@chromium.org> Date: Fri Jul 28 00:55:42 2017 Revert "[autotest] Drop support for running background autotest tests." This reverts commit 1722554376440b8d7d0068e83cdce7cc923b23a4. Reason for revert: There are still users of this feature in the wild, which I missed. Original change's description: > Because feature creep. There is only one user, and there has been only > one user for years. This is adding maintenance burden so drop the > feature. > Also, executing code on the DUT that outlives the server-side test that > triggered is unsupported. > BUG= chromium:684311 > TEST=(1) unittests > (2) Run a client test locally > Change-Id: I2b7290e44534c6f961bd97442f12d9c0917f4d02 > Reviewed-on: https://chromium-review.googlesource.com/562586 > Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org> > Tested-by: Prathmesh Prabhu <pprabhu@chromium.org> > Reviewed-by: Xixuan Wu <xixuan@chromium.org> BUG= chromium:684311 BUG= chromium:749198 Change-Id: Ic37567e9bc8584b080010e7a501fe6204c7df66f Reviewed-on: https://chromium-review.googlesource.com/587333 Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org> Tested-by: Prathmesh Prabhu <pprabhu@chromium.org> Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org> Reviewed-by: Ting-Yuan Huang <laszio@chromium.org> [modify] https://crrev.com/beaee90065f9352067981d8cb1bc631d24c5a17f/server/autotest.py [modify] https://crrev.com/beaee90065f9352067981d8cb1bc631d24c5a17f/server/autotest_unittest.py
,
Jul 28 2017
From the comment in OP, do we need merges here?
,
Jul 28 2017
this is still happening on the R61 branch. Has the revert been done there?
,
Jul 28 2017
,
Jul 28 2017
Yes, we need to merge to R61.
,
Jul 28 2017
Verified on chell-chrome-pfq on master. The profiler is working perfectly.
,
Jul 29 2017
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 1 2017
Still happening on R61... https://tests.corp.google.com/invocations?searchFor=afdo_record
,
Aug 1 2017
,
Aug 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/b8e428f992b16c8ddd5a1e0eaea1cb8b5645b73c commit b8e428f992b16c8ddd5a1e0eaea1cb8b5645b73c Author: Prathmesh Prabhu <pprabhu@chromium.org> Date: Tue Aug 01 21:07:47 2017 Revert "[autotest] Drop server-side support for multi-section client tests" This reverts commit 5d1c546c9611c4bd17c7ede9f9aa7e0e83097c4b. Reason for revert: On top of CL:562586, which needs to be reverted Original change's description: > [autotest] Drop server-side support for multi-section client tests > > This CL removes the server side autotest support for client tests with > multiple steps. Follow up CLs will remove the client side support. > > This is a dead feature that duplicates functionality intended to be used > via server-side tests. > > BUG= chromium:684311 > TEST=(1) unittests > (2) Run a client test locally. > > Change-Id: I7df7a1bfb2bdba8a26b45f6e7b341922f45804e4 > Reviewed-on: https://chromium-review.googlesource.com/562587 > Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org> > Tested-by: Prathmesh Prabhu <pprabhu@chromium.org> > Reviewed-by: Xixuan Wu <xixuan@chromium.org> BUG= chromium:684311 BUG= chromium:749198 Change-Id: Id777838151c9a26015fe9bd8c5ee6b7e35fd638b Reviewed-on: https://chromium-review.googlesource.com/586432 Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org> Tested-by: Prathmesh Prabhu <pprabhu@chromium.org> Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/596835 Commit-Queue: Prathmesh Prabhu <pprabhu@chromium.org> [modify] https://crrev.com/b8e428f992b16c8ddd5a1e0eaea1cb8b5645b73c/server/autotest.py
,
Aug 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/e3d2d75310d28f0311e850823f9399aba1457968 commit e3d2d75310d28f0311e850823f9399aba1457968 Author: Prathmesh Prabhu <pprabhu@chromium.org> Date: Tue Aug 01 21:08:05 2017 Revert "[autotest] Drop support for running background autotest tests." This reverts commit 1722554376440b8d7d0068e83cdce7cc923b23a4. Reason for revert: There are still users of this feature in the wild, which I missed. Original change's description: > Because feature creep. There is only one user, and there has been only > one user for years. This is adding maintenance burden so drop the > feature. > Also, executing code on the DUT that outlives the server-side test that > triggered is unsupported. > BUG= chromium:684311 > TEST=(1) unittests > (2) Run a client test locally > Change-Id: I2b7290e44534c6f961bd97442f12d9c0917f4d02 > Reviewed-on: https://chromium-review.googlesource.com/562586 > Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org> > Tested-by: Prathmesh Prabhu <pprabhu@chromium.org> > Reviewed-by: Xixuan Wu <xixuan@chromium.org> BUG= chromium:684311 BUG= chromium:749198 Change-Id: Ic37567e9bc8584b080010e7a501fe6204c7df66f Reviewed-on: https://chromium-review.googlesource.com/587333 Commit-Ready: Prathmesh Prabhu <pprabhu@chromium.org> Tested-by: Prathmesh Prabhu <pprabhu@chromium.org> Reviewed-by: Prathmesh Prabhu <pprabhu@chromium.org> Reviewed-by: Ting-Yuan Huang <laszio@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/596836 Commit-Queue: Prathmesh Prabhu <pprabhu@chromium.org> [modify] https://crrev.com/e3d2d75310d28f0311e850823f9399aba1457968/server/autotest.py [modify] https://crrev.com/e3d2d75310d28f0311e850823f9399aba1457968/server/autotest_unittest.py
,
Aug 1 2017
,
Aug 3 2017
Closing. Please reopen it if its not fixed. Thanks! |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by laszio@chromium.org
, Jul 26 2017