Improve performance of leak tracker. (#209)

Move analysis of creation stack trace from start of tracking to leak filtering, to perform just for leaking objects.

.vscode/settings.json

@@ -1,15 +1,16 @@
 {
     // Set current working directory to devtools_app.
     "terminal.integrated.cwd": "pkgs/leak_tracker",
-    // The extension `streetsidesoftware.code-spell-checker` automatically adds words to
-    // this list if user chooses the quick fix to add the word to the dictionary.
-    // See https://marketplace.visualstudio.com/items?itemName=streetsidesoftware.code-spell-checker.
     "cSpell.words": [
         "baselining",
+        "callstack",
+        "cupertino",
+        "finalizer",
         "gced",
-        "layerlens",
+        "pkgs",
         "polina",
-        "pubspec",
-        "snapshotting"
+        "polinach",
+        "sublist",
+        "unawaited"
     ],
 }

pkgs/leak_tracker/CHANGELOG.md

@@ -1,3 +1,7 @@
+# 10.0.3
+
+* Improve performance of the case with ignored test helpers.
+
 # 10.0.2
 
 * Require Dart SDK 3.2.0 or later.

pkgs/leak_tracker/analysis_options.yaml

@@ -10,4 +10,6 @@ linter:
     - avoid_print
     - comment_references
     - only_throw_errors
+    - sort_constructors_first
+    - sort_unnamed_constructors_first
     - unawaited_futures

pkgs/leak_tracker/lib/src/leak_tracking/_leak_filter.dart

@@ -10,6 +10,7 @@ import 'primitives/model.dart';
 class LeakFilter {
   final Map<PhaseSettings, _PhaseLeakFilter> _phases = {};
 
+  /// Returns true if the leak should be reported.
   bool shouldReport(LeakType leakType, ObjectRecord record) {
     final filter = _phases.putIfAbsent(
       record.phase,
@@ -28,24 +29,40 @@ class _PhaseLeakFilter {
   final PhaseSettings phase;
 
   bool shouldReport(LeakType leakType, ObjectRecord record) {
+    final bool result;
     switch (leakType) {
       case LeakType.notDisposed:
-        return _shouldReport(
+        result = _shouldReportByTypeAndClass(
           leakType,
           record,
           phase.ignoredLeaks.notDisposed,
         );
       case LeakType.notGCed:
       case LeakType.gcedLate:
-        return _shouldReport(
+        result = _shouldReportByTypeAndClass(
           leakType,
           record,
           phase.ignoredLeaks.experimentalNotGCed,
         );
     }
+
+    // Check for test helpers should happen only in case of leak because
+    // it is performance heavy.
+    // TODO(polina-c):  add a test to ensure that the test helper check does not
+    // run when this is false
+    // https://github.com/dart-lang/leak_tracker/issues/210
+    final shouldCheckCreator =
+        phase.ignoredLeaks.createdByTestHelpers && result;
+
+    if (!shouldCheckCreator) return result;
+
+    final createdByTestHelpers =
+        record.creationChecker?.createdByTestHelpers ?? false;
+    return !createdByTestHelpers;
   }
 
-  bool _shouldReport(
+  /// Returns whether the leak should be reported based on its type and class.
+  bool _shouldReportByTypeAndClass(
     LeakType leakType,
     ObjectRecord record,
     IgnoredLeaksSet ignoredLeaks,

pkgs/leak_tracker/lib/src/leak_tracking/_object_record.dart

@@ -5,6 +5,7 @@
 import '../shared/_primitives.dart';
 import '../shared/shared_model.dart';
 import 'primitives/_gc_counter.dart';
+import 'primitives/_test_helper_detector.dart';
 import 'primitives/model.dart';
 
 /// Information about an object, tracked for leaks.
@@ -13,11 +14,13 @@ class ObjectRecord {
     Object object,
     this.context,
     this.trackedClass,
-    this.phase,
-  )   : ref = WeakReference(object),
+    this.phase, {
+    this.creationChecker,
+  })  : ref = WeakReference(object),
         type = object.runtimeType,
         code = identityHashCode(object);
 
+  /// Weak reference to the tracked object.
   final WeakReference<Object> ref;
 
   /// [IdentityHashCode] of the object.
@@ -26,6 +29,12 @@ class ObjectRecord {
   /// the object is already GCed and thus there is no access to its code.
   final IdentityHashCode code;
 
+  /// [CreationChecker] that contains knowledge about creation.
+  ///
+  /// Is not used in the record, but can be used
+  /// by owners of this object.
+  final CreationChecker? creationChecker;
+
   Map<String, dynamic>? context;
 
   final PhaseSettings phase;

pkgs/leak_tracker/lib/src/leak_tracking/_object_record_set.dart

@@ -7,6 +7,7 @@ import 'package:meta/meta.dart';
 
 import '../shared/_primitives.dart';
 import '_object_record.dart';
+import 'primitives/_test_helper_detector.dart';
 import 'primitives/model.dart';
 
 @visibleForTesting
@@ -61,7 +62,20 @@ class ObjectRecordSet {
     final existing = list.firstWhereOrNull((r) => r.ref.target == object);
     if (existing != null) return existing;
 
-    final result = ObjectRecord(object, context, trackedClass, phase);
+    final creationChecker = phase.ignoredLeaks.createdByTestHelpers
+        ? CreationChecker(
+            creationStack: StackTrace.current,
+            exceptions: phase.ignoredLeaks.testHelperExceptions)
+        : null;
+
+    final result = ObjectRecord(
+      object,
+      context,
+      trackedClass,
+      phase,
+      creationChecker: creationChecker,
+    );
+
     list.add(result);
     _length++;
     return result;

pkgs/leak_tracker/lib/src/leak_tracking/_object_tracker.dart

@@ -15,7 +15,6 @@ import 'primitives/_finalizer.dart';
 import 'primitives/_gc_counter.dart';
 import 'primitives/_retaining_path/_connection.dart';
 import 'primitives/_retaining_path/_retaining_path.dart';
-import 'primitives/_test_helper_detector.dart';
 import 'primitives/model.dart';
 
 /// Keeps collection of object records until
@@ -63,22 +62,11 @@ class ObjectTracker implements LeakProvider {
     throwIfDisposed();
     if (phase.ignoreLeaks) return;
 
-    StackTrace? stackTrace;
-
-    if (phase.ignoredLeaks.createdByTestHelpers) {
-      stackTrace = StackTrace.current;
-      if (isCreatedByTestHelper(
-        stackTrace.toString(),
-        phase.ignoredLeaks.testHelperExceptions,
-      )) return;
-    }
-
     final record =
         _objects.notGCed.putIfAbsent(object, context, phase, trackedClass);
 
     if (phase.leakDiagnosticConfig.collectStackTraceOnStart) {
-      stackTrace ??= StackTrace.current;
-      record.setContext(ContextKeys.startCallstack, stackTrace);
+      record.setContext(ContextKeys.startCallstack, StackTrace.current);
     }
 
     _finalizer.attach(object, record);

pkgs/leak_tracker/lib/src/leak_tracking/primitives/_test_helper_detector.dart

@@ -2,6 +2,8 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'package:meta/meta.dart';
+
 /// Frames pointing the folder `test` or the package `flutter_test`.
 final _testHelperFrame = RegExp(
   r'(?:' +
@@ -34,6 +36,7 @@ final _startFrame = RegExp(
 ///
 /// See details on what means to be created by a test helper
 /// in doc for `LeakTesting.createdByTestHelpers`.
+@visibleForTesting
 bool isCreatedByTestHelper(
   String objectCreationTrace,
   List<RegExp> exceptions,
@@ -53,3 +56,38 @@ bool isCreatedByTestHelper(
   }
   return false;
 }
+
+/// Postponed detector of test helpers.
+///
+/// It is used to detect if the leak was created by a test helper,
+/// only if the leak is detected.
+///
+/// It is needed because the detection is a heavy operation
+/// and should not be done for every tracked object.
+class CreationChecker {
+  /// Creates instance of [CreationChecker].
+  ///
+  /// Stack frames in [creationStack] that match any of [exceptions]
+  /// will be ignored.
+  CreationChecker(
+      {required StackTrace creationStack, required List<RegExp> exceptions})
+      : _creationStack = creationStack,
+        _exceptions = exceptions;
+  StackTrace? _creationStack;
+  List<RegExp>? _exceptions;
+
+  /// True, if the leak was created by a test helper.
+  ///
+  /// This value is cached. The first calculation of the value
+  /// is performance heavy.
+  late final bool createdByTestHelpers = () {
+    final result = isCreatedByTestHelper(
+      _creationStack!.toString(),
+      _exceptions!,
+    );
+    // Nulling the references to make the object eligible for GC.
+    _creationStack = null;
+    _exceptions = null;
+    return result;
+  }();
+}

pkgs/leak_tracker/pubspec.yaml

@@ -1,5 +1,5 @@
 name: leak_tracker
-version: 10.0.2
+version: 10.0.3
 description: A framework for memory leak tracking for Dart and Flutter applications.
 repository: https://github.com/dart-lang/leak_tracker/tree/main/pkgs/leak_tracker
 

pkgs/leak_tracker/test/tests/leak_tracking/_primitives/_test_helper_detector_test.dart

@@ -6,11 +6,10 @@ import 'package:leak_tracker/src/leak_tracking/primitives/_test_helper_detector.
 import 'package:test/test.dart';
 
 class _Test {
+  _Test({required this.stackTrace, required this.isHelper, required this.name});
   final String name;
   final String stackTrace;
   final bool isHelper;
-
-  _Test({required this.stackTrace, required this.isHelper, required this.name});
 }
 
 final _tests = [
@@ -131,4 +130,22 @@ void main() {
       expect(isCreatedByTestHelper(t.stackTrace, []), t.isHelper);
     });
   }
+
+  group('$CreationChecker', () {
+    test('no test helpers', () {
+      expect(
+          CreationChecker(creationStack: StackTrace.current, exceptions: [])
+              .createdByTestHelpers,
+          false);
+    });
+
+    test('test helper', () {
+      expect(
+          CreationChecker(creationStack: _traceFromTestHelper(), exceptions: [])
+              .createdByTestHelpers,
+          true);
+    });
+  });
 }
+
+StackTrace _traceFromTestHelper() => StackTrace.current;

pkgs/leak_tracker_flutter_testing/CHANGELOG.md

@@ -1,5 +1,6 @@
 ## 3.0.0
 
+* Upgrade leak_tracker to 10.0.3.
 * Upgrade leak_tracker_testing to 3.0.0.
 
 ## 2.0.4

pkgs/leak_tracker_flutter_testing/analysis_options.yaml

@@ -8,3 +8,8 @@ analyzer:
 linter:
   rules:
     - avoid_print
+    - comment_references
+    - only_throw_errors
+    - sort_constructors_first
+    - sort_unnamed_constructors_first
+    - unawaited_futures

pkgs/leak_tracker_flutter_testing/lib/src/testing_for_testing/test_case.dart

@@ -8,13 +8,13 @@ import 'package:leak_tracker/leak_tracker.dart';
 import 'package:leak_tracker_testing/leak_tracker_testing.dart';
 import 'package:matcher/expect.dart';
 
-/// Siugnature of `pumpWidget` method.
+/// Signature of `pumpWidget` method.
 typedef PumpWidgetsCallback = Future<void> Function(
   Widget widget, [
   Duration? duration,
 ]);
 
-/// Siugnature of `runAsync` method.
+/// Signature of `runAsync` method.
 typedef RunAsyncCallback<T> = Future<T?> Function(
   Future<T> Function() callback,
 );
@@ -27,6 +27,15 @@ typedef TestCallback = Future<void> Function(
 
 /// A test case to verify leak detection.
 class LeakTestCase {
+  LeakTestCase({
+    required this.name,
+    required this.body,
+    this.notDisposedTotal = 0,
+    this.notGCedTotal = 0,
+    this.notDisposedInHelpers = 0,
+    this.notGCedInHelpers = 0,
+  });
+
   /// Name of the test.
   final String name;
 
@@ -45,15 +54,6 @@ class LeakTestCase {
   /// Expected number of not GCed objects created by test helpers.
   final int notGCedInHelpers;
 
-  LeakTestCase({
-    required this.name,
-    required this.body,
-    this.notDisposedTotal = 0,
-    this.notGCedTotal = 0,
-    this.notDisposedInHelpers = 0,
-    this.notGCedInHelpers = 0,
-  });
-
   /// Verifies [leaks] contain expected leaks for the test.
   ///
   /// [settings] is used to determine:

pkgs/leak_tracker_flutter_testing/pubspec.yaml

@@ -10,7 +10,7 @@ environment:
 dependencies:
   flutter:
     sdk: flutter
-  leak_tracker: '>=10.0.1 <11.0.0'
+  leak_tracker: '>=10.0.3 <11.0.0'
   leak_tracker_testing: '>=2.0.2 <3.0.0'
   matcher: ^0.12.16
   meta: ^1.8.0

pkgs/leak_tracker_flutter_testing/test/tests/end_to_end/test_helpers_test.dart

@@ -32,12 +32,12 @@ void main() {
   });
 
   test('Test leak is ignored.', () async {
-    createTestWidget();
+    _createTestWidget();
 
     LeakTracking.declareNotDisposedObjectsAsLeaks();
     final leaks = await LeakTracking.collectLeaks();
     expect(leaks.notDisposed.length, 0);
   });
 }
 
-StatelessLeakingWidget createTestWidget() => StatelessLeakingWidget();
+StatelessLeakingWidget _createTestWidget() => StatelessLeakingWidget();

pkgs/leak_tracker_testing/analysis_options.yaml

@@ -10,4 +10,6 @@ linter:
     - avoid_print
     - comment_references
     - only_throw_errors
+    - sort_constructors_first
+    - sort_unnamed_constructors_first
     - unawaited_futures

pkgs/memory_usage/analysis_options.yaml

@@ -5,4 +5,6 @@ linter:
     - avoid_catching_errors
     - comment_references
     - only_throw_errors
+    - sort_constructors_first
+    - sort_unnamed_constructors_first
     - unawaited_futures