diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart index 33b2193496d..b8df7d6429d 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/skia_object_cache.dart @@ -351,16 +351,16 @@ class SkiaObjectBox R debugReferrer, T initialValue, this._resurrector) { assert(!browserSupportsFinalizationRegistry); _initialize(debugReferrer, initialValue); - SkiaObjects.manageExpensive(this); - } - - void _initialize(R debugReferrer, T initialValue) { - _update(initialValue); if (Instrumentation.enabled) { Instrumentation.instance.incrementCounter( '${_skDeletable?.constructor.name} created', ); } + SkiaObjects.manageExpensive(this); + } + + void _initialize(R debugReferrer, T initialValue) { + _update(initialValue); if (assertionsEnabled) { debugReferrers.add(debugReferrer); } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/profiler.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/profiler.dart index f8205054d04..ed50fa3bd9c 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/profiler.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/profiler.dart @@ -230,7 +230,20 @@ class Instrumentation { /// Whether instrumentation is enabled. /// /// Check this value before calling any other methods in this class. - static const bool enabled = const bool.fromEnvironment( + static bool get enabled => _enabled; + static set enabled(bool value) { + if (_enabled == value) { + return; + } + + if (!value) { + _instance._counters.clear(); + _instance._printTimer = null; + } + + _enabled = value; + } + static bool _enabled = const bool.fromEnvironment( 'FLUTTER_WEB_ENABLE_INSTRUMENTATION', defaultValue: false, ); @@ -245,7 +258,7 @@ class Instrumentation { static void _checkInstrumentationEnabled() { if (!enabled) { - throw Exception( + throw StateError( 'Cannot use Instrumentation unless it is enabled. ' 'You can enable it by setting the `FLUTTER_WEB_ENABLE_INSTRUMENTATION` ' 'environment variable to true, or by passing ' @@ -255,7 +268,10 @@ class Instrumentation { } } + Map get debugCounters => _counters; final Map _counters = {}; + + Timer? get debugPrintTimer => _printTimer; Timer? _printTimer; /// Increments the count of a particular event by one. @@ -266,7 +282,11 @@ class Instrumentation { _printTimer ??= Timer( const Duration(seconds: 2), () { + if (_printTimer == null || !_enabled) { + return; + } final StringBuffer message = StringBuffer('Engine counters:\n'); + // Entries are sorted for readability and testability. final List> entries = _counters.entries.toList() ..sort((MapEntry a, MapEntry b) { return a.key.compareTo(b.key); diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart index c639e334b00..36e18e6e4dc 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/skia_objects_cache_test.dart @@ -11,6 +11,7 @@ import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart'; import '../matchers.dart'; +import '../spy.dart'; import 'common.dart'; void main() { @@ -114,44 +115,64 @@ void _tests() { group(SkiaObjectBox, () { test('Records stack traces and respects refcounts', () async { - TestSkDeletable.deleteCount = 0; - TestBoxWrapper.resurrectCount = 0; - final TestBoxWrapper original = TestBoxWrapper(); + final ZoneSpy spy = ZoneSpy(); + spy.run(() { + Instrumentation.enabled = true; + TestSkDeletable.deleteCount = 0; + TestBoxWrapper.resurrectCount = 0; + final TestBoxWrapper original = TestBoxWrapper(); - expect(original.box.debugGetStackTraces().length, 1); - expect(original.box.refCount, 1); - expect(original.box.isDeletedPermanently, false); + expect(original.box.debugGetStackTraces().length, 1); + expect(original.box.refCount, 1); + expect(original.box.isDeletedPermanently, false); - final TestBoxWrapper clone = original.clone(); - expect(clone.box, same(original.box)); - expect(clone.box.debugGetStackTraces().length, 2); - expect(clone.box.refCount, 2); - expect(original.box.debugGetStackTraces().length, 2); - expect(original.box.refCount, 2); - expect(original.box.isDeletedPermanently, false); + final TestBoxWrapper clone = original.clone(); + expect(clone.box, same(original.box)); + expect(clone.box.debugGetStackTraces().length, 2); + expect(clone.box.refCount, 2); + expect(original.box.debugGetStackTraces().length, 2); + expect(original.box.refCount, 2); + expect(original.box.isDeletedPermanently, false); - original.dispose(); + original.dispose(); - testCollector.collectNow(); - expect(TestSkDeletable.deleteCount, 0); + testCollector.collectNow(); + expect(TestSkDeletable.deleteCount, 0); - expect(clone.box.debugGetStackTraces().length, 1); - expect(clone.box.refCount, 1); - expect(original.box.debugGetStackTraces().length, 1); - expect(original.box.refCount, 1); + spy.fakeAsync.elapse(const Duration(seconds: 2)); + expect( + spy.printLog, + ['Engine counters:\n' + ' TestSkDeletable created: 1\n'], + ); - clone.dispose(); - expect(clone.box.debugGetStackTraces().length, 0); - expect(clone.box.refCount, 0); - expect(original.box.debugGetStackTraces().length, 0); - expect(original.box.refCount, 0); - expect(original.box.isDeletedPermanently, true); + expect(clone.box.debugGetStackTraces().length, 1); + expect(clone.box.refCount, 1); + expect(original.box.debugGetStackTraces().length, 1); + expect(original.box.refCount, 1); - testCollector.collectNow(); - expect(TestSkDeletable.deleteCount, 1); - expect(TestBoxWrapper.resurrectCount, 0); + clone.dispose(); + expect(clone.box.debugGetStackTraces().length, 0); + expect(clone.box.refCount, 0); + expect(original.box.debugGetStackTraces().length, 0); + expect(original.box.refCount, 0); + expect(original.box.isDeletedPermanently, true); - expect(() => clone.box.unref(clone), throwsAssertionError); + testCollector.collectNow(); + expect(TestSkDeletable.deleteCount, 1); + expect(TestBoxWrapper.resurrectCount, 0); + + expect(() => clone.box.unref(clone), throwsAssertionError); + spy.printLog.clear(); + spy.fakeAsync.elapse(const Duration(seconds: 2)); + expect( + spy.printLog, + ['Engine counters:\n' + ' TestSkDeletable created: 1\n' + ' TestSkDeletable deleted: 1\n'], + ); + Instrumentation.enabled = false; + }); }); test('Can resurrect Skia objects', () async { diff --git a/engine/src/flutter/lib/web_ui/test/engine/profiler_test.dart b/engine/src/flutter/lib/web_ui/test/engine/profiler_test.dart index c2fb54e7fb0..2f7ec51bd7a 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/profiler_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/profiler_test.dart @@ -12,11 +12,23 @@ import 'package:test/test.dart'; import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart'; +import '../spy.dart'; + void main() { internalBootstrapBrowserTest(() => testMain); } void testMain() { + group('$Profiler', () { + _profilerTests(); + }); + + group('$Instrumentation', () { + _instrumentationTests(); + }); +} + +void _profilerTests() { setUp(() { Profiler.isBenchmarkMode = true; Profiler.ensureInitialized(); @@ -73,6 +85,52 @@ void testMain() { }); } +void _instrumentationTests() { + setUp(() { + Instrumentation.enabled = false; + }); + + tearDown(() { + Instrumentation.enabled = false; + }); + + test('when disabled throws instead of initializing', () { + expect(() => Instrumentation.instance, throwsStateError); + }); + + test('when disabled throws instead of incrementing counter', () { + Instrumentation.enabled = true; + final Instrumentation instrumentation = Instrumentation.instance; + Instrumentation.enabled = false; + expect(() => instrumentation.incrementCounter('test'), throwsStateError); + }); + + test('when enabled increments counter', () { + final ZoneSpy spy = ZoneSpy(); + spy.run(() { + Instrumentation.enabled = true; + final Instrumentation instrumentation = Instrumentation.instance; + expect(instrumentation.debugPrintTimer, isNull); + instrumentation.incrementCounter('foo'); + expect(instrumentation.debugPrintTimer, isNotNull); + instrumentation.incrementCounter('foo'); + instrumentation.incrementCounter('bar'); + expect(spy.printLog, isEmpty); + + expect(instrumentation.debugPrintTimer, isNotNull); + spy.fakeAsync.elapse(const Duration(seconds: 2)); + expect(instrumentation.debugPrintTimer, isNull); + expect(spy.printLog, hasLength(1)); + expect( + spy.printLog.single, + 'Engine counters:\n' + ' bar: 1\n' + ' foo: 2\n', + ); + }); + }); +} + class BenchmarkDatapoint { BenchmarkDatapoint(this.name, this.value); diff --git a/engine/src/flutter/lib/web_ui/test/spy.dart b/engine/src/flutter/lib/web_ui/test/spy.dart index 6c02645bf74..8df49dcac10 100644 --- a/engine/src/flutter/lib/web_ui/test/spy.dart +++ b/engine/src/flutter/lib/web_ui/test/spy.dart @@ -2,8 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:async'; import 'dart:typed_data'; +import 'package:quiver/testing/async.dart'; import 'package:ui/src/engine.dart' hide window; import 'package:ui/ui.dart'; @@ -67,3 +69,22 @@ class PlatformMessagesSpy { window.onPlatformMessage = _backup; } } + +/// Runs code in a [FakeAsync] zone and spies on what's going on in it. +class ZoneSpy { + final FakeAsync fakeAsync = FakeAsync(); + final List printLog = []; + + dynamic run(dynamic Function() function) { + final ZoneSpecification printInterceptor = ZoneSpecification( + print: (Zone self, ZoneDelegate parent, Zone zone, String line) { + printLog.add(line); + }, + ); + return Zone.current.fork(specification: printInterceptor).run(() { + return fakeAsync.run((FakeAsync self) { + return function(); + }); + }); + } +}