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

Issue 596464 link

Starred by 2 users
Status: Fixed
Owner:
█ zh...@chromium.org
Last visit > 30 days ago
Closed: Mar 2016
Cc:
█ w...@chromium.org
█ eakuefner@chromium.org
nedngu...@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Many tracing based benchmarks (PageTest based) are failing on Nexus5X

Project Member Reported by nedngu...@google.com, Mar 21 2016 
Example: https://build.chromium.org/p/chromium.perf/builders/Android%20Nexus5X%20Perf%20%282%29/builds/139/steps/smoothness.fling.simple_mobile_sites/logs/stdio

Log:
Traceback (most recent call last):
  File "/b/build/slave/Android_Nexus5X_Perf__2_/build/src/third_party/catapult/telemetry/telemetry/internal/story_runner.py", line 84, in _RunStoryAndProcessErrorIfNeeded
    state.RunStory(results)
  File "/b/build/slave/Android_Nexus5X_Perf__2_/build/src/third_party/catapult/telemetry/telemetry/page/shared_page_state.py", line 329, in RunStory
    self._current_page.Run(self)
  File "/b/build/slave/Android_Nexus5X_Perf__2_/build/src/third_party/catapult/telemetry/telemetry/page/__init__.py", line 83, in Run
    shared_state.page_test.WillNavigateToPage(self, current_tab)
  File "/b/build/slave/Android_Nexus5X_Perf__2_/build/src/tools/perf/measurements/smoothness.py", line 58, in WillNavigateToPage
    self._tbm.WillRunStory(tab.browser.platform)
  File "/b/build/slave/Android_Nexus5X_Perf__2_/build/src/third_party/catapult/telemetry/telemetry/web_perf/timeline_based_measurement.py", line 277, in WillRunStory
    platform.tracing_controller.StartTracing(self._tbm_options.config)
  File "/b/build/slave/Android_Nexus5X_Perf__2_/build/src/third_party/catapult/telemetry/telemetry/core/tracing_controller.py", line 35, in StartTracing
    self._tracing_controller_backend.StartTracing(tracing_config, timeout)
  File "/b/build/slave/Android_Nexus5X_Perf__2_/build/src/third_party/catapult/telemetry/telemetry/internal/platform/tracing_controller_backend.py", line 90, in StartTracing
    if agent.StartAgentTracing(config, timeout):
  File "/b/build/slave/Android_Nexus5X_Perf__2_/build/src/third_party/catapult/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py", line 105, in StartAgentTracing
    started_devtools_tracing = self._StartDevToolsTracing(config, timeout)
  File "/b/build/slave/Android_Nexus5X_Perf__2_/build/src/third_party/catapult/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py", line 82, in _StartDevToolsTracing
    config, config.tracing_category_filter.filter_string, timeout)
  File "/b/build/slave/Android_Nexus5X_Perf__2_/build/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/devtools_client_backend.py", line 334, in StartChromeTracing
    trace_config, custom_categories, timeout)
  File "/b/build/slave/Android_Nexus5X_Perf__2_/build/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/tracing_backend.py", line 126, in StartTracing
    'Tracing.start:\n' + json.dumps(response, indent=2))
TracingUnexpectedResponseException: Inspector returned unexpected response for Tracing.start:
{
  "id": 1, 
  "error": {
    "message": "Tracing is already started", 
    "code": -32603
  }
}

Comment 1 by zh...@chromium.org, Mar 24 2016

Cc: nedngu...@google.com w...@chromium.org
I think I know why this happens now (with help from watk@, thanks!) This happens to all non TBM benchmarks. So if there is leftover /data/local/chrome-trace-config.json file, there is a problem.

For TBM tests, this is fine because all TBM tests are default to startup tracing. And the next test will automatically override that startup config file.

For non-TBM tests, it is default to normal tracing, i.e., start browser, then start tracing. With this leftover startup config file, the browser will be started with tracing on, creating problems.

I will put up a fix shortly.

Comment 2 by zh...@chromium.org, Mar 24 2016

Summary: Many tracing based benchmarks (non-TBM) are failing on Nexus5X (was: Many tracing based benchmarks are failing on Nexus5X)

Comment 3 by zh...@chromium.org, Mar 24 2016

Summary: Many tracing based benchmarks (PageTest based) are failing on Nexus5X (was: Many tracing based benchmarks (non-TBM) are failing on Nexus5X)

Comment 4 by nedngu...@google.com, Mar 25 2016 

I think the proper fix should be when we start the browser, we check whether telemetry "thinks" tracing is on. If "telemetry" doesn't think tracing is on, we should remove the trace file & recover the trace file when browser is closed, or at the end of telemetry test.

Comment 5 by zh...@chromium.org, Mar 25 2016 

Quick fix: https://codereview.chromium.org/1831143003/

The problem is that StopTracing is not part of TearDownState. So for example, when some exception is thrown before stopping tracing, the trace config file will not be cleaned.

To do that, we will need to add API to tracing_controller and do the plumbing. Is that more preferred?

To check the trace file when starting the browser requires similar APIs. I do think checking before starting the browser is more robust.

Comment 6 by nedngu...@google.com, Mar 25 2016 

Zhen: what API do we need to add? I would imagine all we need is:

...some where right before we start browser ...
if not tracing_controller.is_tracing_running:
   remove startup trace file if it exists

Comment 7 by zh...@chromium.org, Mar 25 2016 

Conceptually, that is correct. But how do you know the path of the trace file? It is currently a local variable in chrome_tracing_agent.py. We either need to expose that, or need an API to call into chrome_tracing_agent remove that.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapult/telemetry/telemetry/internal/platform/tracing_agent/chrome_tracing_agent.py&l=22-23

Comment 8 by nedngu...@google.com, Mar 25 2016 

I see, adding chrome_tracing_agent.ClearStarupTracingIfNeeded() SGTM.

Comment 9 by zh...@chromium.org, Mar 25 2016

Status: Fixed (was: Assigned)
Follow-up patch at https://codereview.chromium.org/1827323003/.

The failures due to this error on Nexus 5X have gone (due to the previous quick fix).
►

Sign in to add a comment