Unverified Commit 1993a673 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Performance test cleanup (#20652)

* Fix TODO syntax.
* Clarify messages for some timeouts, to aid debugging.
* Increase some other timeouts that were a needlessly short, to reduce sources of flakes.
* Remove some more timeouts that were mostly redundant, to remove complexity.
* Minor style cleanup.
* Remove some dangerous traps (specifically, hide the explicit start/end times in TimedEvent since they shouldn't matter).
parent 03d6f18f
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
import 'dart:async'; import 'dart:async';
import 'package:flutter_driver/flutter_driver.dart'; import 'package:flutter_driver/flutter_driver.dart';
import 'package:test/test.dart' hide TypeMatcher, isInstanceOf; import 'package:test/test.dart' hide TypeMatcher, isInstanceOf;
......
...@@ -37,7 +37,7 @@ Future<TaskResult> task(TaskFunction task) { ...@@ -37,7 +37,7 @@ Future<TaskResult> task(TaskFunction task) {
_isTaskRegistered = true; _isTaskRegistered = true;
// TODO: allow overriding logging. // TODO(ianh): allow overriding logging.
Logger.root.level = Level.ALL; Logger.root.level = Level.ALL;
Logger.root.onRecord.listen((LogRecord rec) { Logger.root.onRecord.listen((LogRecord rec) {
print('${rec.level.name}: ${rec.time}: ${rec.message}'); print('${rec.level.name}: ${rec.time}: ${rec.message}');
...@@ -53,7 +53,7 @@ class _TaskRunner { ...@@ -53,7 +53,7 @@ class _TaskRunner {
final TaskFunction task; final TaskFunction task;
// TODO: workaround for https://github.com/dart-lang/sdk/issues/23797 // TODO(ianh): workaround for https://github.com/dart-lang/sdk/issues/23797
RawReceivePort _keepAlivePort; RawReceivePort _keepAlivePort;
Timer _startTaskTimeout; Timer _startTaskTimeout;
bool _taskStarted = false; bool _taskStarted = false;
...@@ -86,7 +86,7 @@ class _TaskRunner { ...@@ -86,7 +86,7 @@ class _TaskRunner {
_completer.complete(result); _completer.complete(result);
return result; return result;
} on TimeoutException catch (_) { } on TimeoutException catch (_) {
print('Task timed out after $taskTimeout.'); print('Task timed out in framework.dart after $taskTimeout.');
return new TaskResult.failure('Task timed out after $taskTimeout'); return new TaskResult.failure('Task timed out after $taskTimeout');
} finally { } finally {
print('Cleaning up after task...'); print('Cleaning up after task...');
...@@ -106,7 +106,7 @@ class _TaskRunner { ...@@ -106,7 +106,7 @@ class _TaskRunner {
_keepAlivePort = new RawReceivePort(); _keepAlivePort = new RawReceivePort();
// Timeout if nothing bothers to connect and ask us to run the task. // Timeout if nothing bothers to connect and ask us to run the task.
const Duration taskStartTimeout = Duration(seconds: 10); const Duration taskStartTimeout = Duration(seconds: 60);
_startTaskTimeout = new Timer(taskStartTimeout, () { _startTaskTimeout = new Timer(taskStartTimeout, () {
if (!_taskStarted) { if (!_taskStarted) {
logger.severe('Task did not start in $taskStartTimeout.'); logger.severe('Task did not start in $taskStartTimeout.');
......
...@@ -70,13 +70,13 @@ Future<Map<String, dynamic>> runTask(String taskName, { bool silent = false }) a ...@@ -70,13 +70,13 @@ Future<Map<String, dynamic>> runTask(String taskName, { bool silent = false }) a
final Map<String, dynamic> taskResult = final Map<String, dynamic> taskResult =
await isolate.invokeExtension('ext.cocoonRunTask').timeout(taskTimeoutWithGracePeriod); await isolate.invokeExtension('ext.cocoonRunTask').timeout(taskTimeoutWithGracePeriod);
waitingFor = 'task process to exit'; waitingFor = 'task process to exit';
await runner.exitCode.timeout(const Duration(seconds: 1)); await runner.exitCode.timeout(const Duration(seconds: 60));
return taskResult; return taskResult;
} on TimeoutException catch (timeout) { } on TimeoutException catch (timeout) {
runner.kill(ProcessSignal.sigint); runner.kill(ProcessSignal.sigint);
return <String, dynamic>{ return <String, dynamic>{
'success': false, 'success': false,
'reason': 'Timeout waiting for $waitingFor: ${timeout.message}', 'reason': 'Timeout in runner.dart waiting for $waitingFor: ${timeout.message}',
}; };
} finally { } finally {
if (!runnerFinished) if (!runnerFinished)
......
...@@ -13,9 +13,6 @@ import 'package:flutter_devicelab/framework/framework.dart'; ...@@ -13,9 +13,6 @@ import 'package:flutter_devicelab/framework/framework.dart';
import 'package:flutter_devicelab/framework/ios.dart'; import 'package:flutter_devicelab/framework/ios.dart';
import 'package:flutter_devicelab/framework/utils.dart'; import 'package:flutter_devicelab/framework/utils.dart';
/// The maximum amount of time a single microbenchmark is allowed to take.
const Duration _kBenchmarkTimeout = Duration(minutes: 10);
/// Creates a device lab task that runs benchmarks in /// Creates a device lab task that runs benchmarks in
/// `dev/benchmarks/microbenchmarks` reports results to the dashboard. /// `dev/benchmarks/microbenchmarks` reports results to the dashboard.
TaskFunction createMicrobenchmarkTask() { TaskFunction createMicrobenchmarkTask() {
...@@ -52,7 +49,7 @@ TaskFunction createMicrobenchmarkTask() { ...@@ -52,7 +49,7 @@ TaskFunction createMicrobenchmarkTask() {
return await _readJsonResults(flutterProcess); return await _readJsonResults(flutterProcess);
} }
return _run().timeout(_kBenchmarkTimeout); return _run();
} }
final Map<String, double> allResults = <String, double>{}; final Map<String, double> allResults = <String, double>{};
......
...@@ -89,8 +89,6 @@ TaskFunction createBasicMaterialCompileTest() { ...@@ -89,8 +89,6 @@ TaskFunction createBasicMaterialCompileTest() {
/// Measure application startup performance. /// Measure application startup performance.
class StartupTest { class StartupTest {
static const Duration _startupTimeout = Duration(minutes: 5);
const StartupTest(this.testDirectory, { this.reportMetrics = true }); const StartupTest(this.testDirectory, { this.reportMetrics = true });
final String testDirectory; final String testDirectory;
...@@ -110,7 +108,7 @@ class StartupTest { ...@@ -110,7 +108,7 @@ class StartupTest {
'--trace-startup', '--trace-startup',
'-d', '-d',
deviceId, deviceId,
]).timeout(_startupTimeout); ]);
final Map<String, dynamic> data = json.decode(file('$testDirectory/build/start_up_info.json').readAsStringSync()); final Map<String, dynamic> data = json.decode(file('$testDirectory/build/start_up_info.json').readAsStringSync());
if (!reportMetrics) if (!reportMetrics)
......
...@@ -152,7 +152,7 @@ class TimelineSummary { ...@@ -152,7 +152,7 @@ class TimelineSummary {
final TimelineEvent endEvent = events.current; final TimelineEvent endEvent = events.current;
result.add(new TimedEvent( result.add(new TimedEvent(
beginEvent.timestampMicros, beginEvent.timestampMicros,
endEvent.timestampMicros endEvent.timestampMicros,
)); ));
} }
} }
...@@ -197,15 +197,9 @@ class TimelineSummary { ...@@ -197,15 +197,9 @@ class TimelineSummary {
/// Timing information about an event that happened in the event loop. /// Timing information about an event that happened in the event loop.
class TimedEvent { class TimedEvent {
/// Creates a timed event given begin and end timestamps in microseconds. /// Creates a timed event given begin and end timestamps in microseconds.
TimedEvent(this.beginTimeMicros, this.endTimeMicros) TimedEvent(int beginTimeMicros, int endTimeMicros)
: this.duration = new Duration(microseconds: endTimeMicros - beginTimeMicros); : this.duration = new Duration(microseconds: endTimeMicros - beginTimeMicros);
/// The timestamp when the event began.
final int beginTimeMicros;
/// The timestamp when the event ended.
final int endTimeMicros;
/// The duration of the event. /// The duration of the event.
final Duration duration; final Duration duration;
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment