From 813bc41f96cb44af54b11a5e6a095af051aa5039 Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Wed, 2 Nov 2022 14:13:36 -0400 Subject: [PATCH 1/2] Add support for grouping samples by tag This PR adds support for grouping CPU samples by user or VM tags. This gives developers the ability to see the full CPU profile broken down by user or VM tags all at once rather than having to inspect samples from one tag at a time. This change also removes the root node from the displayed CPU profile tree, meaning that the 'all' node will no longer be displayed. --- .../profiler/cpu_profile_controller.dart | 28 ++- .../screens/profiler/cpu_profile_model.dart | 223 +++++++++++++++-- .../profiler/cpu_profile_transformer.dart | 21 +- .../src/screens/profiler/cpu_profiler.dart | 62 +++-- .../profiler/profiler_screen_controller.dart | 10 +- .../lib/src/shared/profiler_utils.dart | 67 +++++- .../cpu_profiler/cpu_profile_model_test.dart | 27 +++ .../cpu_profile_transformer_test.dart | 31 +++ .../cpu_profiler_controller_test.dart | 82 +++++++ .../test/cpu_profiler/cpu_profiler_test.dart | 118 +++++++++ .../test/test_data/cpu_profile.dart | 224 ++++++++++++++++-- 11 files changed, 809 insertions(+), 84 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_controller.dart b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_controller.dart index 745a705a6f0..21712cf9c6d 100644 --- a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_controller.dart +++ b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_controller.dart @@ -29,6 +29,11 @@ enum CpuProfilerViewType { } } +enum CpuProfilerTagType { + user, + vm; +} + class CpuProfilerController with SearchControllerMixin, @@ -41,6 +46,10 @@ class CpuProfilerController /// the message that no filters are applied. static const userTagNone = 'none'; + /// Special tags that represent profiles broken down by user or VM tags. + static const groupByUserTag = '#group-by-user-tag'; + static const groupByVmTag = '#group-by-vm-tag'; + /// User tag that marks app start up for Flutter apps. /// /// This needs to match the tag set and unset at these locations, @@ -475,7 +484,6 @@ class CpuProfilerController } return filteredDataForTag; } - var data = cpuProfileStore.lookupProfile( label: tag, ); @@ -483,7 +491,17 @@ class CpuProfilerController final fullData = cpuProfileStore.lookupProfile( label: userTagNone, )!; - data = CpuProfilePair.fromUserTag(fullData, tag); + + if (tag == groupByUserTag || tag == groupByVmTag) { + data = CpuProfilePair.withTagRoots( + fullData, + tag == groupByUserTag + ? CpuProfilerTagType.user + : CpuProfilerTagType.vm, + ); + } else { + data = CpuProfilePair.fromUserTag(fullData, tag); + } cpuProfileStore.storeProfile( data, label: tag, @@ -508,7 +526,9 @@ class CpuProfilerController void updateView(CpuProfilerViewType view) { _viewType.value = view; _dataNotifier.value = cpuProfileStore - .lookupProfile(label: _wrapWithFilterSuffix(userTagNone)) + .lookupProfile( + label: _wrapWithFilterSuffix(_userTagFilter.value), + ) ?.getActive(view); } @@ -520,7 +540,6 @@ class CpuProfilerController Future clear() async { reset(); - cpuProfileStore.clear(); await serviceManager.service!.clearSamples(); } @@ -530,6 +549,7 @@ class CpuProfilerController _processingNotifier.value = false; _userTagFilter.value = userTagNone; transformer.reset(); + cpuProfileStore.clear(); resetSearch(); } diff --git a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_model.dart b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_model.dart index 3a6e4926d24..cfbe15521fc 100644 --- a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_model.dart +++ b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_model.dart @@ -77,6 +77,24 @@ class CpuProfilePair { return CpuProfilePair(functionProfile: function, codeProfile: code); } + factory CpuProfilePair.withTagRoots( + CpuProfilePair original, + CpuProfilerTagType tag, + ) { + final function = CpuProfileData.withTagRoots( + original.functionProfile, + tag, + ); + CpuProfileData? code; + if (original.codeProfile != null) { + code = CpuProfileData.withTagRoots( + original.codeProfile!, + tag, + ); + } + return CpuProfilePair(functionProfile: function, codeProfile: code); + } + /// Represents the function view of the CPU profile. This view displays /// function objects rather than code objects, which can potentially contain /// multiple inlined functions. @@ -141,18 +159,9 @@ class CpuProfileData { required this.stackFrames, required this.cpuSamples, required this.profileMetaData, + required this.rootedAtTags, }) { - _cpuProfileRoot = CpuStackFrame._( - id: rootId, - name: rootName, - verboseName: rootName, - category: 'Dart', - rawUrl: '', - packageUri: '', - sourceLine: null, - profileMetaData: profileMetaData, - parentId: null, - ); + _cpuProfileRoot = CpuStackFrame.root(profileMetaData); } factory CpuProfileData.parse(Map json) { @@ -190,6 +199,7 @@ class CpuProfileData { sourceLine: stackFrameJson[sourceLineKey], parentId: stackFrameJson[parentIdKey] ?? rootId, profileMetaData: profileMetaData, + isTag: false, ); stackFrames[stackFrame.id] = stackFrame; } @@ -206,6 +216,7 @@ class CpuProfileData { stackFrames: stackFrames, cpuSamples: samples, profileMetaData: profileMetaData, + rootedAtTags: false, ); } @@ -249,20 +260,138 @@ class CpuProfileData { stackDepth: superProfile.profileMetaData.stackDepth, time: subTimeRange, ), + rootedAtTags: superProfile.rootedAtTags, + ); + } + + /// Generate a cpu profile from [originalData] where the profile is broken + /// down for each tag of the given [type]. + /// + /// [originalData] does not need to be [processed] to run this operation. + factory CpuProfileData.withTagRoots( + CpuProfileData originalData, + CpuProfilerTagType type, + ) { + final useUserTags = type == CpuProfilerTagType.user; + final tags = { + for (final sample in originalData.cpuSamples) + useUserTags ? sample.userTag! : sample.vmTag!, + }; + + final tagProfiles = { + for (final tag in tags) + tag: CpuProfileData._fromTag(originalData, tag, type), + }; + + final metaData = originalData.profileMetaData.copyWith(); + + // Use a SplayTreeMap so that map iteration will be in sorted key order. + // This keeps the visualization of the profile as consistent as possible + // when applying filters. + final SplayTreeMap stackFrames = + SplayTreeMap(stackFrameIdCompare); + + final samples = []; + + int nextId = 1; + + for (final tagProfileEntry in tagProfiles.entries) { + final tag = tagProfileEntry.key; + final tagProfile = tagProfileEntry.value; + if (tagProfile.cpuSamples.isEmpty) { + continue; + } + final isolateId = tagProfile.cpuSamples.first.leafId.split('-').first; + final tagId = '$isolateId-${nextId++}'; + stackFrames[tagId] = CpuStackFrame._( + id: tagId, + name: tag, + verboseName: tag, + category: 'Dart', + rawUrl: '', + packageUri: '', + sourceLine: null, + parentId: null, + profileMetaData: metaData, + isTag: true, + ); + final idMapping = { + rootId: tagId, + }; + + String? getId(String? id) { + if (id == null) { + return null; + } + return idMapping.putIfAbsent(id, () => '$isolateId-${nextId++}'); + } + + tagProfile.stackFrames.forEach((k, v) => getId(k)); + + for (final sample in tagProfile.cpuSamples) { + String? updatedId = getId(sample.leafId); + samples.add( + CpuSampleEvent( + leafId: updatedId!, + userTag: sample.userTag, + vmTag: sample.vmTag, + traceJson: sample.toJson, + ), + ); + var currentStackFrame = tagProfile.stackFrames[sample.leafId]; + while (currentStackFrame != null) { + final parentId = getId(currentStackFrame.parentId); + stackFrames[updatedId!] = currentStackFrame.shallowCopy( + id: updatedId, + copySampleCounts: false, + profileMetaData: metaData, + parentId: parentId, + ); + final parentStackFrameJson = parentId != null + ? originalData.stackFrames[currentStackFrame.parentId] + : null; + updatedId = parentId; + currentStackFrame = parentStackFrameJson; + } + } + } + return CpuProfileData._( + stackFrames: stackFrames, + cpuSamples: samples, + profileMetaData: metaData, + rootedAtTags: true, ); } + /// Generate a cpu profile from [originalData] where each sample contains the + /// vmTag [tag]. + /// + /// [originalData] does not need to be [processed] to run this operation. + factory CpuProfileData.fromVMTag(CpuProfileData originalData, String tag) { + return CpuProfileData._fromTag(originalData, tag, CpuProfilerTagType.vm); + } + /// Generate a cpu profile from [originalData] where each sample contains the /// userTag [tag]. /// /// [originalData] does not need to be [processed] to run this operation. factory CpuProfileData.fromUserTag(CpuProfileData originalData, String tag) { - if (!originalData.userTags.contains(tag)) { + return CpuProfileData._fromTag(originalData, tag, CpuProfilerTagType.user); + } + + factory CpuProfileData._fromTag( + CpuProfileData originalData, + String tag, + CpuProfilerTagType type, + ) { + final useUserTag = type == CpuProfilerTagType.user; + final tags = useUserTag ? originalData.userTags : originalData.vmTags; + if (!tags.contains(tag)) { return CpuProfileData.empty(); } final samplesWithTag = originalData.cpuSamples - .where((sample) => sample.userTag == tag) + .where((sample) => (useUserTag ? sample.userTag : sample.vmTag) == tag) .toList(); assert(samplesWithTag.isNotEmpty); @@ -312,6 +441,7 @@ class CpuProfileData { stackFrames: stackFramesWithTag, cpuSamples: samplesWithTag, profileMetaData: metaData, + rootedAtTags: false, ); } @@ -334,6 +464,7 @@ class CpuProfileData { CpuSampleEvent( leafId: stackFrame.id, userTag: sample.userTag, + vmTag: sample.vmTag, traceJson: sampleJson, ), ); @@ -412,6 +543,7 @@ class CpuProfileData { stackFrames: filteredStackFrames, cpuSamples: filteredCpuSamples, profileMetaData: updatedMetaData, + rootedAtTags: originalData.rootedAtTags, ); } @@ -495,8 +627,8 @@ class CpuProfileData { 'cat': 'Dart', CpuProfileData.stackFrameIdKey: '$isolateId-${tree.frameId}', 'args': { - if (sample.userTag != null) 'userTag': sample.userTag, - if (sample.vmTag != null) 'vmTag': sample.vmTag, + if (sample.userTag != null) userTagKey: sample.userTag, + if (sample.vmTag != null) vmTagKey: sample.vmTag, }, }); } @@ -572,6 +704,7 @@ class CpuProfileData { static const timeOriginKey = 'timeOriginMicros'; static const timeExtentKey = 'timeExtentMicros'; static const userTagKey = 'userTag'; + static const vmTagKey = 'vmTag'; final Map stackFrames; @@ -579,12 +712,20 @@ class CpuProfileData { final CpuProfileMetaData profileMetaData; + /// `true` if the CpuProfileData has tag-based roots. This value is used + /// during the bottom-up transformation to ensure that the tag-based roots + /// are kept at the root of the resulting bottom-up tree. + final bool rootedAtTags; + /// Marks whether this data has already been processed. bool processed = false; List get callTreeRoots { if (!processed) return []; - return _callTreeRoots ??= [_cpuProfileRoot.deepCopy()]; + return _callTreeRoots ??= [ + // Don't display the root node. + ..._cpuProfileRoot.children.map((e) => e.deepCopy()) + ]; } List? _callTreeRoots; @@ -595,6 +736,7 @@ class CpuProfileData { BottomUpTransformer().bottomUpRootsFor( topDownRoot: _cpuProfileRoot, mergeSamples: mergeCpuProfileRoots, + rootedAtTags: rootedAtTags, ); } @@ -617,7 +759,23 @@ class CpuProfileData { return _userTags!; } + Iterable get vmTags { + if (_vmTags != null) { + return _vmTags!; + } + final tags = {}; + for (final cpuSample in cpuSamples) { + final tag = cpuSample.vmTag; + if (tag != null) { + tags.add(tag); + } + } + _vmTags = tags; + return _vmTags!; + } + Iterable? _userTags; + Iterable? _vmTags; late final CpuStackFrame _cpuProfileRoot; @@ -678,7 +836,8 @@ class CpuProfileMetaData extends ProfileMetaData { class CpuSampleEvent extends TraceEvent { CpuSampleEvent({ required this.leafId, - this.userTag, + required this.userTag, + required this.vmTag, required Map traceJson, }) : super(traceJson); @@ -687,9 +846,13 @@ class CpuSampleEvent extends TraceEvent { final userTag = traceJson[TraceEvent.argsKey] != null ? traceJson[TraceEvent.argsKey][CpuProfileData.userTagKey] : null; + final vmTag = traceJson[TraceEvent.argsKey] != null + ? traceJson[TraceEvent.argsKey][CpuProfileData.vmTagKey] + : null; return CpuSampleEvent( leafId: leafId, userTag: userTag, + vmTag: vmTag, traceJson: traceJson, ); } @@ -697,6 +860,7 @@ class CpuSampleEvent extends TraceEvent { final String leafId; final String? userTag; + final String? vmTag; Map get toJson { // [leafId] is the source of truth for the leaf id of this sample. @@ -721,6 +885,7 @@ class CpuStackFrame extends TreeNode required int? sourceLine, required String parentId, required CpuProfileMetaData profileMetaData, + required bool isTag, }) { return CpuStackFrame._( id: id, @@ -732,9 +897,24 @@ class CpuStackFrame extends TreeNode sourceLine: sourceLine, parentId: parentId, profileMetaData: profileMetaData, + isTag: isTag, ); } + factory CpuStackFrame.root(CpuProfileMetaData profileMetaData) => + CpuStackFrame._( + id: CpuProfileData.rootId, + name: CpuProfileData.rootName, + verboseName: CpuProfileData.rootName, + category: 'Dart', + rawUrl: '', + packageUri: '', + sourceLine: null, + profileMetaData: profileMetaData, + parentId: null, + isTag: false, + ); + CpuStackFrame._({ required this.id, required this.name, @@ -745,6 +925,7 @@ class CpuStackFrame extends TreeNode required this.sourceLine, required this.parentId, required CpuProfileMetaData profileMetaData, + required this.isTag, }) : _profileMetaData = profileMetaData; /// Prefix for packages from the core Dart libraries. @@ -786,9 +967,12 @@ class CpuStackFrame extends TreeNode @override String get displayName => name; + final bool isTag; + bool get isNative => _isNative ??= id != CpuProfileData.rootId && packageUri.isEmpty && - !name.startsWith(flutterEnginePrefix); + !name.startsWith(flutterEnginePrefix) && + !isTag; bool? _isNative; @@ -814,6 +998,8 @@ class CpuStackFrame extends TreeNode prefix = '[Dart]'; } else if (isFlutterCore) { prefix = '[Flutter]'; + } else if (isTag) { + prefix = '[Tag]'; } final nameWithPrefix = [prefix, name].join(' '); return [ @@ -858,6 +1044,7 @@ class CpuStackFrame extends TreeNode sourceLine: sourceLine ?? this.sourceLine, parentId: parentId ?? this.parentId, profileMetaData: profileMetaData ?? this.profileMetaData, + isTag: isTag, ); if (copySampleCounts) { copy diff --git a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_transformer.dart b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_transformer.dart index bb544d6727b..0fb77b4d8c7 100644 --- a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_transformer.dart +++ b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_transformer.dart @@ -69,6 +69,21 @@ class CpuProfileTransformer { } } + if (cpuProfileData.rootedAtTags) { + // Check to see if there are any empty tag roots as a result of filtering + // and remove them. + final nodesToRemove = []; + for (int i = 0; i < cpuProfileData.cpuProfileRoot.children.length; ++i) { + final root = cpuProfileData.cpuProfileRoot.children[i]; + if (root.isTag && root.children.isEmpty) { + nodesToRemove.add(i); + } + } + nodesToRemove.reversed.forEach( + cpuProfileData.cpuProfileRoot.removeChildAtIndex, + ); + } + _setExclusiveSampleCountsAndTags(cpuProfileData); cpuProfileData.processed = true; @@ -111,9 +126,9 @@ class CpuProfileTransformer { CpuStackFrame? parent, CpuProfileData cpuProfileData, ) { + // [stackFrame] is the root of a new cpu sample. Add it as a child of + // [cpuProfile]. if (parent == null) { - // [stackFrame] is the root of a new cpu sample. Add it as a child of - // [cpuProfile]. cpuProfileData.cpuProfileRoot.addChild(stackFrame); } else { parent.addChild(stackFrame); @@ -131,7 +146,7 @@ class CpuProfileTransformer { 'you must export the timeline immediately after the AssertionError is ' 'thrown.', ); - if (stackFrame != null) { + if (stackFrame != null && !stackFrame.isTag) { stackFrame.exclusiveSampleCount++; } } diff --git a/packages/devtools_app/lib/src/screens/profiler/cpu_profiler.dart b/packages/devtools_app/lib/src/screens/profiler/cpu_profiler.dart index aef06939726..8e07b3eafe8 100644 --- a/packages/devtools_app/lib/src/screens/profiler/cpu_profiler.dart +++ b/packages/devtools_app/lib/src/screens/profiler/cpu_profiler.dart @@ -418,31 +418,47 @@ class UserTagDropdown extends StatelessWidget { height: defaultButtonHeight, child: DevToolsTooltip( message: tooltip, - child: RoundedDropDownButton( - isDense: true, - style: Theme.of(context).textTheme.bodyMedium, - value: userTag, - items: [ - _buildMenuItem( - display: '$filterByTag ${CpuProfilerController.userTagNone}', - value: CpuProfilerController.userTagNone, - ), - // We don't want to show the 'Default' tag if it is the only - // tag available. The 'none' tag above is equivalent in this - // case. - if (!(userTags.length == 1 && - userTags.first == UserTag.defaultTag.label)) - for (final tag in userTags) + child: ValueListenableBuilder( + valueListenable: preferences.vmDeveloperModeEnabled, + builder: (context, vmDeveloperModeEnabled, _) { + return RoundedDropDownButton( + isDense: true, + style: Theme.of(context).textTheme.bodyMedium, + value: userTag, + items: [ _buildMenuItem( - display: '$filterByTag $tag', - value: tag, + display: + '$filterByTag ${CpuProfilerController.userTagNone}', + value: CpuProfilerController.userTagNone, ), - ], - onChanged: userTags.isEmpty || - (userTags.length == 1 && - userTags.first == UserTag.defaultTag.label) - ? null - : (String? tag) => _onUserTagChanged(tag!, context), + // We don't want to show the 'Default' tag if it is the only + // tag available. The 'none' tag above is equivalent in this + // case. + if (!(userTags.length == 1 && + userTags.first == UserTag.defaultTag.label)) ...[ + for (final tag in userTags) + _buildMenuItem( + display: '$filterByTag $tag', + value: tag, + ), + _buildMenuItem( + display: 'Group by: User Tag', + value: CpuProfilerController.groupByUserTag, + ), + ], + if (vmDeveloperModeEnabled) + _buildMenuItem( + display: 'Group by: VM Tag', + value: CpuProfilerController.groupByVmTag, + ) + ], + onChanged: userTags.isEmpty || + (userTags.length == 1 && + userTags.first == UserTag.defaultTag.label) + ? null + : (String? tag) => _onUserTagChanged(tag!, context), + ); + }, ), ), ); diff --git a/packages/devtools_app/lib/src/screens/profiler/profiler_screen_controller.dart b/packages/devtools_app/lib/src/screens/profiler/profiler_screen_controller.dart index e08b6a6cdce..787cb8b75a8 100644 --- a/packages/devtools_app/lib/src/screens/profiler/profiler_screen_controller.dart +++ b/packages/devtools_app/lib/src/screens/profiler/profiler_screen_controller.dart @@ -31,13 +31,21 @@ class ProfilerScreenController extends DisposableController switchToIsolate(serviceManager.isolateManager.selectedIsolate.value); }); - addAutoDisposeListener(preferences.vmDeveloperModeEnabled, () { + addAutoDisposeListener(preferences.vmDeveloperModeEnabled, () async { if (preferences.vmDeveloperModeEnabled.value) { // If VM developer mode was just enabled, clear the profile store // since the existing entries won't have code profiles and cannot be // constructed from function profiles. cpuProfilerController.cpuProfileStore.clear(); cpuProfilerController.reset(); + } else { + // If VM developer mode is disabled and we're grouping by VM tags, we + // need to default to the basic view of the profile. + final userTagFilter = cpuProfilerController.userTagFilter.value; + if (userTagFilter == CpuProfilerController.groupByVmTag) { + await cpuProfilerController + .loadDataWithTag(CpuProfilerController.userTagNone); + } } // Always reset to the function view when the VM developer mode state // changes. The selector is hidden when VM developer mode is disabled diff --git a/packages/devtools_app/lib/src/shared/profiler_utils.dart b/packages/devtools_app/lib/src/shared/profiler_utils.dart index c40ead794a9..275943cfc1e 100644 --- a/packages/devtools_app/lib/src/shared/profiler_utils.dart +++ b/packages/devtools_app/lib/src/shared/profiler_utils.dart @@ -102,23 +102,70 @@ class ProfileMetaData { /// Process for converting a [ProfilableDataMixin] into a bottom-up /// representation of the profile. +/// +/// [rootedAtTags] specifies whether or not the top-down tree is rooted +/// at synthetic nodes representing user / VM tags. class BottomUpTransformer> { List bottomUpRootsFor({ required T topDownRoot, required void Function(List) mergeSamples, + // TODO(bkonyi): can this take a list instead of a single root? + required bool rootedAtTags, }) { - final bottomUpRoots = generateBottomUpRoots( - node: topDownRoot, - currentBottomUpRoot: null, - bottomUpRoots: [], - ); + List bottomUpRoots; + // If the top-down tree has synthetic tag nodes as its roots, we need to + // skip the synthetic nodes when inverting the tree and re-insert them at + // the root. + if (rootedAtTags) { + bottomUpRoots = []; + for (final tagRoot in topDownRoot.children) { + final root = tagRoot.shallowCopy() as T; + + // Generate bottom up roots for each child of the synthetic tag node + // and insert them into the new synthetic tag node, [root]. + for (final child in tagRoot.children) { + root.addAllChildren( + generateBottomUpRoots( + node: child, + currentBottomUpRoot: null, + bottomUpRoots: [], + ), + ); + } + + // Cascade sample counts only for the non-tag nodes as the tag nodes + // are synthetic and we'll calculate the counts for the tag nodes + // later. + root.children.forEach(cascadeSampleCounts); + mergeSamples(root.children); + bottomUpRoots.add(root); + } + } else { + bottomUpRoots = generateBottomUpRoots( + node: topDownRoot, + currentBottomUpRoot: null, + bottomUpRoots: [], + ); - // Set the bottom up sample counts for each sample. - bottomUpRoots.forEach(cascadeSampleCounts); + // Set the bottom up sample counts for each sample. + bottomUpRoots.forEach(cascadeSampleCounts); - // Merge samples when possible starting at the root (the leaf node of the - // original sample). - mergeSamples(bottomUpRoots); + // Merge samples when possible starting at the root (the leaf node of the + // original sample). + mergeSamples(bottomUpRoots); + } + + if (rootedAtTags) { + // Calculate the total time for each tag root. The sum of the inclusive + // times for each child for the tag node is the total time spent with the + // given tag active. + for (final tagRoot in bottomUpRoots) { + tagRoot._inclusiveSampleCount = tagRoot.children.fold( + 0, + (prev, e) => prev + e._inclusiveSampleCount!, + ); + } + } return bottomUpRoots; } diff --git a/packages/devtools_app/test/cpu_profiler/cpu_profile_model_test.dart b/packages/devtools_app/test/cpu_profiler/cpu_profile_model_test.dart index b80f44ecd30..6e23ae14a03 100644 --- a/packages/devtools_app/test/cpu_profiler/cpu_profile_model_test.dart +++ b/packages/devtools_app/test/cpu_profiler/cpu_profile_model_test.dart @@ -205,6 +205,7 @@ void main() { sourceLine: null, parentId: '', profileMetaData: profileMetaData, + isTag: false, ).isNative, isFalse, ); @@ -223,6 +224,28 @@ void main() { expect(flutterEngineStackFrame.isFlutterCore, isTrue); }); + test('isTag', () { + expect(stackFrameA.isTag, isFalse); + expect(stackFrameB.isTag, isFalse); + expect(stackFrameC.isTag, isFalse); + expect(flutterEngineStackFrame.isTag, isFalse); + expect( + CpuStackFrame( + id: CpuProfileData.rootId, + name: 'MyTag', + verboseName: 'MyTag', + category: 'Dart', + rawUrl: '', + packageUri: '', + sourceLine: null, + parentId: '', + profileMetaData: profileMetaData, + isTag: true, + ).isTag, + isTrue, + ); + }); + test('sampleCount', () { expect(testStackFrame.inclusiveSampleCount, equals(10)); }); @@ -344,6 +367,10 @@ void main() { }); test('tooltip', () { + expect( + tagFrameA.tooltip, + equals('[Tag] TagA - 0.0 ms'), + ); expect( stackFrameA.tooltip, equals('[Native] A - 0.1 ms'), diff --git a/packages/devtools_app/test/cpu_profiler/cpu_profile_transformer_test.dart b/packages/devtools_app/test/cpu_profiler/cpu_profile_transformer_test.dart index 0b31e27564f..5ccca9ca7cd 100644 --- a/packages/devtools_app/test/cpu_profiler/cpu_profile_transformer_test.dart +++ b/packages/devtools_app/test/cpu_profiler/cpu_profile_transformer_test.dart @@ -134,6 +134,7 @@ void main() { final List bottomUpRoots = transformer.bottomUpRootsFor( topDownRoot: testStackFrame, mergeSamples: mergeCpuProfileRoots, + rootedAtTags: false, ); // Verify the original stack frame was not modified. @@ -150,5 +151,35 @@ void main() { } expect(buf.toString(), equals(bottomUpGolden)); }); + + test('bottomUpRootsFor rootedAtTags', () { + expect( + testTagRootedStackFrame.profileAsString(), + equals(testTagRootedStackFrameStringGolden), + ); + + // Note: this needs to be rooted at a root frame before transforming as + // a tree rooted at a root frame is what is provided in cpu_profile_model.dart. + final List bottomUpRoots = transformer.bottomUpRootsFor( + topDownRoot: CpuStackFrame.root(zeroProfileMetaData) + ..addChild(testTagRootedStackFrame), + mergeSamples: mergeCpuProfileRoots, + rootedAtTags: true, + ); + + // Verify the original stack frame was not modified. + expect( + testTagRootedStackFrame.profileAsString(), + equals(testTagRootedStackFrameStringGolden), + ); + + expect(bottomUpRoots.length, equals(1)); + + final buf = StringBuffer(); + for (CpuStackFrame stackFrame in bottomUpRoots) { + buf.writeln(stackFrame.profileAsString()); + } + expect(buf.toString(), equals(tagRootedBottomUpGolden)); + }); }); } diff --git a/packages/devtools_app/test/cpu_profiler/cpu_profiler_controller_test.dart b/packages/devtools_app/test/cpu_profiler/cpu_profiler_controller_test.dart index 15f628ee1ee..56445fbecd1 100644 --- a/packages/devtools_app/test/cpu_profiler/cpu_profiler_controller_test.dart +++ b/packages/devtools_app/test/cpu_profiler/cpu_profiler_controller_test.dart @@ -308,6 +308,52 @@ void main() { Frame1 - children: 1 - excl: 0 - incl: 2 Frame5 - children: 1 - excl: 1 - incl: 2 Frame6 - children: 0 - excl: 1 - incl: 1 +''', + ), + ); + + await controller.loadDataWithTag(CpuProfilerController.groupByUserTag); + expect( + controller.dataNotifier.value!.cpuProfileRoot.profileAsString(), + equals( + ''' + all - children: 3 - excl: 0 - incl: 5 + userTagA - children: 1 - excl: 0 - incl: 2 + Frame1 - children: 2 - excl: 0 - incl: 2 + Frame2 - children: 1 - excl: 0 - incl: 1 + Frame3 - children: 0 - excl: 1 - incl: 1 + Frame5 - children: 0 - excl: 1 - incl: 1 + userTagB - children: 1 - excl: 0 - incl: 1 + Frame1 - children: 1 - excl: 0 - incl: 1 + Frame2 - children: 1 - excl: 0 - incl: 1 + Frame4 - children: 0 - excl: 1 - incl: 1 + userTagC - children: 1 - excl: 0 - incl: 2 + Frame1 - children: 1 - excl: 0 - incl: 2 + Frame5 - children: 1 - excl: 1 - incl: 2 + Frame6 - children: 0 - excl: 1 - incl: 1 +''', + ), + ); + + await controller.loadDataWithTag(CpuProfilerController.groupByVmTag); + expect( + controller.dataNotifier.value!.cpuProfileRoot.profileAsString(), + equals( + ''' + all - children: 3 - excl: 0 - incl: 5 + vmTagA - children: 1 - excl: 0 - incl: 2 + Frame1 - children: 2 - excl: 0 - incl: 2 + Frame2 - children: 1 - excl: 0 - incl: 1 + Frame3 - children: 0 - excl: 1 - incl: 1 + Frame5 - children: 0 - excl: 1 - incl: 1 + vmTagB - children: 1 - excl: 0 - incl: 1 + Frame1 - children: 1 - excl: 0 - incl: 1 + Frame2 - children: 1 - excl: 0 - incl: 1 + Frame4 - children: 0 - excl: 1 - incl: 1 + vmTagC - children: 1 - excl: 0 - incl: 2 + Frame1 - children: 1 - excl: 0 - incl: 2 + Frame5 - children: 1 - excl: 1 - incl: 2 + Frame6 - children: 0 - excl: 1 - incl: 1 ''', ), ); @@ -380,6 +426,42 @@ void main() { all - children: 1 - excl: 0 - incl: 2 Frame5 - children: 1 - excl: 1 - incl: 2 Frame6 - children: 0 - excl: 1 - incl: 1 +''', + ), + ); + + await controller.loadDataWithTag(CpuProfilerController.groupByUserTag); + expect( + controller.dataNotifier.value!.cpuProfileRoot.profileAsString(), + equals( + ''' + all - children: 3 - excl: 0 - incl: 5 + userTagA - children: 2 - excl: 0 - incl: 2 + Frame2 - children: 0 - excl: 1 - incl: 1 + Frame5 - children: 0 - excl: 1 - incl: 1 + userTagB - children: 1 - excl: 0 - incl: 1 + Frame2 - children: 0 - excl: 1 - incl: 1 + userTagC - children: 1 - excl: 0 - incl: 2 + Frame5 - children: 1 - excl: 1 - incl: 2 + Frame6 - children: 0 - excl: 1 - incl: 1 +''', + ), + ); + + await controller.loadDataWithTag(CpuProfilerController.groupByVmTag); + expect( + controller.dataNotifier.value!.cpuProfileRoot.profileAsString(), + equals( + ''' + all - children: 3 - excl: 0 - incl: 5 + vmTagA - children: 2 - excl: 0 - incl: 2 + Frame2 - children: 0 - excl: 1 - incl: 1 + Frame5 - children: 0 - excl: 1 - incl: 1 + vmTagB - children: 1 - excl: 0 - incl: 1 + Frame2 - children: 0 - excl: 1 - incl: 1 + vmTagC - children: 1 - excl: 0 - incl: 2 + Frame5 - children: 1 - excl: 1 - incl: 2 + Frame6 - children: 0 - excl: 1 - incl: 1 ''', ), ); diff --git a/packages/devtools_app/test/cpu_profiler/cpu_profiler_test.dart b/packages/devtools_app/test/cpu_profiler/cpu_profiler_test.dart index 162590eec49..c547407e8b3 100644 --- a/packages/devtools_app/test/cpu_profiler/cpu_profiler_test.dart +++ b/packages/devtools_app/test/cpu_profiler/cpu_profiler_test.dart @@ -427,6 +427,7 @@ void main() { expect(find.text('Filter by tag: userTagA'), findsWidgets); expect(find.text('Filter by tag: userTagB'), findsWidgets); expect(find.text('Filter by tag: userTagC'), findsWidgets); + expect(find.text('Group by: User Tag'), findsWidgets); await tester.tap(find.text('Call Tree')); await tester.pumpAndSettle(); @@ -446,6 +447,9 @@ void main() { expect(find.richText('Frame4'), findsOneWidget); expect(find.richText('Frame5'), findsOneWidget); expect(find.richText('Frame6'), findsOneWidget); + expect(find.text('userTagA'), findsNothing); + expect(find.text('userTagB'), findsNothing); + expect(find.text('userTagC'), findsNothing); await tester.tap(find.byType(UserTagDropdown)); await tester.pumpAndSettle(); @@ -464,6 +468,9 @@ void main() { expect(find.text('Frame4'), findsNothing); expect(find.richText('Frame5'), findsOneWidget); expect(find.text('Frame6'), findsNothing); + expect(find.text('userTagA'), findsNothing); + expect(find.text('userTagB'), findsNothing); + expect(find.text('userTagC'), findsNothing); await tester.tap(find.byType(UserTagDropdown)); await tester.pumpAndSettle(); @@ -482,6 +489,9 @@ void main() { expect(find.text('Frame4'), findsNothing); expect(find.text('Frame5'), findsNothing); expect(find.text('Frame6'), findsNothing); + expect(find.text('userTagA'), findsNothing); + expect(find.text('userTagB'), findsNothing); + expect(find.text('userTagC'), findsNothing); await tester.tap(find.byType(UserTagDropdown)); await tester.pumpAndSettle(); @@ -500,6 +510,114 @@ void main() { expect(find.text('Frame4'), findsNothing); expect(find.richText('Frame5'), findsOneWidget); expect(find.richText('Frame6'), findsOneWidget); + expect(find.text('userTagA'), findsNothing); + expect(find.text('userTagB'), findsNothing); + expect(find.text('userTagC'), findsNothing); + }); + }); + + group('Group by ', () { + late ProfilerScreenController controller; + + setUp(() async { + controller = ProfilerScreenController(); + preferences.toggleVmDeveloperMode(true); + cpuProfileData = CpuProfileData.parse(cpuProfileDataWithUserTagsJson); + for (final filter in controller.cpuProfilerController.toggleFilters) { + filter.enabled.value = false; + } + final data = CpuProfilePair( + functionProfile: cpuProfileData, + // Function and code profiles have the same structure, so just use + // the function profile in place of a dedicated code profile for + // testing since we don't care about the contents as much as we + // care about testing the ability to switch between function and + // code profile views. + codeProfile: cpuProfileData, + ); + await data.process( + transformer: controller.cpuProfilerController.transformer, + processId: 'test', + ); + // Call this to force the value of `_dataByTag[userTagNone]` to be set. + controller.cpuProfilerController.loadProcessedData( + data, + storeAsUserTagNone: true, + ); + }); + + testWidgetsWithWindowSize('user tags', windowSize, (tester) async { + // We need to pump the entire `ProfilerScreenBody` widget because the + // CpuProfiler widget has `cpuProfileData` passed in from there, and + // CpuProfiler needs to be rebuilt on data updates. + await tester.pumpWidget( + wrapWithControllers( + const ProfilerScreenBody(), + profiler: controller, + ), + ); + + await tester.tap(find.text('Call Tree')); + await tester.pumpAndSettle(); + expect(find.byType(CpuCallTreeTable), findsOneWidget); + await tester.tap(find.byType(UserTagDropdown)); + await tester.pumpAndSettle(); + await tester.tap(find.text('Group by: User Tag').last); + await tester.pumpAndSettle(); + await tester.tap(find.text('Expand All')); + await tester.pumpAndSettle(); + + expect(find.richText('Frame1'), findsNWidgets(3)); + expect(find.richText('Frame2'), findsNWidgets(2)); + expect(find.richText('Frame3'), findsNWidgets(1)); + expect(find.richText('Frame4'), findsNWidgets(1)); + expect(find.richText('Frame5'), findsNWidgets(2)); + expect(find.richText('Frame6'), findsNWidgets(1)); + expect(find.richText('userTagA'), findsOneWidget); + expect(find.richText('userTagB'), findsOneWidget); + expect(find.richText('userTagC'), findsOneWidget); + }); + + testWidgetsWithWindowSize('VM tags', windowSize, (tester) async { + // We need to pump the entire `ProfilerScreenBody` widget because the + // CpuProfiler widget has `cpuProfileData` passed in from there, and + // CpuProfiler needs to be rebuilt on data updates. + await tester.pumpWidget( + wrapWithControllers( + const ProfilerScreenBody(), + profiler: controller, + ), + ); + + await tester.tap(find.text('Call Tree')); + await tester.pumpAndSettle(); + expect(find.byType(CpuCallTreeTable), findsOneWidget); + await tester.tap(find.byType(UserTagDropdown)); + await tester.pumpAndSettle(); + await tester.tap(find.text('Group by: VM Tag').last); + await tester.pumpAndSettle(); + await tester.tap(find.text('Expand All')); + await tester.pumpAndSettle(); + + expect(find.richText('Frame1'), findsNWidgets(3)); + expect(find.richText('Frame2'), findsNWidgets(2)); + expect(find.richText('Frame3'), findsNWidgets(1)); + expect(find.richText('Frame4'), findsNWidgets(1)); + expect(find.richText('Frame5'), findsNWidgets(2)); + expect(find.richText('Frame6'), findsNWidgets(1)); + expect(find.richText('vmTagA'), findsOneWidget); + expect(find.richText('vmTagB'), findsOneWidget); + expect(find.richText('vmTagC'), findsOneWidget); + + // Check that disabling VM developer mode when grouping by VM tag + // automatically resets the view to 'Filter by tag: none'. + preferences.toggleVmDeveloperMode(false); + await tester.pumpAndSettle(); + expect(find.byType(CpuCallTreeTable), findsOneWidget); + expect(find.text('Filter by tag: none'), findsOneWidget); + await tester.tap(find.byType(UserTagDropdown)); + await tester.pumpAndSettle(); + expect(find.text('Group by: VM Tag'), findsNothing); }); }); }); diff --git a/packages/devtools_app/test/test_data/cpu_profile.dart b/packages/devtools_app/test/test_data/cpu_profile.dart index 850fc9b5ac4..bfd8bf7d640 100644 --- a/packages/devtools_app/test/test_data/cpu_profile.dart +++ b/packages/devtools_app/test/test_data/cpu_profile.dart @@ -88,6 +88,7 @@ final Map cpuProfileDataWithUserTagsJson = { 'cat': 'Dart', 'args': { 'userTag': 'userTagA', + 'vmTag': 'vmTagA', }, 'sf': '140357727781376-3' }, @@ -100,6 +101,7 @@ final Map cpuProfileDataWithUserTagsJson = { 'cat': 'Dart', 'args': { 'userTag': 'userTagB', + 'vmTag': 'vmTagB', }, 'sf': '140357727781376-4' }, @@ -112,6 +114,7 @@ final Map cpuProfileDataWithUserTagsJson = { 'cat': 'Dart', 'args': { 'userTag': 'userTagA', + 'vmTag': 'vmTagA', }, 'sf': '140357727781376-5' }, @@ -124,6 +127,7 @@ final Map cpuProfileDataWithUserTagsJson = { 'cat': 'Dart', 'args': { 'userTag': 'userTagC', + 'vmTag': 'vmTagC', }, 'sf': '140357727781376-5' }, @@ -136,6 +140,7 @@ final Map cpuProfileDataWithUserTagsJson = { 'cat': 'Dart', 'args': { 'userTag': 'userTagC', + 'vmTag': 'vmTagC', }, 'sf': '140357727781376-6' }, @@ -488,56 +493,104 @@ final Map goldenCpuSamplesJson = { 'implicit': false, 'abstract': false } - } + }, + { + 'kind': 'Tag', + 'inclusiveTicks': 0, + 'exclusiveTicks': 0, + 'resolvedUrl': '', + 'function': { + 'type': '@Function', + 'id': '', + 'name': 'Foo', + 'owner': null, + 'static': false, + 'const': false, + 'implicit': false, + 'abstract': false + } + }, + { + 'kind': 'Tag', + 'inclusiveTicks': 0, + 'exclusiveTicks': 0, + 'resolvedUrl': '', + 'function': { + 'type': '@Function', + 'id': '', + 'name': 'Default', + 'owner': null, + 'static': false, + 'const': false, + 'implicit': false, + 'abstract': false + } + }, ], 'samples': [ { 'tid': 42247, 'timestamp': 47377796685, 'stack': [4, 3, 2, 1, 0], - 'truncated': true + 'truncated': true, + 'userTag': 'Foo', + 'vmTag': 'Dart', }, { 'tid': 42247, 'timestamp': 47377797975, 'stack': [7, 6, 5, 2, 1, 0], - 'truncated': true + 'truncated': true, + 'userTag': 'Foo', + 'vmTag': 'Dart', }, { 'tid': 42247, 'timestamp': 47377799063, 'stack': [10, 9, 8], - 'truncated': true + 'truncated': true, + 'userTag': 'Foo', + 'vmTag': 'Dart', }, { 'tid': 42247, 'timestamp': 47377800363, 'stack': [13, 12, 11, 8], - 'truncated': true + 'truncated': true, + 'userTag': 'Default', + 'vmTag': 'VM', }, { 'tid': 42247, 'timestamp': 47377800463, 'stack': [13, 12, 11, 8], - 'truncated': true + 'truncated': true, + 'userTag': 'Default', + 'vmTag': 'VM', }, { 'tid': 42247, 'timestamp': 47377800563, 'stack': [13, 12, 11, 8], - 'truncated': true + 'truncated': true, + 'userTag': 'Default', + 'vmTag': 'VM', }, { 'tid': 42247, 'timestamp': 47377800663, 'stack': [14, 13, 12, 11, 8], - 'truncated': true + 'truncated': true, + 'userTag': 'Default', + 'vmTag': 'VM', }, { 'tid': 42247, 'timestamp': 47377800763, 'stack': [16, 15, 13, 12, 11, 8], - 'truncated': true + 'truncated': true, + 'userTag': 'Default', + 'vmTag': 'VM', } ] }; @@ -859,7 +912,10 @@ final filteredCpuSampleTraceEvents = [ 'tid': 42247, 'ts': 47377796685, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Foo', + 'vmTag': 'Dart', + }, 'sf': '140357727781376-5' }, { @@ -869,7 +925,10 @@ final filteredCpuSampleTraceEvents = [ 'tid': 42247, 'ts': 47377797975, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Foo', + 'vmTag': 'Dart', + }, 'sf': '140357727781376-8' }, { @@ -879,7 +938,10 @@ final filteredCpuSampleTraceEvents = [ 'tid': 42247, 'ts': 47377799063, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Foo', + 'vmTag': 'Dart', + }, 'sf': '140357727781376-11' }, { @@ -889,7 +951,10 @@ final filteredCpuSampleTraceEvents = [ 'tid': 42247, 'ts': 47377800363, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Default', + 'vmTag': 'VM', + }, 'sf': '140357727781376-13' }, { @@ -899,7 +964,10 @@ final filteredCpuSampleTraceEvents = [ 'tid': 42247, 'ts': 47377800463, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Default', + 'vmTag': 'VM', + }, 'sf': '140357727781376-13' }, { @@ -909,7 +977,10 @@ final filteredCpuSampleTraceEvents = [ 'tid': 42247, 'ts': 47377800563, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Default', + 'vmTag': 'VM', + }, 'sf': '140357727781376-13' }, { @@ -919,7 +990,10 @@ final filteredCpuSampleTraceEvents = [ 'tid': 42247, 'ts': 47377800663, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Default', + 'vmTag': 'VM', + }, 'sf': '140357727781376-15' }, { @@ -929,7 +1003,10 @@ final filteredCpuSampleTraceEvents = [ 'tid': 42247, 'ts': 47377800763, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Default', + 'vmTag': 'VM', + }, 'sf': '140357727781376-17' } ]; @@ -944,7 +1021,10 @@ final List> goldenCpuProfileTraceEvents = 'tid': 42247, 'ts': 47377800363, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Default', + 'vmTag': 'VM', + }, 'sf': '140357727781376-14' }, { @@ -954,7 +1034,10 @@ final List> goldenCpuProfileTraceEvents = 'tid': 42247, 'ts': 47377800463, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Default', + 'vmTag': 'VM', + }, 'sf': '140357727781376-14' }, { @@ -964,7 +1047,10 @@ final List> goldenCpuProfileTraceEvents = 'tid': 42247, 'ts': 47377800563, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Default', + 'vmTag': 'VM', + }, 'sf': '140357727781376-14' }, { @@ -974,7 +1060,10 @@ final List> goldenCpuProfileTraceEvents = 'tid': 42247, 'ts': 47377800663, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Default', + 'vmTag': 'VM', + }, 'sf': '140357727781376-15' }, { @@ -984,7 +1073,10 @@ final List> goldenCpuProfileTraceEvents = 'tid': 42247, 'ts': 47377800763, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Default', + 'vmTag': 'VM', + }, 'sf': '140357727781376-17' } ]); @@ -997,7 +1089,10 @@ final subProfileTraceEvents = [ 'tid': 42247, 'ts': 47377796685, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Foo', + 'vmTag': 'Dart', + }, 'sf': '140357727781376-5' }, { @@ -1007,7 +1102,10 @@ final subProfileTraceEvents = [ 'tid': 42247, 'ts': 47377797975, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Foo', + 'vmTag': 'Dart', + }, 'sf': '140357727781376-8' }, { @@ -1017,7 +1115,10 @@ final subProfileTraceEvents = [ 'tid': 42247, 'ts': 47377799063, 'cat': 'Dart', - 'args': {}, + 'args': { + 'userTag': 'Foo', + 'vmTag': 'Dart', + }, 'sf': '140357727781376-11' }, ]; @@ -1104,6 +1205,19 @@ final CpuProfileMetaData profileMetaData = CpuProfileMetaData( ..end = const Duration(microseconds: 100), ); +final CpuStackFrame tagFrameA = CpuStackFrame( + id: 'id_tag_0', + name: 'TagA', + verboseName: 'TagA', + category: 'Dart', + rawUrl: '', + packageUri: '', + sourceLine: null, + parentId: CpuProfileData.rootId, + profileMetaData: profileMetaData, + isTag: true, +)..exclusiveSampleCount = 0; + final CpuStackFrame stackFrameA = CpuStackFrame( id: 'id_0', name: 'A', @@ -1114,6 +1228,7 @@ final CpuStackFrame stackFrameA = CpuStackFrame( sourceLine: null, parentId: CpuProfileData.rootId, profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 0; final CpuStackFrame stackFrameB = CpuStackFrame( @@ -1126,6 +1241,7 @@ final CpuStackFrame stackFrameB = CpuStackFrame( sourceLine: 2222, parentId: 'id_0', profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 0; final CpuStackFrame stackFrameC = CpuStackFrame( @@ -1139,7 +1255,9 @@ final CpuStackFrame stackFrameC = CpuStackFrame( sourceLine: 3333, parentId: 'id_1', profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 2; + final CpuStackFrame stackFrameD = CpuStackFrame( id: 'id_3', name: 'D', @@ -1150,6 +1268,7 @@ final CpuStackFrame stackFrameD = CpuStackFrame( sourceLine: null, parentId: 'id_1', profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 2; final CpuStackFrame stackFrameE = CpuStackFrame( @@ -1162,7 +1281,9 @@ final CpuStackFrame stackFrameE = CpuStackFrame( sourceLine: null, parentId: 'id_3', profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 1; + final CpuStackFrame stackFrameF = CpuStackFrame( id: 'id_5', name: 'F', @@ -1173,6 +1294,7 @@ final CpuStackFrame stackFrameF = CpuStackFrame( sourceLine: null, parentId: 'id_4', profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 0; final CpuStackFrame stackFrameF2 = CpuStackFrame( @@ -1185,6 +1307,7 @@ final CpuStackFrame stackFrameF2 = CpuStackFrame( sourceLine: null, parentId: 'id_3', profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 3; final CpuStackFrame stackFrameC2 = CpuStackFrame( @@ -1198,6 +1321,7 @@ final CpuStackFrame stackFrameC2 = CpuStackFrame( sourceLine: 3333, parentId: 'id_5', profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 1; final CpuStackFrame stackFrameC3 = CpuStackFrame( @@ -1211,6 +1335,7 @@ final CpuStackFrame stackFrameC3 = CpuStackFrame( sourceLine: 3333, parentId: 'id_6', profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 1; final CpuStackFrame stackFrameC4 = CpuStackFrame( @@ -1225,6 +1350,7 @@ final CpuStackFrame stackFrameC4 = CpuStackFrame( sourceLine: 47, parentId: 'id_6', profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 1; final CpuStackFrame stackFrameG = CpuStackFrame( @@ -1239,6 +1365,7 @@ final CpuStackFrame stackFrameG = CpuStackFrame( sourceLine: null, parentId: 'id_0', profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 1; final CpuStackFrame testStackFrame = stackFrameA @@ -1252,6 +1379,11 @@ final CpuStackFrame testStackFrame = stackFrameA ), ); +final CpuStackFrame testTagRootedStackFrame = tagFrameA + ..addChild( + testStackFrame, + ); + const String testStackFrameStringGolden = ''' A - children: 1 - excl: 0 - incl: 10 B - children: 2 - excl: 0 - incl: 10 @@ -1327,6 +1459,46 @@ const String bottomUpGolden = ''' '''; +const String testTagRootedStackFrameStringGolden = ''' + TagA - children: 1 - excl: 0 - incl: 10 + A - children: 1 - excl: 0 - incl: 10 + B - children: 2 - excl: 0 - incl: 10 + C - children: 0 - excl: 2 - incl: 2 + D - children: 2 - excl: 2 - incl: 8 + E - children: 1 - excl: 1 - incl: 2 + F - children: 1 - excl: 0 - incl: 1 + C - children: 0 - excl: 1 - incl: 1 + F - children: 1 - excl: 3 - incl: 4 + C - children: 0 - excl: 1 - incl: 1 +'''; + +const String tagRootedBottomUpGolden = ''' + TagA - children: 4 - excl: 0 - incl: 10 + C - children: 2 - excl: 4 - incl: 4 + B - children: 1 - excl: 2 - incl: 2 + A - children: 0 - excl: 2 - incl: 2 + F - children: 2 - excl: 2 - incl: 2 + E - children: 1 - excl: 1 - incl: 1 + D - children: 1 - excl: 1 - incl: 1 + B - children: 1 - excl: 1 - incl: 1 + A - children: 0 - excl: 1 - incl: 1 + D - children: 1 - excl: 1 - incl: 1 + B - children: 1 - excl: 1 - incl: 1 + A - children: 0 - excl: 1 - incl: 1 + D - children: 1 - excl: 2 - incl: 2 + B - children: 1 - excl: 2 - incl: 2 + A - children: 0 - excl: 2 - incl: 2 + E - children: 1 - excl: 1 - incl: 1 + D - children: 1 - excl: 1 - incl: 1 + B - children: 1 - excl: 1 - incl: 1 + A - children: 0 - excl: 1 - incl: 1 + F - children: 1 - excl: 3 - incl: 3 + D - children: 1 - excl: 3 - incl: 3 + B - children: 1 - excl: 3 - incl: 3 + A - children: 0 - excl: 3 - incl: 3 + +'''; + final CpuProfileMetaData zeroProfileMetaData = CpuProfileMetaData( sampleCount: 0, samplePeriod: 50, @@ -1346,6 +1518,7 @@ final CpuStackFrame zeroStackFrame = CpuStackFrame( sourceLine: null, parentId: CpuProfileData.rootId, profileMetaData: zeroProfileMetaData, + isTag: false, )..exclusiveSampleCount = 0; final flutterEngineStackFrame = CpuStackFrame( @@ -1358,4 +1531,5 @@ final flutterEngineStackFrame = CpuStackFrame( sourceLine: null, parentId: CpuProfileData.rootId, profileMetaData: profileMetaData, + isTag: false, )..exclusiveSampleCount = 1; From a85844c05809d02e2733e8b41a5c7614830927ac Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Fri, 4 Nov 2022 15:47:55 -0400 Subject: [PATCH 2/2] Address comments --- .../profiler/cpu_profile_controller.dart | 2 +- .../screens/profiler/cpu_profile_model.dart | 49 ++++++++++--------- .../profiler/cpu_profile_transformer.dart | 12 +++-- 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_controller.dart b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_controller.dart index 0092cc0bbcd..03b02047b2f 100644 --- a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_controller.dart +++ b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_controller.dart @@ -31,7 +31,7 @@ enum CpuProfilerViewType { enum CpuProfilerTagType { user, - vm; + vm, } class CpuProfilerController diff --git a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_model.dart b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_model.dart index 419159acd21..7b05bfecb7e 100644 --- a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_model.dart +++ b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_model.dart @@ -80,17 +80,18 @@ class CpuProfilePair { factory CpuProfilePair.withTagRoots( CpuProfilePair original, - CpuProfilerTagType tag, + CpuProfilerTagType tagType, ) { final function = CpuProfileData.withTagRoots( original.functionProfile, - tag, + tagType, ); + final codeProfile = original.codeProfile; CpuProfileData? code; - if (original.codeProfile != null) { + if (codeProfile != null) { code = CpuProfileData.withTagRoots( - original.codeProfile!, - tag, + codeProfile, + tagType, ); } return CpuProfilePair(functionProfile: function, codeProfile: code); @@ -266,14 +267,14 @@ class CpuProfileData { } /// Generate a cpu profile from [originalData] where the profile is broken - /// down for each tag of the given [type]. + /// down for each tag of the given [tagType]. /// /// [originalData] does not need to be [processed] to run this operation. factory CpuProfileData.withTagRoots( CpuProfileData originalData, - CpuProfilerTagType type, + CpuProfilerTagType tagType, ) { - final useUserTags = type == CpuProfilerTagType.user; + final useUserTags = tagType == CpuProfilerTagType.user; final tags = { for (final sample in originalData.cpuSamples) useUserTags ? sample.userTag! : sample.vmTag!, @@ -281,7 +282,7 @@ class CpuProfileData { final tagProfiles = { for (final tag in tags) - tag: CpuProfileData._fromTag(originalData, tag, type), + tag: CpuProfileData._fromTag(originalData, tag, tagType), }; final metaData = originalData.profileMetaData.copyWith(); @@ -320,17 +321,12 @@ class CpuProfileData { rootId: tagId, }; - String? getId(String? id) { - if (id == null) { - return null; - } - return idMapping.putIfAbsent(id, () => '$isolateId-${nextId++}'); - } - - tagProfile.stackFrames.forEach((k, v) => getId(k)); + tagProfile.stackFrames.forEach((k, v) { + idMapping.putIfAbsent(k, () => '$isolateId-${nextId++}'); + }); for (final sample in tagProfile.cpuSamples) { - String? updatedId = getId(sample.leafId); + String? updatedId = idMapping[sample.leafId]; samples.add( CpuSampleEvent( leafId: updatedId!, @@ -341,7 +337,7 @@ class CpuProfileData { ); var currentStackFrame = tagProfile.stackFrames[sample.leafId]; while (currentStackFrame != null) { - final parentId = getId(currentStackFrame.parentId); + final parentId = idMapping[currentStackFrame.parentId]; stackFrames[updatedId!] = currentStackFrame.shallowCopy( id: updatedId, copySampleCounts: false, @@ -713,9 +709,10 @@ class CpuProfileData { final CpuProfileMetaData profileMetaData; - /// `true` if the CpuProfileData has tag-based roots. This value is used - /// during the bottom-up transformation to ensure that the tag-based roots - /// are kept at the root of the resulting bottom-up tree. + /// `true` if the CpuProfileData has tag-based roots. + /// + /// This value is used during the bottom-up transformation to ensure that the + /// tag-based roots are kept at the root of the resulting bottom-up tree. final bool rootedAtTags; /// Marks whether this data has already been processed. @@ -771,8 +768,7 @@ class CpuProfileData { tags.add(tag); } } - _vmTags = tags; - return _vmTags!; + return _vmTags = tags; } Iterable? _userTags; @@ -956,6 +952,11 @@ class CpuStackFrame extends TreeNode @override String get displayName => name; + /// Set to `true` if this stack frame is a synthetic frame representing a + /// user or VM tag. + /// + /// These synthetic frames are inserted at the root of the profile when + /// samples are being grouped by tag. final bool isTag; bool get isNative => _isNative ??= id != CpuProfileData.rootId && diff --git a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_transformer.dart b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_transformer.dart index 0fb77b4d8c7..d06b8fce278 100644 --- a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_transformer.dart +++ b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_transformer.dart @@ -72,14 +72,16 @@ class CpuProfileTransformer { if (cpuProfileData.rootedAtTags) { // Check to see if there are any empty tag roots as a result of filtering // and remove them. - final nodesToRemove = []; - for (int i = 0; i < cpuProfileData.cpuProfileRoot.children.length; ++i) { + final nodeIndiciesToRemove = []; + for (int i = cpuProfileData.cpuProfileRoot.children.length - 1; + i >= 0; + --i) { final root = cpuProfileData.cpuProfileRoot.children[i]; if (root.isTag && root.children.isEmpty) { - nodesToRemove.add(i); + nodeIndiciesToRemove.add(i); } } - nodesToRemove.reversed.forEach( + nodeIndiciesToRemove.forEach( cpuProfileData.cpuProfileRoot.removeChildAtIndex, ); } @@ -127,7 +129,7 @@ class CpuProfileTransformer { CpuProfileData cpuProfileData, ) { // [stackFrame] is the root of a new cpu sample. Add it as a child of - // [cpuProfile]. + // [cpuProfileRoot]. if (parent == null) { cpuProfileData.cpuProfileRoot.addChild(stackFrame); } else {