-
Notifications
You must be signed in to change notification settings - Fork 346
Diff heap statistics. #4501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Diff heap statistics. #4501
Changes from 7 commits
88e3c0d
40f96b4
4f35c1b
87794ef
070f797
54a36a1
bd4949c
98f58ef
19a7c37
86700a3
ef2d558
9991886
fd89707
4e3f5a0
472625b
1931fea
0bab199
e51cd2a
78d65f6
9e2e970
93900af
5afdbfb
da0c005
e57318c
f1d9623
4115819
ce85cdd
d489a31
7f7bae1
3df81f8
e470e4c
7a85608
13d2756
fbc3a6e
64b20f1
362dd29
34d6f11
a00e502
07eb2fe
41a7553
380be7f
45dc0e1
002d7d5
6238161
15fe292
37230b1
5f1186b
f028530
91ca15d
688ba9b
dce7eb7
b83af3c
5966ba6
97ba965
5478c00
25fb537
e50bf91
1ee043a
65dd972
087043b
94cbbf8
1ea913d
dc4e915
4fea4a7
57d8f5f
9e7ff13
0f6ce0f
8671ab4
5d14a5a
95b5878
e6412a5
9ab7569
e4949de
4c16c17
cd91650
f7fc09e
a6e6743
63dc0eb
73d7096
75d3423
3d1eda5
fc4dd57
16237aa
828970a
ab3ec0a
f357350
e27a0d8
f7c70b3
9bb8a87
7ef3213
1961b34
afc095e
def70a3
7fd6469
7ddb3a9
992008b
600b508
8b4a10f
82eec49
4dbb5b7
466c9b2
c3237db
a3f683a
4a8c2e8
8740e12
6cffd19
b7aca0c
47d0bc7
ad372be
fe6f4e9
cd6d092
06ea6ba
4f87f3e
daa54c4
03b1888
6152a03
eadab87
de6b323
9303d1d
627fee3
203577a
b715b69
0eb648b
d932e1d
ffa4eb1
1d3fbbd
a7a7819
ba4d20a
e481ac8
2961a48
099e4e9
17b7a4e
49c4c5c
75e1017
d994ca7
befc361
8753246
af05352
c70a2ea
ec57489
3c26ddf
d7adc73
fbc2334
1bc7da9
66702a5
8d20b1a
bce22a6
94a652f
ff510c0
f735e65
f598998
94f4b04
2b75990
3647fdf
c6a81a2
6b29f53
5163358
86393ef
9ee2f4b
e64236a
48dd684
60d024d
ee60ec8
1b1b019
7f54c00
a704f48
3ffb431
9739f90
63cf573
47e569f
7a1f04b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,13 +11,13 @@ import '../../../shared/heap/model.dart'; | |
import 'heap_diff.dart'; | ||
|
||
abstract class DiffListItem extends DisposableController { | ||
/// Number, that, if shown in name, should be unique in the list. | ||
/// Number, that if shown in name, should be unique in the list. | ||
/// | ||
/// If the number is not shown, it should be 0. | ||
/// If the number is not expected to be shown in UI, it should be 0. | ||
int get displayNumber; | ||
|
||
ValueListenable<bool> get isProcessing => _isProcessing; | ||
final _isProcessing = ValueNotifier<bool>(false); | ||
ValueListenable<bool> get isProcessing => _isProcessing; | ||
polina-c marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/// If true, the item contains data, that can be compared and analyzed. | ||
bool get hasData; | ||
|
@@ -37,11 +37,16 @@ class SnapshotListItem extends DiffListItem with AutoDisposeControllerMixin { | |
this.displayNumber, | ||
this._isolateName, | ||
this.diffStore, | ||
this.selectedClass, | ||
) { | ||
_isProcessing.value = true; | ||
receiver.whenComplete(() async { | ||
final data = await receiver; | ||
if (data != null) heap = AdaptedHeap(data); | ||
if (data != null) { | ||
heap = AdaptedHeap(data); | ||
updateSelectedRecord(); | ||
addAutoDisposeListener(selectedClass, () => updateSelectedRecord()); | ||
} | ||
_isProcessing.value = false; | ||
}); | ||
} | ||
|
@@ -59,7 +64,17 @@ class SnapshotListItem extends DiffListItem with AutoDisposeControllerMixin { | |
|
||
var sorting = ColumnSorting(); | ||
|
||
final diffWith = ValueNotifier<SnapshotListItem?>(null); | ||
final _diffWith = ValueNotifier<SnapshotListItem?>(null); | ||
ValueListenable<SnapshotListItem?> get diffWith => _diffWith; | ||
void setDiffWith(SnapshotListItem? value) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment around ordering. Also, any reason we aren't using a formal setter here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed ordering |
||
_diffWith.value = value; | ||
updateSelectedRecord(); | ||
} | ||
|
||
final ValueListenable<String?> selectedClass; | ||
polina-c marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
final _selectedRecord = ValueNotifier<HeapStatsRecord?>(null); | ||
ValueListenable<HeapStatsRecord?> get selectedRecord => _selectedRecord; | ||
polina-c marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
@override | ||
bool get hasData => heap != null; | ||
|
@@ -70,6 +85,9 @@ class SnapshotListItem extends DiffListItem with AutoDisposeControllerMixin { | |
if (itemToDiffWith == null) return theHeap.stats; | ||
return diffStore.compare(theHeap, itemToDiffWith.heap!).stats; | ||
} | ||
|
||
void updateSelectedRecord() => | ||
_selectedRecord.value = statsToShow.recordsByClass[selectedClass.value]; | ||
} | ||
|
||
class ColumnSorting { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,8 +10,8 @@ import 'package:flutter/foundation.dart'; | |
import '../../../../../primitives/utils.dart'; | ||
import '../../../primitives/memory_utils.dart'; | ||
import '../../../shared/heap/model.dart'; | ||
import 'Item_controller.dart'; | ||
import 'heap_diff.dart'; | ||
import 'model.dart'; | ||
|
||
class DiffPaneController { | ||
DiffPaneController(this.snapshotTaker); | ||
|
@@ -35,7 +35,7 @@ class DiffPaneController { | |
DiffListItem get selectedItem => snapshots.value[selectedIndex.value]; | ||
|
||
/// Full name for the selected class. | ||
ValueNotifier<String?> get selectedClass => _selectedClass; | ||
ValueListenable<String?> get selectedClass => _selectedClass; | ||
final _selectedClass = ValueNotifier<String?>(null); | ||
void setSelectedClass(String? value) => _selectedClass.value = value; | ||
|
||
|
@@ -52,6 +52,7 @@ class DiffPaneController { | |
_nextDisplayNumber(), | ||
currentIsolateName ?? '<isolate-not-detected>', | ||
diffStore, | ||
selectedClass, | ||
), | ||
); | ||
await future; | ||
|
@@ -61,6 +62,9 @@ class DiffPaneController { | |
} | ||
|
||
Future<void> clearSnapshots() async { | ||
for (var i = 0; i < snapshots.value.length; i++) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The index should start with 1 here. Fixed. |
||
snapshots.value[i].dispose(); | ||
} | ||
_snapshots.removeRange(1, snapshots.value.length); | ||
_selectedIndex.value = 0; | ||
} | ||
|
@@ -73,6 +77,7 @@ class DiffPaneController { | |
|
||
void deleteCurrentSnapshot() { | ||
assert(selectedItem is SnapshotListItem); | ||
selectedItem.dispose(); | ||
_snapshots.removeRange(selectedIndex.value, selectedIndex.value + 1); | ||
// We must change the selectedIndex, because otherwise the content will | ||
// not be re-rendered. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,8 +7,8 @@ import 'package:flutter/material.dart'; | |
import '../../../../../shared/common_widgets.dart'; | ||
import '../../../../../shared/split.dart'; | ||
import '../../../shared/heap/model.dart'; | ||
import '../controller/Item_controller.dart'; | ||
import '../controller/diff_pane_controller.dart'; | ||
import '../controller/model.dart'; | ||
import 'class_details.dart'; | ||
import 'stats_table.dart'; | ||
|
||
|
@@ -31,6 +31,8 @@ class SnapshotView extends StatelessWidget { | |
} else { | ||
final heap1 = item.heap!; | ||
final heap2 = item.diffWith.value!.heap!; | ||
|
||
// TODO(polina-c): make comparison async. | ||
stats = controller.diffStore.compare(heap1, heap2).stats; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if this can be expensive, we shouldn't do this in build. If it is expensive, we could do this in the controller before we set isProcessing to false (that way we still show a loading spinner while we are calculating this). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is ok for small applications. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this has the potential to slow down the UI at all, we should definitely ship it out to a future and have the UI with a future builder and a spinner or something :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok Added TODO and item to #3949 (comment) |
||
} | ||
|
||
|
@@ -54,7 +56,7 @@ class SnapshotView extends StatelessWidget { | |
), | ||
), | ||
const OutlineDecoration( | ||
child: ClassDetails(heapClass: null), | ||
child: HeapClassDetails(heapClass: null), | ||
), | ||
], | ||
); | ||
|
Uh oh!
There was an error while loading. Please reload this page.