Skip to content

Adding drop down to diff table #4354

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.

Sign up for GitHub

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

Merged
carolynqu merged 13 commits into from
Aug 10, 2022
Merged

Adding drop down to diff table #4354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a63832a
work in progress
Aug 5, 2022
45fcb02
drop down
Aug 5, 2022
3b30ab5
simplifying top level variables
Aug 8, 2022
e982b5d
setting back to false
Aug 8, 2022
c15104b
_activeDiffMap getter
Aug 9, 2022
e193b30
null check
Aug 9, 2022
69d1589
adding tests
Aug 9, 2022
788e0dd
no deferred app
Aug 9, 2022
62db729
fixing tests
Aug 9, 2022
1471f5b
format
Aug 9, 2022
08ba2d6
adding spacing
Aug 10, 2022
526d6b4
clearing gets rid of deferred
Aug 10, 2022
66ba7dc
will put clearing logic in another pr
Aug 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 159 additions & 62 deletions packages/devtools_app/lib/src/screens/app_size/app_size_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,20 @@ enum AppUnit {
}
}

class DiffTreeMap {
const DiffTreeMap({
required this.combined,
required this.increaseOnly,
required this.decreaseOnly,
});

final TreemapNode? combined;

final TreemapNode? increaseOnly;

final TreemapNode? decreaseOnly;
}

class AppSizeController {
static const unsupportedFileTypeError =
'Failed to load size analysis file: file type not supported.\n\n'
Expand Down Expand Up @@ -160,21 +174,43 @@ class AppSizeController {
}
}

DiffTreeMap? get _activeDiffMap {
final appUnit = _selectedAppUnit.value;

switch (appUnit) {
case AppUnit.mainOnly:
return _mainDiffTreeMap;
case AppUnit.deferredOnly:
return _deferredDiffTreeMap;
case AppUnit.entireApp:
default:
return _diffTreeMap;
}
}

TreemapNode? get _activeDiffRoot {
switch (_activeDiffTreeType.value) {
final diffTreeType = _activeDiffTreeType.value;
final diffMap = _activeDiffMap;
Comment thread
carolynqu marked this conversation as resolved.

if (diffMap == null) {
return null;
}

switch (diffTreeType) {
case DiffTreeType.increaseOnly:
return _increasedDiffTreeRoot;
return diffMap.increaseOnly;
case DiffTreeType.decreaseOnly:
return _decreasedDiffTreeRoot;
return diffMap.decreaseOnly;
case DiffTreeType.combined:
return diffMap.combined;
default:
return _combinedDiffTreeRoot;
return diffMap.combined;
}
}

TreemapNode? _increasedDiffTreeRoot;
TreemapNode? _decreasedDiffTreeRoot;
TreemapNode? _combinedDiffTreeRoot;
DiffTreeMap? _diffTreeMap;
DiffTreeMap? _mainDiffTreeMap;
DiffTreeMap? _deferredDiffTreeMap;

Map<String, dynamic>? get _dataForAppUnit {
switch (_selectedAppUnit.value) {
Expand Down Expand Up @@ -220,9 +256,9 @@ class AppSizeController {
_diffRoot.value = null;
_oldDiffJsonFile.value = null;
_newDiffJsonFile.value = null;
_increasedDiffTreeRoot = null;
_decreasedDiffTreeRoot = null;
_combinedDiffTreeRoot = null;
_diffTreeMap = null;
_mainDiffTreeMap = null;
_deferredDiffTreeMap = null;
_diffCallGraphRoot.value = null;
_oldDiffCallGraph = null;
_newDiffCallGraph = null;
Expand Down Expand Up @@ -252,9 +288,13 @@ class AppSizeController {
ValueListenable<AppUnit> get selectedAppUnit => _selectedAppUnit;
final _selectedAppUnit = ValueNotifier<AppUnit>(AppUnit.entireApp);

void changeSelectedAppUnit(AppUnit appUnit) {
void changeSelectedAppUnit(AppUnit appUnit, Key tabKey) {
_selectedAppUnit.value = appUnit;
_loadApp(_dataForAppUnit!);
if (tabKey == AppSizeScreen.analysisTabKey) {
_loadApp(_dataForAppUnit!);
} else {
changeDiffRoot(_activeDiffRoot);
}
}

/// Notifies that the json files are currently being processed.
Expand Down Expand Up @@ -343,7 +383,8 @@ class AppSizeController {
) {
if (_hasDeferredInfo(jsonFile)) {
jsonFile['children'] = _extractChildren(jsonFile)
.where((child) => child['isDeferred'] == true);
.where((child) => child['isDeferred'] == true)
.toList();
jsonFile['n'] = _deferredNodeName;
}
return jsonFile;
Expand Down Expand Up @@ -385,55 +426,54 @@ class AppSizeController {
await delayForBatchProcessing(micros: 10000);

Map<String, dynamic> diffMap;
Map<String, dynamic>? mainDiffMap;
Map<String, dynamic>? deferredDiffMap;

if (oldFile.isAnalyzeSizeFile && newFile.isAnalyzeSizeFile) {
var oldFileJson = oldFile.data as Map<String, dynamic>;
var newFileJson = newFile.data as Map<String, dynamic>;
final oldFileJson = oldFile.data as Map<String, dynamic>;
final newFileJson = newFile.data as Map<String, dynamic>;

if (_hasDeferredInfo(oldFileJson) || _hasDeferredInfo(newFileJson)) {
if (!_hasDeferredInfo(oldFileJson) && !_hasDeferredInfo(newFileJson)) {
diffMap = _generateDiffMapFromAnalyzeSizeFiles(
oldFileJson: oldFileJson,
newFileJson: newFileJson,
);
} else {
_isDeferredApp.value = deferredLoadingSupportEnabled;
Map<String, dynamic> oldEntireAppFileJson = oldFileJson;
Map<String, dynamic> newEntireAppFileJson = newFileJson;

if (!_hasDeferredInfo(oldFileJson)) {
oldFileJson = _wrapInArtificialRoot(oldFileJson);
oldEntireAppFileJson = _wrapInArtificialRoot(oldFileJson);
} else if (!_hasDeferredInfo(newFileJson)) {
newFileJson = _wrapInArtificialRoot(newFileJson);
newEntireAppFileJson = _wrapInArtificialRoot(newFileJson);
}
}

final oldApkProgramInfo = ProgramInfo();
_apkJsonToProgramInfo(
program: oldApkProgramInfo,
parent: oldApkProgramInfo.root,
json: oldFileJson,
);
final oldMainOnlyFileJson =
_extractMainUnit(Map.from(oldEntireAppFileJson));
final newMainOnlyFileJson =
_extractMainUnit(Map.from(newEntireAppFileJson));

// Extract the precompiler trace from the old file, if it exists, and
// generate a call graph.
final oldPrecompilerTrace = oldFileJson.remove('precompiler-trace');
if (oldPrecompilerTrace != null) {
_oldDiffCallGraph = generateCallGraphWithDominators(
oldPrecompilerTrace,
NodeType.packageNode,
final oldDeferredOnlyFileJson =
_extractDeferredUnits(Map.from(oldEntireAppFileJson));
final newDeferredOnlyFileJson =
_extractDeferredUnits(Map.from(newEntireAppFileJson));

diffMap = _generateDiffMapFromAnalyzeSizeFiles(
oldFileJson: oldEntireAppFileJson,
newFileJson: newEntireAppFileJson,
);
}

final newApkProgramInfo = ProgramInfo();
_apkJsonToProgramInfo(
program: newApkProgramInfo,
parent: newApkProgramInfo.root,
json: newFileJson,
);
mainDiffMap = _generateDiffMapFromAnalyzeSizeFiles(
oldFileJson: oldMainOnlyFileJson,
newFileJson: newMainOnlyFileJson,
);

// Extract the precompiler trace from the new file, if it exists, and
// generate a call graph.
final newPrecompilerTrace = newFileJson.remove('precompiler-trace');
if (newPrecompilerTrace != null) {
_newDiffCallGraph = generateCallGraphWithDominators(
newPrecompilerTrace,
NodeType.packageNode,
deferredDiffMap = _generateDiffMapFromAnalyzeSizeFiles(
oldFileJson: oldDeferredOnlyFileJson,
newFileJson: newDeferredOnlyFileJson,
);
}

diffMap = compareProgramInfo(oldApkProgramInfo, newApkProgramInfo);
} else {
try {
diffMap = buildComparisonTreemap(oldFile.data, newFile.data);
Expand All @@ -458,25 +498,82 @@ class AppSizeController {
diffMap['n'] = isDeferredApp.value ? _entireAppNodeName : _rootNodeName;

// TODO(peterdjlee): Try to move the non-active tree generation to separate isolates.
_combinedDiffTreeRoot = generateDiffTree(
diffMap,
DiffTreeType.combined,
skipNodesWithNoByteSizeChange: !isDeferredApp.value,
// Entire app or root (for non-deferred):
_diffTreeMap = _generateDiffTrees(diffMap);

if (isDeferredApp.value) {
// For main only:
_mainDiffTreeMap = _generateDiffTrees(mainDiffMap!);

// For deferred only:
_deferredDiffTreeMap = _generateDiffTrees(deferredDiffMap!);
}

changeDiffRoot(_activeDiffRoot);

_processingNotifier.value = false;
}

DiffTreeMap _generateDiffTrees(Map<String, dynamic> diffMap) {
// TODO(peterdjlee): Try to move the non-active tree generation to separate isolates.
return DiffTreeMap(
combined: generateDiffTree(
diffMap,
DiffTreeType.combined,
skipNodesWithNoByteSizeChange: !isDeferredApp.value,
),
increaseOnly: generateDiffTree(
diffMap,
DiffTreeType.increaseOnly,
skipNodesWithNoByteSizeChange: !isDeferredApp.value,
),
decreaseOnly: generateDiffTree(
diffMap,
DiffTreeType.decreaseOnly,
skipNodesWithNoByteSizeChange: !isDeferredApp.value,
),
);
_increasedDiffTreeRoot = generateDiffTree(
diffMap,
DiffTreeType.increaseOnly,
skipNodesWithNoByteSizeChange: !isDeferredApp.value,
}

Map<String, dynamic> _generateDiffMapFromAnalyzeSizeFiles({
required Map<String, dynamic> oldFileJson,
required Map<String, dynamic> newFileJson,
}) {
final oldApkProgramInfo = ProgramInfo();
_apkJsonToProgramInfo(
program: oldApkProgramInfo,
parent: oldApkProgramInfo.root,
json: oldFileJson,
);
_decreasedDiffTreeRoot = generateDiffTree(
diffMap,
DiffTreeType.decreaseOnly,
skipNodesWithNoByteSizeChange: !isDeferredApp.value,

// Extract the precompiler trace from the old file, if it exists, and
// generate a call graph.
final oldPrecompilerTrace = oldFileJson.remove('precompiler-trace');
if (oldPrecompilerTrace != null) {
_oldDiffCallGraph = generateCallGraphWithDominators(
oldPrecompilerTrace,
NodeType.packageNode,
);
}

final newApkProgramInfo = ProgramInfo();
_apkJsonToProgramInfo(
program: newApkProgramInfo,
parent: newApkProgramInfo.root,
json: newFileJson,
);

changeDiffRoot(_activeDiffRoot);
// Extract the precompiler trace from the new file, if it exists, and
// generate a call graph.
final newPrecompilerTrace = newFileJson.remove('precompiler-trace');
if (newPrecompilerTrace != null) {
_newDiffCallGraph = generateCallGraphWithDominators(
newPrecompilerTrace,
NodeType.packageNode,
);
}

_processingNotifier.value = false;
return compareProgramInfo(oldApkProgramInfo, newApkProgramInfo);
}

Map<String, dynamic> _wrapInArtificialRoot(Map<String, dynamic> json) {
Expand Down
12 changes: 6 additions & 6 deletions packages/devtools_app/lib/src/screens/app_size/app_size_screen.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,11 @@ class _AppSizeBodyState extends State<AppSizeBody>
),
Row(
children: [
if (currentTab.key == AppSizeScreen.analysisTabKey &&
isDeferredApp)
_buildAppUnitDropdown(),
if (currentTab.key == AppSizeScreen.diffTabKey)
if (isDeferredApp) _buildAppUnitDropdown(currentTab.key!),
Copy link
Copy Markdown
Member

@kenzieschmoll kenzieschmoll Aug 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some spacing here:

if (isDeferredApp) ...[
  _buildAppUnitDropdown(currentTab.key!),
  const SizedBox(width: defaultSpacing),
],

Copy link
Copy Markdown
Contributor Author

@carolynqu carolynqu Aug 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added spacing so that it only impacts the visuals when both drop downs are there.

if (currentTab.key == AppSizeScreen.diffTabKey) ...[
const SizedBox(width: defaultSpacing),
_buildDiffTreeTypeDropdown(),
],
const SizedBox(width: defaultSpacing),
_buildClearButton(currentTab.key!),
],
Expand Down Expand Up @@ -253,7 +253,7 @@ class _AppSizeBodyState extends State<AppSizeBody>
);
}

DropdownButtonHideUnderline _buildAppUnitDropdown() {
DropdownButtonHideUnderline _buildAppUnitDropdown(Key tabKey) {
return DropdownButtonHideUnderline(
key: AppSizeScreen.appUnitDropdownKey,
child: DropdownButton<AppUnit>(
Expand All @@ -265,7 +265,7 @@ class _AppSizeBodyState extends State<AppSizeBody>
],
onChanged: (newAppUnit) {
setState(() {
controller.changeSelectedAppUnit(newAppUnit!);
controller.changeSelectedAppUnit(newAppUnit!, tabKey);
});
},
),
Expand Down
Loading