Support closures and records in outbound live references. (#5333)
diff --git a/packages/devtools_app/lib/src/shared/diagnostics/dart_object_node.dart b/packages/devtools_app/lib/src/shared/diagnostics/dart_object_node.dart index 333e996..50955c6 100644 --- a/packages/devtools_app/lib/src/shared/diagnostics/dart_object_node.dart +++ b/packages/devtools_app/lib/src/shared/diagnostics/dart_object_node.dart
@@ -11,8 +11,10 @@ import '../memory/adapted_heap_data.dart'; import '../primitives/trees.dart'; import '../primitives/utils.dart'; +import '../vm_utils.dart'; import 'diagnostics_node.dart'; import 'generic_instance_reference.dart'; +import 'helpers.dart'; import 'inspector_service.dart'; // TODO(jacobr): gracefully handle cases where the isolate has closed and @@ -240,8 +242,7 @@ final value = this.value; if (value is InstanceRef) { if (value.kind != null && - (value.kind!.endsWith('List') || - value.kind == InstanceKind.kList || + (isList(value) || value.kind == InstanceKind.kMap || value.kind == InstanceKind.kRecord || isSet)) { @@ -273,23 +274,31 @@ @override bool get isExpandable { + final theRef = ref; + final instanceRef = theRef?.instanceRef; + if (theRef is ObjectReferences) { + if (instanceRef?.length == 0) return false; + return theRef.refNodeType.isRoot || + children.isNotEmpty || + childCount > 0 || + !isPrimitiveInstanceKind(instanceRef?.kind); + } if (treeInitializeComplete || children.isNotEmpty || childCount > 0) { return children.isNotEmpty || childCount > 0; } - if (ref?.heapSelection != null) return true; - final diagnostic = ref?.diagnostic; + final diagnostic = theRef?.diagnostic; if (diagnostic != null && ((diagnostic.inlineProperties.isNotEmpty) || diagnostic.hasChildren)) return true; + // TODO(jacobr): do something smarter to avoid expandable variable flicker. - final instanceRef = ref?.instanceRef; if (instanceRef != null) { if (instanceRef.kind == InstanceKind.kStackTrace) { return true; } return instanceRef.valueAsString == null; } - final value = ref?.value; + final value = theRef?.value; return (value is! String?) && (value is! num?) && (value is! bool?); } @@ -332,10 +341,9 @@ } } // List, Map, Uint8List, Uint16List, etc... - if (kind != null && kind == InstanceKind.kList || + if (isList(value) || kind == InstanceKind.kMap || - kind == InstanceKind.kSet || - kind!.endsWith('List')) { + kind == InstanceKind.kSet) { // TODO(elliette): Determine the signature from type parameters, see: // https://api.flutter.dev/flutter/vm_service/ClassRef/typeParameters.html // DWDS provides us with a readable format including type parameters in
diff --git a/packages/devtools_app/lib/src/shared/diagnostics/generic_instance_reference.dart b/packages/devtools_app/lib/src/shared/diagnostics/generic_instance_reference.dart index b63c69d..b4859d9 100644 --- a/packages/devtools_app/lib/src/shared/diagnostics/generic_instance_reference.dart +++ b/packages/devtools_app/lib/src/shared/diagnostics/generic_instance_reference.dart
@@ -96,4 +96,6 @@ const RefNodeType([this.direction]); final RefDirection? direction; + + bool get isRoot => {refRoot, staticRefRoot, liveRefRoot}.contains(this); }
diff --git a/packages/devtools_app/lib/src/shared/diagnostics/helpers.dart b/packages/devtools_app/lib/src/shared/diagnostics/helpers.dart index 47c527d..3810983 100644 --- a/packages/devtools_app/lib/src/shared/diagnostics/helpers.dart +++ b/packages/devtools_app/lib/src/shared/diagnostics/helpers.dart
@@ -10,7 +10,7 @@ import 'dart_object_node.dart'; /// Gets object by object reference using offset and childCount from [variable] -/// to get list items. +/// for list items. Future<Object?> getObject({ required IsolateRef? isolateRef, required ObjRef value, @@ -23,3 +23,10 @@ count: variable?.childCount, ); } + +bool isList(ObjRef? ref) { + if (ref is! InstanceRef) return false; + final kind = ref.kind; + if (kind == null) return false; + return kind.endsWith('List') || kind == InstanceKind.kList; +}
diff --git a/packages/devtools_app/lib/src/shared/diagnostics/primitives/record_fields.dart b/packages/devtools_app/lib/src/shared/diagnostics/primitives/record_fields.dart new file mode 100644 index 0000000..8164b4e --- /dev/null +++ b/packages/devtools_app/lib/src/shared/diagnostics/primitives/record_fields.dart
@@ -0,0 +1,36 @@ +// Copyright 2023 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:vm_service/vm_service.dart'; + +class RecordFields { + RecordFields(List<BoundField>? fields) { + positional = <BoundField>[]; + named = <BoundField>[]; + for (final field in fields ?? []) { + if (_isPositionalField(field)) { + positional.add(field); + } else { + named.add(field); + } + } + + _sortPositionalFields(positional); + } + + late final List<BoundField> positional; + late final List<BoundField> named; + + static bool _isPositionalField(BoundField field) => field.name is int; + + // Sorts positional fields in ascending order: + static void _sortPositionalFields(List<BoundField> fields) { + fields.sort((field1, field2) { + assert(field1.name is int && field2.name is int); + final name1 = field1.name as int; + final name2 = field2.name as int; + return name1.compareTo(name2); + }); + } +}
diff --git a/packages/devtools_app/lib/src/shared/diagnostics/references.dart b/packages/devtools_app/lib/src/shared/diagnostics/references.dart index 49c2776..bb461b2 100644 --- a/packages/devtools_app/lib/src/shared/diagnostics/references.dart +++ b/packages/devtools_app/lib/src/shared/diagnostics/references.dart
@@ -15,6 +15,7 @@ import 'dart_object_node.dart'; import 'generic_instance_reference.dart'; import 'helpers.dart'; +import 'primitives/record_fields.dart'; void addReferencesRoot(DartObjectNode variable, GenericInstanceRef ref) { variable.addChild( @@ -197,7 +198,6 @@ final instance = await getObject( isolateRef: isolateRef, value: ref.instanceRef!, - variable: variable, ); if (instance is Instance) { @@ -238,10 +238,22 @@ ); break; case InstanceKind.kRecord: - // TODO: add references + variable.addAllChildren( + _createLiveOutboundReferencesForRecord( + value, + isolateRef, + heapSelection.withoutObject(), + ), + ); break; case InstanceKind.kClosure: - // TODO: add references + variable.addAllChildren( + await _createLiveOutboundReferencesForClosure( + value, + isolateRef, + heapSelection.withoutObject(), + ), + ); break; default: break; @@ -249,7 +261,7 @@ if (value.fields != null && value.kind != InstanceKind.kRecord) { variable.addAllChildren( - _createLiveReferencesForFields( + _createLiveOutboundReferencesForFields( value, isolateRef, heapSelection.withoutObject(), @@ -266,13 +278,18 @@ HeapObjectSelection heapSelection, { String namePrefix = '', }) { - if (instance is! InstanceRef) return; - final classRef = instance.classRef!; - if (HeapClassName.fromClassRef(classRef).isNull) return; + if (instance is! InstanceRef && instance is! ContextRef) return; + + var name = ''; + if (instance is InstanceRef) { + final classRef = instance.classRef!; + if (HeapClassName.fromClassRef(classRef).isNull) return; + name = classRef.name ?? ''; + } variables.add( DartObjectNode.references( - '$namePrefix${classRef.name}', + '$namePrefix$name', ObjectReferences( refNodeType: refNodeType, isolateRef: isolateRef, @@ -338,7 +355,80 @@ return variables; } -List<DartObjectNode> _createLiveReferencesForFields( +Future<List<DartObjectNode>> _createLiveOutboundReferencesForClosure( + Instance instance, + IsolateRef isolateRef, + HeapObjectSelection heapSelection, +) async { + final variables = <DartObjectNode>[]; + final contextRef = instance.closureContext; + if (contextRef == null) return []; + + final context = await getObject(isolateRef: isolateRef, value: contextRef); + if (context is! Context) return []; + + if (context.parent != null) { + _addLiveReferenceToNode( + variables, + isolateRef, + context.parent, + RefNodeType.liveOutRefs, + heapSelection.withoutObject(), + namePrefix: 'parent', + ); + } + + final contextVariables = context.variables ?? []; + for (int i = 0; i < contextVariables.length; i++) { + _addLiveReferenceToNode( + variables, + isolateRef, + contextVariables[i].value, + RefNodeType.liveOutRefs, + heapSelection.withoutObject(), + namePrefix: '[$i]', + ); + } + + return variables; +} + +List<DartObjectNode> _createLiveOutboundReferencesForRecord( + Instance instance, + IsolateRef isolateRef, + HeapObjectSelection heapSelection, +) { + final variables = <DartObjectNode>[]; + final fields = RecordFields(instance.fields); + + // Always show positional fields before named fields: + for (var i = 0; i < fields.positional.length; i++) { + final field = fields.positional[i]; + _addLiveReferenceToNode( + variables, + isolateRef, + field.value, + RefNodeType.liveOutRefs, + heapSelection.withoutObject(), + namePrefix: '[$i]', + ); + } + + for (final field in fields.named) { + _addLiveReferenceToNode( + variables, + isolateRef, + field.value, + RefNodeType.liveOutRefs, + heapSelection.withoutObject(), + namePrefix: '${field.name}:', + ); + } + + return variables; +} + +List<DartObjectNode> _createLiveOutboundReferencesForFields( Instance instance, IsolateRef isolateRef, HeapObjectSelection heapSelection,
diff --git a/packages/devtools_app/lib/src/shared/diagnostics/variable_factory.dart b/packages/devtools_app/lib/src/shared/diagnostics/variable_factory.dart index 14c5d4d..2d92d53 100644 --- a/packages/devtools_app/lib/src/shared/diagnostics/variable_factory.dart +++ b/packages/devtools_app/lib/src/shared/diagnostics/variable_factory.dart
@@ -15,6 +15,7 @@ import 'dart_object_node.dart'; import 'diagnostics_node.dart'; import 'inspector_service.dart'; +import 'primitives/record_fields.dart'; List<DartObjectNode> createVariablesForStackTrace( Instance stackTrace, @@ -331,7 +332,6 @@ Instance instance, IsolateRef? isolateRef, ) { - //TODO(polina-c): handle asReferences final variables = <DartObjectNode>[]; final associations = instance.associations ?? []; @@ -341,7 +341,7 @@ // representation. final hasPrimitiveKey = associations.fold<bool>( false, - (p, e) => p || isPrimativeInstanceKind(e.key.kind), + (p, e) => p || isPrimitiveInstanceKind(e.key.kind), ); for (var i = 0; i < associations.length; i++) { final association = associations[i]; @@ -520,28 +520,19 @@ Instance instance, IsolateRef? isolateRef, ) { - final positionalFields = <BoundField>[]; - final namedFields = <BoundField>[]; - for (final field in instance.fields ?? []) { - if (_isPositionalField(field)) { - positionalFields.add(field); - } else { - namedFields.add(field); - } - } - // Sort positional fields in ascending order: - _sortPositionalFields(positionalFields); + final fields = RecordFields(instance.fields); + return [ // Always show positional fields before named fields: - for (var i = 0; i < positionalFields.length; i++) + for (var i = 0; i < fields.positional.length; i++) DartObjectNode.fromValue( // Positional fields are designated by their getter syntax, eg $1, $2, // $3, etc: name: '\$${i + 1}', - value: positionalFields[i].value, + value: fields.positional[i].value, isolateRef: isolateRef, ), - for (final field in namedFields) + for (final field in fields.named) DartObjectNode.fromValue( name: field.name, value: field.value, @@ -550,17 +541,6 @@ ]; } -bool _isPositionalField(BoundField field) => field.name is int; - -void _sortPositionalFields(List<BoundField> fields) { - fields.sort((field1, field2) { - assert(field1.name is int && field2.name is int); - final name1 = field1.name as int; - final name2 = field2.name as int; - return name1.compareTo(name2); - }); -} - List<DartObjectNode> createVariablesForFields( Instance instance, IsolateRef? isolateRef, {
diff --git a/packages/devtools_app/lib/src/shared/vm_utils.dart b/packages/devtools_app/lib/src/shared/vm_utils.dart index 0e47694..d0173ac 100644 --- a/packages/devtools_app/lib/src/shared/vm_utils.dart +++ b/packages/devtools_app/lib/src/shared/vm_utils.dart
@@ -8,7 +8,7 @@ import 'globals.dart'; import 'memory/class_name.dart'; -bool isPrimativeInstanceKind(String? kind) { +bool isPrimitiveInstanceKind(String? kind) { return kind == InstanceKind.kBool || kind == InstanceKind.kDouble || kind == InstanceKind.kInt ||
diff --git a/packages/devtools_app/macos/Runner.xcodeproj/project.pbxproj b/packages/devtools_app/macos/Runner.xcodeproj/project.pbxproj index 5be5926..9d5c39f 100644 --- a/packages/devtools_app/macos/Runner.xcodeproj/project.pbxproj +++ b/packages/devtools_app/macos/Runner.xcodeproj/project.pbxproj
@@ -28,6 +28,7 @@ 33CC10F62044A3C60003C045 /* MainMenu.xib in Resources */ = {isa = PBXBuildFile; fileRef = 33CC10F42044A3C60003C045 /* MainMenu.xib */; }; 33CC11132044BFA00003C045 /* MainFlutterWindow.swift in Sources */ = {isa = PBXBuildFile; fileRef = 33CC11122044BFA00003C045 /* MainFlutterWindow.swift */; }; E678665441E5C0F7F629BAD5 /* Pods_Runner.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5062035DDDD18FB35E98D5B6 /* Pods_Runner.framework */; }; + F91BFA4332AA28C192B7B82A /* Pods_RunnerTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 649BD5F421DBAE62CB45DB38 /* Pods_RunnerTests.framework */; }; /* End PBXBuildFile section */ /* Begin PBXContainerItemProxy section */ @@ -79,10 +80,14 @@ 33E51914231749380026EE4D /* Release.entitlements */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.plist.entitlements; path = Release.entitlements; sourceTree = "<group>"; }; 33E5194F232828860026EE4D /* AppInfo.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = AppInfo.xcconfig; sourceTree = "<group>"; }; 5062035DDDD18FB35E98D5B6 /* Pods_Runner.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Runner.framework; sourceTree = BUILT_PRODUCTS_DIR; }; + 649BD5F421DBAE62CB45DB38 /* Pods_RunnerTests.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_RunnerTests.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 68C587FFA5A0B8F46A0C5150 /* Pods-Runner.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Runner.release.xcconfig"; path = "Target Support Files/Pods-Runner/Pods-Runner.release.xcconfig"; sourceTree = "<group>"; }; 7AFA3C8E1D35360C0083082E /* Release.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Release.xcconfig; sourceTree = "<group>"; }; 9740EEB21CF90195004384FC /* Debug.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = Debug.xcconfig; sourceTree = "<group>"; }; A7CE48BF63861DD9F3A9FA2F /* Pods-Runner.profile.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Runner.profile.xcconfig"; path = "Target Support Files/Pods-Runner/Pods-Runner.profile.xcconfig"; sourceTree = "<group>"; }; + BC6DAC699B0C200C374E0DE1 /* Pods-RunnerTests.profile.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-RunnerTests.profile.xcconfig"; path = "Target Support Files/Pods-RunnerTests/Pods-RunnerTests.profile.xcconfig"; sourceTree = "<group>"; }; + D55F9235E34A37D8DC0CF6B1 /* Pods-RunnerTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-RunnerTests.debug.xcconfig"; path = "Target Support Files/Pods-RunnerTests/Pods-RunnerTests.debug.xcconfig"; sourceTree = "<group>"; }; + F7EDBE938059E9DA7BF7376A /* Pods-RunnerTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-RunnerTests.release.xcconfig"; path = "Target Support Files/Pods-RunnerTests/Pods-RunnerTests.release.xcconfig"; sourceTree = "<group>"; }; /* End PBXFileReference section */ /* Begin PBXFrameworksBuildPhase section */ @@ -90,6 +95,7 @@ isa = PBXFrameworksBuildPhase; buildActionMask = 2147483647; files = ( + F91BFA4332AA28C192B7B82A /* Pods_RunnerTests.framework in Frameworks */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -185,6 +191,9 @@ 11BB555C0F1767B9B5CB7CE0 /* Pods-Runner.debug.xcconfig */, 68C587FFA5A0B8F46A0C5150 /* Pods-Runner.release.xcconfig */, A7CE48BF63861DD9F3A9FA2F /* Pods-Runner.profile.xcconfig */, + D55F9235E34A37D8DC0CF6B1 /* Pods-RunnerTests.debug.xcconfig */, + F7EDBE938059E9DA7BF7376A /* Pods-RunnerTests.release.xcconfig */, + BC6DAC699B0C200C374E0DE1 /* Pods-RunnerTests.profile.xcconfig */, ); name = Pods; path = Pods; @@ -194,6 +203,7 @@ isa = PBXGroup; children = ( 5062035DDDD18FB35E98D5B6 /* Pods_Runner.framework */, + 649BD5F421DBAE62CB45DB38 /* Pods_RunnerTests.framework */, ); name = Frameworks; sourceTree = "<group>"; @@ -205,6 +215,7 @@ isa = PBXNativeTarget; buildConfigurationList = 331C80DE294CF71000263BE5 /* Build configuration list for PBXNativeTarget "RunnerTests" */; buildPhases = ( + A8D6C82A0A1533AF4531BF53 /* [CP] Check Pods Manifest.lock */, 331C80D1294CF70F00263BE5 /* Sources */, 331C80D2294CF70F00263BE5 /* Frameworks */, 331C80D3294CF70F00263BE5 /* Resources */, @@ -349,6 +360,28 @@ shellPath = /bin/sh; shellScript = "\"$FLUTTER_ROOT\"/packages/flutter_tools/bin/macos_assemble.sh && touch Flutter/ephemeral/tripwire"; }; + A8D6C82A0A1533AF4531BF53 /* [CP] Check Pods Manifest.lock */ = { + isa = PBXShellScriptBuildPhase; + buildActionMask = 2147483647; + files = ( + ); + inputFileListPaths = ( + ); + inputPaths = ( + "${PODS_PODFILE_DIR_PATH}/Podfile.lock", + "${PODS_ROOT}/Manifest.lock", + ); + name = "[CP] Check Pods Manifest.lock"; + outputFileListPaths = ( + ); + outputPaths = ( + "$(DERIVED_FILE_DIR)/Pods-RunnerTests-checkManifestLockResult.txt", + ); + runOnlyForDeploymentPostprocessing = 0; + shellPath = /bin/sh; + shellScript = "diff \"${PODS_PODFILE_DIR_PATH}/Podfile.lock\" \"${PODS_ROOT}/Manifest.lock\" > /dev/null\nif [ $? != 0 ] ; then\n # print error to STDERR\n echo \"error: The sandbox is not in sync with the Podfile.lock. Run 'pod install' or update your CocoaPods installation.\" >&2\n exit 1\nfi\n# This output is used by Xcode 'outputs' to avoid re-running this script phase.\necho \"SUCCESS\" > \"${SCRIPT_OUTPUT_FILE_0}\"\n"; + showEnvVarsInLog = 0; + }; DC1C8B6797A659BE5B59B986 /* [CP] Check Pods Manifest.lock */ = { isa = PBXShellScriptBuildPhase; buildActionMask = 2147483647; @@ -439,6 +472,7 @@ /* Begin XCBuildConfiguration section */ 331C80DB294CF71000263BE5 /* Debug */ = { isa = XCBuildConfiguration; + baseConfigurationReference = D55F9235E34A37D8DC0CF6B1 /* Pods-RunnerTests.debug.xcconfig */; buildSettings = { BUNDLE_LOADER = "$(TEST_HOST)"; CURRENT_PROJECT_VERSION = 1; @@ -453,6 +487,7 @@ }; 331C80DC294CF71000263BE5 /* Release */ = { isa = XCBuildConfiguration; + baseConfigurationReference = F7EDBE938059E9DA7BF7376A /* Pods-RunnerTests.release.xcconfig */; buildSettings = { BUNDLE_LOADER = "$(TEST_HOST)"; CURRENT_PROJECT_VERSION = 1; @@ -467,6 +502,7 @@ }; 331C80DD294CF71000263BE5 /* Profile */ = { isa = XCBuildConfiguration; + baseConfigurationReference = BC6DAC699B0C200C374E0DE1 /* Pods-RunnerTests.profile.xcconfig */; buildSettings = { BUNDLE_LOADER = "$(TEST_HOST)"; CURRENT_PROJECT_VERSION = 1;
diff --git a/packages/devtools_app/test/test_infra/fixtures/memory_app/lib/main.dart b/packages/devtools_app/test/test_infra/fixtures/memory_app/lib/main.dart index f263ad4..c1a47be 100644 --- a/packages/devtools_app/test/test_infra/fixtures/memory_app/lib/main.dart +++ b/packages/devtools_app/test/test_infra/fixtures/memory_app/lib/main.dart
@@ -87,28 +87,40 @@ mapSimpleKey = null; mapSimpleValue = null; map = null; + // record = null; } else { - childClass = _MyGarbage(_level + 1, _note); + _MyGarbage createInstance({String? note}) => + _MyGarbage(_level + 1, note ?? _note); - childList = - Iterable.generate(_width, (_) => _MyGarbage(_level + 1, _note)) - .toList(); + childClass = createInstance(); + + childList = Iterable.generate(_width, (_) => createInstance()).toList(); mapSimpleKey = Map.fromIterable( Iterable.generate(_width), - value: (_) => _MyGarbage(_level + 1, _note), + value: (_) => createInstance(), ); mapSimpleValue = Map.fromIterable( Iterable.generate(_width), - key: (_) => _MyGarbage(_level + 1, _note), + key: (_) => createInstance(), ); map = Map.fromIterable( Iterable.generate(_width), - key: (_) => _MyGarbage(_level + 1, _note), - value: (_) => _MyGarbage(_level + 1, _note), + key: (_) => createInstance(), + value: (_) => createInstance(), ); + + final _closureMember = createInstance(note: 'closure'); + closure = () { + if (identityHashCode(_closureMember) < 0) { + // We need this block to show compiler [_closureMember] is in use. + } + }; + + // TODO(polina-c): uncomment after figuring out how to enable records in dart format + // record = ('foo', count: 100, garbage: createInstance(note: 'record')); } } @@ -127,4 +139,8 @@ late final Map<dynamic, _MyGarbage>? mapSimpleKey; late final Map<_MyGarbage, dynamic>? mapSimpleValue; late final Map<_MyGarbage, _MyGarbage>? map; + late final void Function() closure; + + // TODO(polina-c): uncomment after figuring out how to enable records in dart format + // late final (String, {int count, _MyGarbage garbage})? record; }