Reorganize leak tracker for better performance. (#106)

pkgs/leak_tracker/CHANGELOG.md

@@ -1,7 +1,10 @@
 # 9.0.0
 
+* Refactor to improve performance of regression tests with leak tracking.
+* Remove API that is not used in Flutter Framework.
+* Rename `LeakTrackingConfiguration` to `LeakTrackingConfig`.
 * Remove global flag [collectDebugInformationForLeaks].
-* Rename `checkNonGCed` to `checkNotGCed`.
+* Rename `checkNonGCed` to `checkNotGCed` and `collectRetainingPathForNonGCed` to `collectRetainingPathForNotGCed`.
 * Group global items related to leak tracking, in abstract class LeakTracking.
 * Rename `gcCountBuffer` to `numberOfGcCycles` and `disposalTimeBuffer` to `disposalTime`.
 

pkgs/leak_tracker/lib/leak_tracker.dart

@@ -2,8 +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.
 
+export 'src/leak_tracking/_primitives/model.dart';
 export 'src/leak_tracking/leak_tracking.dart';
-export 'src/leak_tracking/model.dart';
 export 'src/leak_tracking/orchestration.dart';
 export 'src/shared/shared_model.dart';
 export 'src/usage_tracking/model.dart';

pkgs/leak_tracker/lib/src/leak_tracking/DEPENDENCIES.md

@@ -6,21 +6,19 @@ Dependencies that create loop are markes with `!`.
 ```mermaid
 flowchart TD;
 _dispatcher.dart-->_object_tracker.dart;
-_leak_reporter.dart-->model.dart;
+_leak_filter.dart-->_object_record.dart;
+_leak_filter.dart-->_primitives;
+_leak_reporter.dart-->_primitives;
 _leak_tracker.dart-->_leak_reporter.dart;
 _leak_tracker.dart-->_object_tracker.dart;
-_leak_tracker.dart-->model.dart;
-_object_record.dart-->_gc_counter.dart;
-_object_tracker.dart-->_finalizer.dart;
-_object_tracker.dart-->_gc_counter.dart;
+_leak_tracker.dart-->_primitives;
+_object_record.dart-->_primitives;
+_object_tracker.dart-->_leak_filter.dart;
 _object_tracker.dart-->_object_record.dart;
-_object_tracker.dart-->_retaining_path;
-_object_tracker.dart-->model.dart;
+_object_tracker.dart-->_primitives;
 leak_tracking.dart-->_dispatcher.dart;
 leak_tracking.dart-->_leak_tracker.dart;
-leak_tracking.dart-->model.dart;
-orchestration.dart-->_retaining_path;
-orchestration.dart-->leak_tracking.dart;
-orchestration.dart-->model.dart;
+leak_tracking.dart-->_primitives;
+orchestration.dart-->_primitives;
 ```
 

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

@@ -0,0 +1,71 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// 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.s
+
+import '../shared/shared_model.dart';
+import '_object_record.dart';
+import '_primitives/model.dart';
+
+/// Decides which leaks to report based on allow lists of the phase.
+class LeakFilter {
+  final Map<PhaseSettings, _PhaseLeakFilter> _phases = {};
+
+  bool shouldReport(LeakType leakType, ObjectRecord record) {
+    final filter = _phases.putIfAbsent(
+      record.phase,
+      () => _PhaseLeakFilter(record.phase),
+    );
+    return filter.shouldReport(leakType, record);
+  }
+}
+
+class _PhaseLeakFilter {
+  _PhaseLeakFilter(this.phase);
+
+  /// Number of leaks by (object type, leak type) for limited allowlists.
+  final _count = <(String, LeakType), int>{};
+
+  final PhaseSettings phase;
+
+  bool shouldReport(LeakType leakType, ObjectRecord record) {
+    switch (leakType) {
+      case LeakType.notDisposed:
+        return _shouldReport(
+          leakType,
+          record,
+          phase.allowAllNotDisposed,
+          phase.notDisposedAllowList,
+        );
+      case LeakType.notGCed:
+      case LeakType.gcedLate:
+        return _shouldReport(
+          leakType,
+          record,
+          phase.allowAllNotGCed,
+          phase.notGCedAllowList,
+        );
+    }
+  }
+
+  bool _shouldReport(
+    LeakType leakType,
+    ObjectRecord record,
+    bool allAllowed,
+    Map<String, int?> allowList,
+  ) {
+    assert(record.phase == phase);
+    if (allAllowed) return false;
+    final objectType = record.type.toString();
+    if (!allowList.containsKey(objectType)) return true;
+    final allowedCount = allowList[objectType];
+    if (allowedCount == null) return false;
+
+    final actualCount = _count.update(
+      (objectType, leakType),
+      (value) => value + 1,
+      ifAbsent: () => 1,
+    );
+
+    return actualCount > allowedCount;
+  }
+}

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

@@ -7,7 +7,7 @@ import 'dart:async';
 import '../devtools_integration/delivery.dart';
 import '../shared/_util.dart';
 import '../shared/shared_model.dart';
-import 'model.dart';
+import '_primitives/model.dart';
 
 /// Checks [leakProvider] either by schedule or by request.
 ///

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

@@ -2,17 +2,18 @@
 // 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 '../shared/_primitives.dart';
 import '_leak_reporter.dart';
 import '_object_tracker.dart';
-import 'model.dart';
+import '_primitives/model.dart';
 
 class LeakTracker {
-  LeakTracker(LeakTrackingConfiguration config) {
+  LeakTracker(LeakTrackingConfig config, ObjectRef<PhaseSettings> phase) {
     objectTracker = ObjectTracker(
-      leakDiagnosticConfig: config.leakDiagnosticConfig,
       disposalTime: config.disposalTime,
       numberOfGcCycles: config.numberOfGcCycles,
       maxRequestsForRetainingPath: config.maxRequestsForRetainingPath,
+      phase: phase,
     );
 
     leakReporter = LeakReporter(

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

@@ -4,7 +4,8 @@
 
 import '../shared/_primitives.dart';
 import '../shared/shared_model.dart';
-import '_gc_counter.dart';
+import '_primitives/_gc_counter.dart';
+import '_primitives/model.dart';
 
 /// Object collections to track leaks.
 ///
@@ -83,11 +84,20 @@ class ObjectRecords {
 
 /// Information about an object, tracked for leaks.
 class ObjectRecord {
-  ObjectRecord(this.code, this.context, this.type, this.trackedClass);
+  ObjectRecord(
+    this.code,
+    this.context,
+    this.type,
+    this.trackedClass,
+    this.phase,
+  );
 
   final IdentityHashCode code;
+
   Map<String, dynamic>? context;
 
+  final PhaseSettings phase;
+
   /// Type of the tracked object.
   final Type type;
 
@@ -171,5 +181,6 @@ class ObjectRecord {
         context: context,
         code: code,
         trackedClass: trackedClass,
+        phase: phase.name,
       );
 }

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

@@ -9,12 +9,13 @@ import 'package:meta/meta.dart';
 
 import '../shared/_primitives.dart';
 import '../shared/shared_model.dart';
-import '_finalizer.dart';
-import '_gc_counter.dart';
+import '_leak_filter.dart';
 import '_object_record.dart';
-import '_retaining_path/_connection.dart';
-import '_retaining_path/_retaining_path.dart';
-import 'model.dart';
+import '_primitives/_finalizer.dart';
+import '_primitives/_gc_counter.dart';
+import '_primitives/_retaining_path/_connection.dart';
+import '_primitives/_retaining_path/_retaining_path.dart';
+import '_primitives/model.dart';
 
 /// Keeps collection of object records until
 /// disposal and garbage gollection.
@@ -24,10 +25,10 @@ import 'model.dart';
 class ObjectTracker implements LeakProvider {
   /// The optional parameters are injected for testing purposes.
   ObjectTracker({
-    this.leakDiagnosticConfig = const LeakDiagnosticConfig(),
     required this.disposalTime,
     required this.numberOfGcCycles,
     required this.maxRequestsForRetainingPath,
+    required this.phase,
     FinalizerBuilder? finalizerBuilder,
     GcCounter? gcCounter,
     IdentityHashCoder? coder,
@@ -49,20 +50,24 @@ class ObjectTracker implements LeakProvider {
 
   final _objects = ObjectRecords();
 
-  bool disposed = false;
+  final _leakFilter = LeakFilter();
 
-  final LeakDiagnosticConfig leakDiagnosticConfig;
+  bool disposed = false;
 
   final int numberOfGcCycles;
 
   final int? maxRequestsForRetainingPath;
 
+  final ObjectRef<PhaseSettings> phase;
+
   void startTracking(
     Object object, {
     required Map<String, dynamic>? context,
     required String trackedClass,
   }) {
     throwIfDisposed();
+    if (phase.value.isPaused) return;
+
     final code = _coder(object);
     assert(code > 0);
     if (_checkForDuplicate(code)) return;
@@ -74,9 +79,10 @@ class ObjectTracker implements LeakProvider {
       context,
       object.runtimeType,
       trackedClass,
+      phase.value,
     );
 
-    if (leakDiagnosticConfig
+    if (phase.value.leakDiagnosticConfig
         .shouldCollectStackTraceOnStart(object.runtimeType.toString())) {
       record.setContext(ContextKeys.startCallstack, StackTrace.current);
     }
@@ -139,7 +145,7 @@ class ObjectTracker implements LeakProvider {
     final record = _notGCed(code);
     record.mergeContext(context);
 
-    if (leakDiagnosticConfig
+    if (phase.value.leakDiagnosticConfig
         .shouldCollectStackTraceOnDisposal(object.runtimeType.toString())) {
       record.setContext(ContextKeys.disposalCallstack, StackTrace.current);
     }
@@ -177,7 +183,9 @@ class ObjectTracker implements LeakProvider {
     _objects.assertIntegrity();
 
     final List<int>? objectsToGetPath =
-        leakDiagnosticConfig.collectRetainingPathForNonGCed ? [] : null;
+        phase.value.leakDiagnosticConfig.collectRetainingPathForNotGCed
+            ? []
+            : null;
 
     final now = clock.now();
     for (int code in _objects.notGCedDisposedOk.toList(growable: false)) {
@@ -262,12 +270,22 @@ class ObjectTracker implements LeakProvider {
 
     final result = Leaks({
       LeakType.notDisposed: _objects.gcedNotDisposedLeaks
+          .where(
+            (record) => _leakFilter.shouldReport(LeakType.notDisposed, record),
+          )
           .map((record) => record.toLeakReport())
           .toList(),
       LeakType.notGCed: _objects.notGCedDisposedLate
+          .where(
+            (code) => _leakFilter.shouldReport(
+              LeakType.notGCed,
+              _notGCed(code),
+            ),
+          )
           .map((code) => _notGCed(code).toLeakReport())
           .toList(),
       LeakType.gcedLate: _objects.gcedLateLeaks
+          .where((record) => _leakFilter.shouldReport(LeakType.notGCed, record))
           .map((record) => record.toLeakReport())
           .toList(),
     });

pkgs/leak_tracker/lib/src/leak_tracking/_primitives/README.md

@@ -0,0 +1,2 @@
+Items (folders or libraries) in this folder do not depend on each other
+and on other libraries in `leak_tracking`.