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

Issue 848989 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Avoid unnecessary calls to GetSystemInfo() in telemetry tests

Project Member Reported by sadrul@chromium.org, Jun 2 2018

Issue description

Telemetry tests check the supports_system_info property before calling GetSystemInfo(). However, supports_system_info makes a call to GetSystemInfo(), and immediately discards the response. Each call to GetSystemInfo() makes a request to the browser. So, instead of making two calls to the browser for GetSystemInfo(), make a single call, so that we can avoid unnecessary interactions with the browser from the test.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/29c2214918048a603d152b178c0d51756c80dbcd

commit 29c2214918048a603d152b178c0d51756c80dbcd
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Sat Jun 02 05:51:34 2018

Permit always calling GetSystemInfo().

In preparation for removing supports_system_info, allow always calling
GetSystemInfo(), so that code in chrome can be updated to stop checking
supports_system_info, and always calling GetSystemInfo().

BUG= chromium:848989 

Change-Id: I5ba21e8aff1f7a89c223ad393a13b786f454a3a7
Reviewed-on: https://chromium-review.googlesource.com/1084071
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>

[modify] https://crrev.com/29c2214918048a603d152b178c0d51756c80dbcd/telemetry/telemetry/internal/backends/browser_backend.py

Project Member

Comment 2 by bugdroid1@chromium.org, Jun 2 2018

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

commit 8ee0edaf2c481aa166a64a1509fd8cfd55e768a9
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Sat Jun 02 07:19:05 2018

Roll src/third_party/catapult caee0de..29c2214 (1 commits)

https://chromium.googlesource.com/catapult.git/+log/caee0de..29c2214


git log caee0de..29c2214 --date=short --no-merges --format='%ad %ae %s'
2018-06-02 sadrul@chromium.org Permit always calling GetSystemInfo().


Created with:
  gclient setdep -r src/third_party/catapult@29c2214

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:848989 
TBR=sullivan@chromium.org

Change-Id: Ib9daf12698ca562236be0202d0edc887610a02bb
Reviewed-on: https://chromium-review.googlesource.com/1084076
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#563935}
[modify] https://crrev.com/8ee0edaf2c481aa166a64a1509fd8cfd55e768a9/DEPS

Project Member

Comment 3 by bugdroid1@chromium.org, Jun 2 2018

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

commit a848bd1b5bdc0699f554da7719f97878a9f4d513
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Sat Jun 02 07:36:50 2018

gpu tests: Avoid calling supports_system_info.

Directly call GetSystemInfo(), and check for null, instead of checking
supports_system_info property.

TBR=nednguyen@ for trivial change in tools/perf/
BUG= 848989 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I5feee8b5768d83da0ba902953bc5cf85c36a27bc
Reviewed-on: https://chromium-review.googlesource.com/1084057
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563936}
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/cloud_storage_integration_test_base.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/context_lost_integration_test.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/gpu_process_integration_test.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/gpu_test_expectations.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/gpu_test_expectations_unittest.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/info_collection_test.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/test_expectations_unittest.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/verify_all_expectations_unittest.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/tools/perf/page_sets/webgl_supported_shared_state.py

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 2 2018

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

commit a848bd1b5bdc0699f554da7719f97878a9f4d513
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Sat Jun 02 07:36:50 2018

gpu tests: Avoid calling supports_system_info.

Directly call GetSystemInfo(), and check for null, instead of checking
supports_system_info property.

TBR=nednguyen@ for trivial change in tools/perf/
BUG= 848989 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I5feee8b5768d83da0ba902953bc5cf85c36a27bc
Reviewed-on: https://chromium-review.googlesource.com/1084057
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563936}
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/cloud_storage_integration_test_base.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/context_lost_integration_test.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/gpu_process_integration_test.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/gpu_test_expectations.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/gpu_test_expectations_unittest.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/info_collection_test.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/test_expectations_unittest.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/content/test/gpu/gpu_tests/verify_all_expectations_unittest.py
[modify] https://crrev.com/a848bd1b5bdc0699f554da7719f97878a9f4d513/tools/perf/page_sets/webgl_supported_shared_state.py

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 2 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/54e864df3f870ba7c3b9a47ce392eb1a6db57edc

commit 54e864df3f870ba7c3b9a47ce392eb1a6db57edc
Author: Sadrul Habib Chowdhury <sadrul@chromium.org>
Date: Sat Jun 02 21:03:54 2018

Remove supports_system_info.

Remove supports_system_info property, and updated the callers to
directly call GetSystemInfo() and check for its existence instead.
supports_system_info makes a call to GetSystemInfo(), which triggers a
request back to the browser, and immediately discards the response. So,
instead of making two calls to the browser for GetSystemInfo(), make a
single call.

When running locally on a Z840, this saves ~1.5 seconds for each test.

BUG= chromium:848989 

Change-Id: Ie9d0a3d5ccd13f12722395c6e8d77feb4e1ae9a4
Reviewed-on: https://chromium-review.googlesource.com/1081058
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Ned Nguyen <nednguyen@google.com>
Reviewed-by: Kenneth Russell <kbr@chromium.org>

[modify] https://crrev.com/54e864df3f870ba7c3b9a47ce392eb1a6db57edc/telemetry/telemetry/internal/backends/browser_backend.py
[modify] https://crrev.com/54e864df3f870ba7c3b9a47ce392eb1a6db57edc/telemetry/telemetry/internal/backends/chrome/chrome_browser_backend.py
[modify] https://crrev.com/54e864df3f870ba7c3b9a47ce392eb1a6db57edc/telemetry/telemetry/testing/fakes/__init__.py
[modify] https://crrev.com/54e864df3f870ba7c3b9a47ce392eb1a6db57edc/telemetry/telemetry/internal/browser/browser_unittest.py
[modify] https://crrev.com/54e864df3f870ba7c3b9a47ce392eb1a6db57edc/telemetry/telemetry/internal/browser/browser.py

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 3 2018

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

commit 61252bb23004ce629ff80ef8944812744158d49d
Author: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Sun Jun 03 00:06:00 2018

Roll src/third_party/catapult 29c2214..54e864d (1 commits)

https://chromium.googlesource.com/catapult.git/+log/29c2214..54e864d


git log 29c2214..54e864d --date=short --no-merges --format='%ad %ae %s'
2018-06-02 sadrul@chromium.org Remove supports_system_info.


Created with:
  gclient setdep -r src/third_party/catapult@54e864d

The AutoRoll server is located here: https://catapult-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:848989 
TBR=sullivan@chromium.org

Change-Id: I924a862762d5e1a7441f7fb6471dc895dd8e3fa8
Reviewed-on: https://chromium-review.googlesource.com/1084172
Reviewed-by: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Commit-Queue: catapult-chromium-autoroll <catapult-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#563955}
[modify] https://crrev.com/61252bb23004ce629ff80ef8944812744158d49d/DEPS

Status: Fixed (was: Started)

Sign in to add a comment