Fix CPU profiler tab time estimates (#8941)

Fixes #8870.

This fixes all views on the cpu profiler tab to take into account time where no code was executing (no samples taken).

The approach taken is to use the sampleCount * medianObservedSamplePeriod to get the total duration, which is then used in all the other calculations, instead of the total duration of the profile. This isn't necessarily perfect but should be much better than what we had before, and I don't know that we can do any better.

We use the median observed sample period because it is more accurate and reliable than the reported sample period.

For the reproduction case in the issue you now get this:

<img width="1247" alt="Screenshot 2025-02-26 at 2 21 13 PM" src="https://github.com/user-attachments/assets/aca32479-b6d0-4298-9bdb-7f25b4990a22" />

Instead of this:

<img width="1273" alt="Screenshot 2025-02-24 at 11 36 12 AM" src="https://github.com/user-attachments/assets/b106296c-07c4-4542-b85d-02a8105dba72" />

Author: jakemac53

packages/devtools_app/lib/src/screens/profiler/cpu_profile_model.dart

@@ -5,6 +5,7 @@
 import 'package:devtools_shared/devtools_shared.dart';
 import 'package:flutter/foundation.dart';
 import 'package:logging/logging.dart';
+import 'package:meta/meta.dart';
 import 'package:vm_service/vm_service.dart' as vm_service;
 
 import '../../shared/charts/flame_chart.dart';
@@ -170,9 +171,24 @@ class CpuProfileData with Serializable {
 
   factory CpuProfileData.fromJson(Map<String, Object?> json_) {
     final json = _CpuProfileDataJson(json_);
+
+    // All CPU samples.
+    final samples = json.traceEvents ?? <CpuSampleEvent>[];
+
+    // Sort the samples so we can compute the observed time difference between
+    // each sample.
+    samples.sort((a, b) => a.timestampMicros!.compareTo(b.timestampMicros!));
+
+    // Prefer the approximate observed median time between samples over the
+    // reported sample period.
+    //
+    // See https://github.com/flutter/devtools/pull/8941 for more information.
+    final samplePeriod =
+        observedSamplePeriod(samples) ?? json.samplePeriod ?? 0;
+
     final profileMetaData = CpuProfileMetaData(
       sampleCount: json.sampleCount ?? 0,
-      samplePeriod: json.samplePeriod ?? 0,
+      samplePeriod: samplePeriod,
       stackDepth: json.stackDepth ?? 0,
       time:
           (json.timeOriginMicros != null && json.timeExtentMicros != null)
@@ -211,9 +227,6 @@ class CpuProfileData with Serializable {
       stackFrames[stackFrame.id] = stackFrame;
     }
 
-    // Initialize all CPU samples.
-    final samples = json.traceEvents ?? [];
-
     return CpuProfileData._(
       stackFrames: stackFrames,
       cpuSamples: samples,
@@ -379,9 +392,7 @@ class CpuProfileData with Serializable {
             .toList();
     assert(samplesWithTag.isNotEmpty);
 
-    final originalTime = originalData.profileMetaData.time!.duration;
-    final microsPerSample =
-        originalTime.inMicroseconds / originalData.profileMetaData.sampleCount;
+    final microsPerSample = originalData.profileMetaData.samplePeriod;
     final newSampleCount = samplesWithTag.length;
     final metaData = originalData.profileMetaData.copyWith(
       sampleCount: newSampleCount,
@@ -486,7 +497,7 @@ class CpuProfileData with Serializable {
       return null;
     }
 
-    final originalTime = originalData.profileMetaData.time!.duration;
+    final originalTime = originalData.profileMetaData.measuredDuration;
     final microsPerSample =
         originalTime.inMicroseconds / originalData.profileMetaData.sampleCount;
     final updatedMetaData = originalData.profileMetaData.copyWith(
@@ -791,13 +802,11 @@ extension type _CpuProfileDataJson(Map<String, Object?> json) {
 class CpuProfileMetaData extends ProfileMetaData {
   CpuProfileMetaData({
     required super.sampleCount,
-    required this.samplePeriod,
+    required super.samplePeriod,
     required this.stackDepth,
     required super.time,
   });
 
-  final int samplePeriod;
-
   final int stackDepth;
 
   CpuProfileMetaData copyWith({
@@ -1386,3 +1395,57 @@ extension on vm_service.CpuSamples {
     return traceObject;
   }
 }
+
+/// Efficiently approximates the observed sample period of [samples], by
+/// calculating the approximate median time difference between each sample.
+///
+/// The [samples] must be sorted by timestampMicros before calling this
+/// function.
+///
+/// If there are fewer than 100 samples, returns `null`, because there isn't
+/// enough data to be confident about the observed sample period.
+///
+/// This does not return the exact median, but instead the median of medians
+/// of groups of 5 elements, which makes the algorithm much more efficient.
+@visibleForTesting
+int? observedSamplePeriod(List<CpuSampleEvent> samples) {
+  if (samples.length < 100) return null;
+
+  // To compute the median efficiently, we compute the median of groups of 5
+  // elements, and then grab the median of those medians by sorting, which
+  // brings us a linear time complexity while retaining high accuracy.
+  final mediansOfGroupsOf5 = <int>[];
+  for (var i = 1; i + 5 < samples.length; i += 5) {
+    // The time diff between the sample at index and the previous sample.
+    int diff(int index) {
+      final result =
+          samples[index].timestampMicros! - samples[index - 1].timestampMicros!;
+      assert(result >= 0);
+      return result;
+    }
+
+    mediansOfGroupsOf5.add(
+      _median5(diff(i), diff(i + 1), diff(i + 2), diff(i + 3), diff(i + 4)),
+    );
+  }
+  mediansOfGroupsOf5.sort();
+  return mediansOfGroupsOf5[(mediansOfGroupsOf5.length / 2).floor()];
+}
+
+/// Computes the median of 5 numbers without allocating a list
+/// or actually sorting all the numbers.
+int _median5(int a, int b, int c, int d, int e) {
+  while (true) {
+    if (c < a) {
+      (a, c) = (c, a);
+    } else if (c < b) {
+      (b, c) = (c, b);
+    } else if (c > d) {
+      (c, d) = (d, c);
+    } else if (c > e) {
+      (c, e) = (e, c);
+    } else {
+      return c;
+    }
+  }
+}

packages/devtools_app/lib/src/screens/profiler/panes/method_table/method_table_model.dart

@@ -68,15 +68,16 @@ class MethodTableGraphNode extends GraphNode with SearchableDataMixin {
 
   Duration get selfTime => Duration(
     microseconds:
-        (selfTimeRatio * profileMetaData.time!.duration.inMicroseconds).round(),
+        (selfTimeRatio * profileMetaData.measuredDuration.inMicroseconds)
+            .round(),
   );
 
   double get totalTimeRatio =>
       safeDivide(totalCount, profileMetaData.sampleCount);
 
   Duration get totalTime => Duration(
     microseconds:
-        (totalTimeRatio * profileMetaData.time!.duration.inMicroseconds)
+        (totalTimeRatio * profileMetaData.measuredDuration.inMicroseconds)
             .round(),
   );
 

packages/devtools_app/lib/src/shared/config_specific/framework_initialize/_framework_initialize_web.dart

@@ -98,11 +98,11 @@ void _sendKeyPressToParent(KeyboardEvent event) {
 class BrowserStorage implements Storage {
   @override
   Future<String?> getValue(String key) async {
-    return window.localStorage[key];
+    return window.localStorage.getItem(key);
   }
 
   @override
   Future<void> setValue(String key, String value) async {
-    window.localStorage[key] = value;
+    window.localStorage.setItem(key, value);
   }
 }

packages/devtools_app/lib/src/shared/utils/profiler_utils.dart

@@ -42,7 +42,7 @@ mixin ProfilableDataMixin<T extends TreeNode<T>> on TreeNode<T> {
 
   late Duration totalTime = Duration(
     microseconds:
-        (totalTimeRatio * profileMetaData.time!.duration.inMicroseconds)
+        (totalTimeRatio * profileMetaData.measuredDuration.inMicroseconds)
             .round(),
   );
 
@@ -53,7 +53,8 @@ mixin ProfilableDataMixin<T extends TreeNode<T>> on TreeNode<T> {
 
   late Duration selfTime = Duration(
     microseconds:
-        (selfTimeRatio * profileMetaData.time!.duration.inMicroseconds).round(),
+        (selfTimeRatio * profileMetaData.measuredDuration.inMicroseconds)
+            .round(),
   );
 
   double get inclusiveSampleRatio =>
@@ -97,11 +98,31 @@ mixin ProfilableDataMixin<T extends TreeNode<T>> on TreeNode<T> {
 }
 
 class ProfileMetaData {
-  const ProfileMetaData({required this.sampleCount, required this.time});
+  const ProfileMetaData({
+    required this.sampleCount,
+    required this.samplePeriod,
+    required this.time,
+  });
 
+  /// The total number of samples in this profile.
   final int sampleCount;
 
+  /// The sample period for this profile in microseconds.
+  final int samplePeriod;
+
+  /// The time range of the entire profile.
+  ///
+  /// Note that there may be periods of time with no samples, so the duration
+  /// of this time should not be used in any calculations for how long a given
+  /// sample took, instead use [measuredDuration].
   final TimeRange? time;
+
+  /// The amount of time measured by all the samples taken in this profile.
+  ///
+  /// This is different from [time] which is just the start to end time of the
+  /// entire profile which includes time where no samples were taken.
+  Duration get measuredDuration =>
+      Duration(microseconds: sampleCount * samplePeriod);
 }
 
 /// Process for converting a [ProfilableDataMixin] into a bottom-up

packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md

@@ -40,6 +40,8 @@ TODO: Remove this section if there are not any general updates.
   * [#8892](https://github.com/flutter/devtools/pull/8892)
   * [#8878](https://github.com/flutter/devtools/pull/8878)
   * [#8839](https://github.com/flutter/devtools/pull/8839)
+* Fixed incorrect duration calculations when there is time during which no
+  samples were taken - [#8941](https://github.com/flutter/devtools/pull/8941).
 
 ## Memory updates
 

packages/devtools_app/test/screens/cpu_profiler/cpu_profile_model_test.dart

@@ -4,6 +4,7 @@
 
 import 'package:devtools_app/src/screens/profiler/cpu_profile_model.dart';
 import 'package:devtools_app/src/service/service_manager.dart';
+import 'package:devtools_app/src/shared/primitives/trace_event.dart';
 import 'package:devtools_app/src/shared/primitives/utils.dart';
 import 'package:devtools_app_shared/utils.dart';
 import 'package:devtools_test/devtools_test.dart';
@@ -438,4 +439,53 @@ void main() {
     expect(stackFrameC.matches(stackFrameG), isFalse);
     expect(stackFrameC.matches(stackFrameC4), isFalse);
   });
+
+  group('observedSamplePeriod', () {
+    CpuSampleEvent sampleEventWithTimestamp(int timestampMicros) {
+      return CpuSampleEvent.fromJson({
+        CpuProfileData.stackFrameIdKey: '0',
+        ChromeTraceEvent.timestampKey: timestampMicros,
+      });
+    }
+
+    test('returns null if less than 100 samples', () {
+      expect(observedSamplePeriod([]), isNull);
+      expect(
+        observedSamplePeriod(List.filled(99, sampleEventWithTimestamp(1))),
+        isNull,
+      );
+      expect(
+        observedSamplePeriod(
+          List.generate(100, (i) => sampleEventWithTimestamp(i)),
+        ),
+        1,
+      );
+    });
+
+    test('asserts that samples are sorted', () {
+      expect(
+        () => observedSamplePeriod([
+          sampleEventWithTimestamp(10),
+          sampleEventWithTimestamp(5),
+          // Need at least 100 elements for anything to happen.
+          ...List.filled(100, sampleEventWithTimestamp(1)),
+        ]),
+        throwsA(isA<AssertionError>()),
+      );
+    });
+
+    test('computes an approximate time difference median', () {
+      final samples = List.generate(
+        100,
+        (i) => sampleEventWithTimestamp(i * i),
+      );
+      // Better to hard code this than rely on computing the median correctly.
+      const actualMedianSamplePeriod = 50 * 50 - 49 * 49;
+      expect(
+        observedSamplePeriod(samples),
+        // Ensure we are within 5%, this is not a perfect median.
+        closeTo(actualMedianSamplePeriod, actualMedianSamplePeriod * 0.05),
+      );
+    });
+  });
 }

packages/devtools_app/test/screens/cpu_profiler/method_table/method_table_model_test.dart

@@ -17,6 +17,7 @@ void main() {
       selfCount: 1,
       profileMetaData: ProfileMetaData(
         sampleCount: 10,
+        samplePeriod: 250,
         time:
             TimeRange()
               ..start = Duration.zero
@@ -32,6 +33,7 @@ void main() {
       selfCount: 4,
       profileMetaData: ProfileMetaData(
         sampleCount: 10,
+        samplePeriod: 250,
         time:
             TimeRange()
               ..start = Duration.zero
@@ -47,6 +49,7 @@ void main() {
       selfCount: 2,
       profileMetaData: ProfileMetaData(
         sampleCount: 10,
+        samplePeriod: 250,
         time:
             TimeRange()
               ..start = Duration.zero
@@ -62,6 +65,7 @@ void main() {
       selfCount: 3,
       profileMetaData: ProfileMetaData(
         sampleCount: 10,
+        samplePeriod: 250,
         time:
             TimeRange()
               ..start = Duration.zero

packages/devtools_app/test/test_infra/test_data/cpu_profiler/cpu_profile.dart

@@ -1211,7 +1211,9 @@ final profileMetaData = CpuProfileMetaData(
   time:
       TimeRange()
         ..start = const Duration()
-        ..end = const Duration(microseconds: 10000),
+        // Note this intentionally adds 10000 microseconds more than what
+        // was measured, regression test for Issue #8870.
+        ..end = const Duration(microseconds: 20000),
 );
 
 final tagFrameA = CpuStackFrame(