Combine shallow and retained size columns into one to save horizontal space. (#5299)
diff --git a/packages/devtools_app/lib/src/screens/memory/panes/diff/controller/diff_pane_controller.dart b/packages/devtools_app/lib/src/screens/memory/panes/diff/controller/diff_pane_controller.dart index 71cc19c..1593b2e 100644 --- a/packages/devtools_app/lib/src/screens/memory/panes/diff/controller/diff_pane_controller.dart +++ b/packages/devtools_app/lib/src/screens/memory/panes/diff/controller/diff_pane_controller.dart
@@ -19,6 +19,7 @@ import '../../../shared/heap/heap.dart'; import '../../../shared/heap/model.dart'; import '../../../shared/primitives/memory_utils.dart'; +import '../../../shared/primitives/simple_elements.dart'; import 'heap_diff.dart'; import 'item_controller.dart'; import 'simple_controllers.dart'; @@ -35,6 +36,8 @@ final retainingPathController = RetainingPathController(); + final sizeTypeToShowForDiff = ValueNotifier<SizeType>(SizeType.retained); + final core = CoreData(); late final derived = DerivedData(core);
diff --git a/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/class_details/paths.dart b/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/class_details/paths.dart index 06c7fc7..76401ca 100644 --- a/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/class_details/paths.dart +++ b/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/class_details/paths.dart
@@ -54,7 +54,7 @@ _ShallowSizeColumn(bool isDiff) : super( isDiff ? 'Shallow\nSize Delta' : 'Shallow\nDart Size', - titleTooltip: shallowSizeColumnTooltip, + titleTooltip: SizeType.shallow.description, fixedWidthPx: scaleByFontFactor(85.0), alignment: ColumnAlignment.right, ); @@ -77,7 +77,7 @@ _RetainedSizeColumn(bool isDiff) : super( isDiff ? 'Retained\nSize Delta' : 'Retained\nDart Size', - titleTooltip: retainedSizeColumnTooltip, + titleTooltip: SizeType.retained.description, fixedWidthPx: scaleByFontFactor(85.0), alignment: ColumnAlignment.right, );
diff --git a/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_diff.dart b/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_diff.dart index b0a1fdb..8d308f7 100644 --- a/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_diff.dart +++ b/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_diff.dart
@@ -6,6 +6,7 @@ import '../../../../../shared/analytics/analytics.dart' as ga; import '../../../../../shared/analytics/constants.dart' as gac; +import '../../../../../shared/common_widgets.dart'; import '../../../../../shared/feature_flags.dart'; import '../../../../../shared/globals.dart'; import '../../../../../shared/memory/adapted_heap_data.dart'; @@ -27,11 +28,6 @@ persisted, } -enum _SizeType { - shallow, - retained, -} - class _ClassNameColumn extends ColumnData<DiffClassStats> implements ColumnRenderer<DiffClassStats>, @@ -185,7 +181,7 @@ ); final _DataPart dataPart; - final _SizeType sizeType; + final SizeType sizeType; static String columnTitle(_DataPart dataPart) { switch (dataPart) { @@ -203,7 +199,7 @@ @override int getValue(DiffClassStats classStats) { switch (sizeType) { - case _SizeType.shallow: + case SizeType.shallow: switch (dataPart) { case _DataPart.created: return classStats.total.created.shallowSize; @@ -214,7 +210,7 @@ case _DataPart.persisted: return classStats.total.persisted.shallowSize; } - case _SizeType.retained: + case SizeType.retained: switch (dataPart) { case _DataPart.created: return classStats.total.created.retainedSize; @@ -243,19 +239,21 @@ class _ClassesTableDiffColumns { _ClassesTableDiffColumns( - this.classFilterButton, { + this.classFilterButton, + this.sizeTypeToShow, { required this.before, required this.after, }); final Widget classFilterButton; - final retainedSizeDeltaColumn = - _SizeColumn(_DataPart.delta, _SizeType.retained); + late final sizeDeltaColumn = _SizeColumn(_DataPart.delta, sizeTypeToShow); final AdaptedHeapData before; final AdaptedHeapData after; + final SizeType sizeTypeToShow; + late final List<ColumnData<DiffClassStats>> columnList = <ColumnData<DiffClassStats>>[ _ClassNameColumn(classFilterButton), @@ -263,17 +261,51 @@ _InstanceColumn(_DataPart.deleted, before), _InstanceColumn(_DataPart.delta, null), _InstanceColumn(_DataPart.persisted, after), - _SizeColumn(_DataPart.created, _SizeType.shallow), - _SizeColumn(_DataPart.deleted, _SizeType.shallow), - _SizeColumn(_DataPart.delta, _SizeType.shallow), - _SizeColumn(_DataPart.persisted, _SizeType.shallow), - _SizeColumn(_DataPart.created, _SizeType.retained), - _SizeColumn(_DataPart.deleted, _SizeType.retained), - retainedSizeDeltaColumn, - _SizeColumn(_DataPart.persisted, _SizeType.retained), + _SizeColumn(_DataPart.created, sizeTypeToShow), + _SizeColumn(_DataPart.deleted, sizeTypeToShow), + sizeDeltaColumn, + _SizeColumn(_DataPart.persisted, sizeTypeToShow), ]; } +class _SizeTitle extends StatelessWidget { + const _SizeTitle({required this.sizeTypeToShowForDiff}); + final ValueNotifier<SizeType> sizeTypeToShowForDiff; + + @override + Widget build(BuildContext context) { + final sizeType = sizeTypeToShowForDiff.value; + + return maybeWrapWithTooltip( + child: Padding( + padding: const EdgeInsets.all(densePadding), + child: Row( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + RoundedDropDownButton<SizeType>( + isDense: true, + value: sizeType, + onChanged: (SizeType? value) => + sizeTypeToShowForDiff.value = value!, + items: SizeType.values + .map( + (sizeType) => DropdownMenuItem<SizeType>( + value: sizeType, + child: Text(sizeType.displayName), + ), + ) + .toList(), + ), + const SizedBox(width: denseSpacing), + const Text('Size'), + ], + ), + ), + tooltip: '${sizeType.displayName} size:\n${sizeType.description}', + ); + } +} + class ClassesTableDiff extends StatelessWidget { const ClassesTableDiff({ Key? key, @@ -282,60 +314,65 @@ required this.classFilterButton, required this.before, required this.after, + required this.sizeTypeToShowForDiff, }) : super(key: key); final List<DiffClassStats> classes; final ValueNotifier<DiffClassStats?> selection; final AdaptedHeapData before; final AdaptedHeapData after; + final ValueNotifier<SizeType> sizeTypeToShowForDiff; - static final _columnGroups = [ - ColumnGroup.fromText( - title: '', - range: const Range(0, 1), - ), - ColumnGroup.fromText( - title: 'Instances', - range: const Range(1, 5), - tooltip: nonGcableInstancesColumnTooltip, - ), - ColumnGroup.fromText( - title: 'Shallow Dart Size', - range: const Range(5, 9), - tooltip: shallowSizeColumnTooltip, - ), - ColumnGroup.fromText( - title: 'Retained Dart Size', - range: const Range(9, 13), - tooltip: retainedSizeColumnTooltip, - ), - ]; + List<ColumnGroup> _columnGroups(SizeType sizeType, BuildContext context) { + return [ + ColumnGroup.fromText( + title: '', + range: const Range(0, 1), + ), + ColumnGroup.fromText( + title: 'Instances', + range: const Range(1, 5), + tooltip: nonGcableInstancesColumnTooltip, + ), + ColumnGroup( + title: _SizeTitle(sizeTypeToShowForDiff: sizeTypeToShowForDiff), + range: const Range(5, 9), + ), + ]; + } final Widget classFilterButton; @override Widget build(BuildContext context) { - // We want to preserve the sorting and sort directions for ClassesTableDiff - // no matter what the data passed to it is. - const dataKey = 'ClassesTableDiff'; - final columns = _ClassesTableDiffColumns( - classFilterButton, - before: before, - after: after, - ); - return FlatTable<DiffClassStats>( - columns: columns.columnList, - columnGroups: _columnGroups, - data: classes, - dataKey: dataKey, - keyFactory: (e) => Key(e.heapClass.fullName), - selectionNotifier: selection, - onItemSelected: (_) => ga.select( - gac.memory, - gac.MemoryEvent.diffClassDiffSelect, - ), - defaultSortColumn: columns.retainedSizeDeltaColumn, - defaultSortDirection: SortDirection.descending, + return ValueListenableBuilder<SizeType>( + valueListenable: sizeTypeToShowForDiff, + builder: (context, sizeType, _) { + // We want to preserve the sorting and sort directions for ClassesTableDiff + // no matter what the data passed to it is. + const dataKey = 'ClassesTableDiff'; + final columns = _ClassesTableDiffColumns( + classFilterButton, + sizeType, + before: before, + after: after, + ); + + return FlatTable<DiffClassStats>( + columns: columns.columnList, + columnGroups: _columnGroups(sizeType, context), + data: classes, + dataKey: dataKey, + keyFactory: (e) => Key(e.heapClass.fullName), + selectionNotifier: selection, + onItemSelected: (_) => ga.select( + gac.memory, + gac.MemoryEvent.diffClassDiffSelect, + ), + defaultSortColumn: columns.sizeDeltaColumn, + defaultSortDirection: SortDirection.descending, + ); + }, ); } }
diff --git a/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_single.dart b/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_single.dart index c4c03dc..36ee4e5 100644 --- a/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_single.dart +++ b/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/classes_table_single.dart
@@ -118,7 +118,7 @@ _ShallowSizeColumn() : super( 'Shallow\nDart Size', - titleTooltip: shallowSizeColumnTooltip, + titleTooltip: SizeType.shallow.description, fixedWidthPx: scaleByFontFactor(85.0), alignment: ColumnAlignment.right, ); @@ -141,7 +141,7 @@ _RetainedSizeColumn(this.totalSize) : super( 'Retained Dart Size', - titleTooltip: retainedSizeColumnTooltip, + titleTooltip: SizeType.retained.description, fixedWidthPx: scaleByFontFactor(140.0), alignment: ColumnAlignment.right, );
diff --git a/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/snapshot_view.dart b/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/snapshot_view.dart index fa4fd95..3d2ce08 100644 --- a/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/snapshot_view.dart +++ b/packages/devtools_app/lib/src/screens/memory/panes/diff/widgets/snapshot_view.dart
@@ -73,6 +73,7 @@ classFilterButton: classFilterButton, before: diffHeapClasses.before, after: diffHeapClasses.after, + sizeTypeToShowForDiff: controller.sizeTypeToShowForDiff, ); } else { throw StateError('singleClasses or diffClasses should not be null.');
diff --git a/packages/devtools_app/lib/src/screens/memory/panes/profile/profile_view.dart b/packages/devtools_app/lib/src/screens/memory/panes/profile/profile_view.dart index 481cc45..027973f 100644 --- a/packages/devtools_app/lib/src/screens/memory/panes/profile/profile_view.dart +++ b/packages/devtools_app/lib/src/screens/memory/panes/profile/profile_view.dart
@@ -139,7 +139,7 @@ _FieldDartHeapSizeColumn({required super.heap}) : super._( title: 'Dart Heap', - titleTooltip: shallowSizeColumnTooltip, + titleTooltip: SizeType.shallow.description, ); @override
diff --git a/packages/devtools_app/lib/src/screens/memory/shared/primitives/simple_elements.dart b/packages/devtools_app/lib/src/screens/memory/shared/primitives/simple_elements.dart index 51e1344..8f74649 100644 --- a/packages/devtools_app/lib/src/screens/memory/shared/primitives/simple_elements.dart +++ b/packages/devtools_app/lib/src/screens/memory/shared/primitives/simple_elements.dart
@@ -2,17 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -const String shallowSizeColumnTooltip = - 'The total shallow size of all of the instances.\n' - 'The shallow size of an object is the size of the object\n' - 'plus the references it holds to other Dart objects\n' - "in its fields (this doesn't include the size of\n" - 'the fields - just the size of the references).'; - -const String retainedSizeColumnTooltip = - 'Total shallow Dart size of objects plus shallow Dart size of objects they retain,\n' - 'taking into account only the shortest retaining path for the referenced objects.'; - const String nonGcableInstancesColumnTooltip = 'Number of instances of the class,\n' 'that are reachable, i.e. have a retaining path from the root\n' @@ -37,3 +26,26 @@ final String hash; String get value => '$url#$hash'; } + +enum SizeType { + shallow( + displayName: 'Shallow', + description: 'The total shallow size of all of the instances.\n' + 'The shallow size of an object is the size of the object\n' + 'plus the references it holds to other Dart objects\n' + "in its fields (this doesn't include the size of\n" + 'the fields - just the size of the references).', + ), + retained( + displayName: 'Retained', + description: + 'Total shallow Dart size of objects plus shallow Dart size of objects they retain,\n' + 'taking into account only the shortest retaining path for the referenced objects.', + ), + ; + + const SizeType({required this.displayName, required this.description}); + + final String displayName; + final String description; +}
diff --git a/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_custom_diff.png b/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_custom_diff.png index 97ee8af..91870c7 100644 --- a/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_custom_diff.png +++ b/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_custom_diff.png Binary files differ
diff --git a/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_except_diff.png b/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_except_diff.png index 7959f26..928e0c0 100644 --- a/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_except_diff.png +++ b/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_except_diff.png Binary files differ
diff --git a/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_only_diff.png b/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_only_diff.png index 97ee8af..91870c7 100644 --- a/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_only_diff.png +++ b/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_only_diff.png Binary files differ
diff --git a/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_scene_diff.png b/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_scene_diff.png index 3f8dcb0..f7e660a 100644 --- a/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_scene_diff.png +++ b/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_scene_diff.png Binary files differ
diff --git a/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_showAll_diff.png b/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_showAll_diff.png index 7959f26..928e0c0 100644 --- a/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_showAll_diff.png +++ b/packages/devtools_app/test/test_infra/goldens/memory_diff_snapshot_showAll_diff.png Binary files differ
