Skip to content
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

Improve inspecting large Map and List types #3497

Merged
merged 20 commits into from
Nov 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,9 @@ class DebuggerController extends DisposableController
// Collecting frames for Dart web applications can be slow. At the potential
// cost of a flicker in the stack view, display only the top frame
// initially.
// TODO(elliette): Find a better solution for this. Currently, this means
// we fetch all variable objects twice (once in _getFullStack and once in
// in_createStackFrameWithLocation).
if (await serviceManager.connectedApp.isDartWebApp) {
_populateFrameInfo(
[
Expand Down
108 changes: 97 additions & 11 deletions packages/devtools_app/lib/src/debugger/debugger_model.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

import 'dart:convert';
import 'dart:math';
import 'dart:typed_data';

import 'package:flutter/foundation.dart';
Expand Down Expand Up @@ -380,10 +381,28 @@ Future<void> buildVariablesTree(
}
}
}
if (instanceRef != null && serviceManager.service != null) {

if (variable.childCount > Variable.MAX_CHILDREN_IN_GROUPING) {
final numChildrenInGrouping =
variable.childCount >= pow(Variable.MAX_CHILDREN_IN_GROUPING, 2)
? (roundToNearestPow10(variable.childCount) /
Variable.MAX_CHILDREN_IN_GROUPING)
.floor()
: Variable.MAX_CHILDREN_IN_GROUPING;

var start = variable.offset ?? 0;
final end = start + variable.childCount;
while (start < end) {
final count = min(end - start, numChildrenInGrouping);
variable.addChild(
Variable.grouping(variable.ref, offset: start, count: count),
);
start += count;
}
} else if (instanceRef != null && serviceManager.service != null) {
try {
final dynamic result = await serviceManager.service
.getObject(variable.ref.isolateRef.id, instanceRef.id);
final dynamic result =
await _getObjectWithRetry(instanceRef.id, variable);
if (result is Instance) {
if (result.associations != null) {
variable.addAllChildren(
Expand Down Expand Up @@ -439,8 +458,8 @@ Future<void> buildVariablesTree(
// TODO(jacobr): cache the full class hierarchy so we can cheaply check
// instanceRef is DiagnosticsNode without having to do an eval.
if (instanceRef != null &&
(instanceRef.classRef.name == 'DiagnosticableTreeNode' ||
instanceRef.classRef.name == 'DiagnosticsProperty')) {
(instanceRef.classRef?.name == 'DiagnosticableTreeNode' ||
instanceRef.classRef?.name == 'DiagnosticsProperty')) {
// The user is expecting to see the object the DiagnosticsNode is
// describing not the DiagnosticsNode itself.
try {
Expand Down Expand Up @@ -475,6 +494,25 @@ Future<void> buildVariablesTree(
variable.treeInitializeComplete = true;
}

// TODO(elliette): Remove once the fix for dart-lang/webdev/issues/1439
// is landed. This works around a bug in DWDS where an error is thrown if
// `getObject` is called with offset/count for an object that has no length.
Future<Obj> _getObjectWithRetry(
String objectId,
Variable variable,
) async {
try {
final dynamic result = await serviceManager.service.getObject(
variable.ref.isolateRef.id, objectId,
offset: variable.offset, count: variable.childCount);
elliette marked this conversation as resolved.
Show resolved Hide resolved
return result;
} catch (e) {
final dynamic result = await serviceManager.service
.getObject(variable.ref.isolateRef.id, objectId);
return result;
}
}

Future<Variable> _buildVariable(
RemoteDiagnosticsNode diagnostic,
ObjectGroupBase inspectorService,
Expand Down Expand Up @@ -528,8 +566,9 @@ List<Variable> _createVariablesForAssociations(
value: association.value,
isolateRef: isolateRef,
);
final entryNum = instance.offset == null ? i : i + instance.offset;
variables.add(
Variable.text('[Entry $i]')
Variable.text('[Entry $entryNum]')
..addChild(key)
..addChild(value),
);
Expand Down Expand Up @@ -604,9 +643,10 @@ List<Variable> _createVariablesForBytes(
}

for (int i = 0; i < result.length; i++) {
final name = instance.offset == null ? i : i + instance.offset;
variables.add(
Variable.fromValue(
name: '[$i]',
name: '[$name]',
value: result[i],
isolateRef: isolateRef,
),
Expand All @@ -621,9 +661,10 @@ List<Variable> _createVariablesForElements(
) {
final variables = <Variable>[];
for (int i = 0; i < instance.elements.length; i++) {
final name = instance.offset == null ? i : i + instance.offset;
variables.add(
Variable.fromValue(
name: '[$i]',
name: '[$name]',
value: instance.elements[i],
isolateRef: isolateRef,
),
Expand Down Expand Up @@ -656,8 +697,13 @@ List<Variable> _createVariablesForFields(
// InstanceRef objects have become sentinels.
// TODO(jacobr): consider a new class name. This class is more just the data
// model for a tree of Dart objects with properties rather than a "Variable".
// TODO(elliette): Refactor to make name, ref, text named (and potentially
// required?) parameters. Give ref a type.
class Variable extends TreeNode<Variable> {
Variable._(this.name, ref, this.text) : _ref = ref {
Variable._(this.name, ref, this.text, {int offset, int childCount})
Copy link
Member

Choose a reason for hiding this comment

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

make name, ref, and text required and named. Also, why doesn't ref have a type?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a TODO to follow this up with that change, since it looks like it would be a bit of a refactor from what we currently have and I would rather keep that refactor separate. Let me know if this is okay!

Copy link
Member

Choose a reason for hiding this comment

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

sgtm

: _ref = ref,
_offset = offset,
_childCount = childCount {
indentChildren = ref?.diagnostic?.style != DiagnosticsTreeStyle.flat;
}

Expand Down Expand Up @@ -702,18 +748,58 @@ class Variable extends TreeNode<Variable> {
return Variable._(null, null, text);
}

factory Variable.grouping(
GenericInstanceRef ref, {
@required int offset,
@required int count,
}) {
return Variable._(
null,
ref,
'[$offset - ${offset + count - 1}]',
offset: offset,
childCount: count,
);
}

static const MAX_CHILDREN_IN_GROUPING = 100;

final String text;
final String name;
GenericInstanceRef get ref => _ref;
GenericInstanceRef _ref;

/// The point to fetch the variable from (in the case of large variables that
/// we fetch only parts of at a time).
int get offset => _offset ?? 0;

int _offset;

int get childCount {
if (_childCount != null) return _childCount;

final value = this.value;
if (value is InstanceRef) {
if (value.kind != null &&
(value.kind.endsWith('List') ||
value.kind == InstanceKind.kList ||
value.kind == InstanceKind.kMap)) {
return value.length ?? 0;
elliette marked this conversation as resolved.
Show resolved Hide resolved
}
}

return 0;
}

int _childCount;

bool treeInitializeStarted = false;
bool treeInitializeComplete = false;

@override
bool get isExpandable {
if (treeInitializeComplete || children.isNotEmpty) {
return children.isNotEmpty;
if (treeInitializeComplete || children.isNotEmpty || childCount > 0) {
return children.isNotEmpty || childCount > 0;
}
final diagnostic = ref.diagnostic;
if (diagnostic != null &&
Expand Down
9 changes: 8 additions & 1 deletion packages/devtools_app/lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,14 @@ T nullSafeMax<T extends num>(T a, T b) {
return max<T>(a, b);
}

int log2(num x) => (log(x) / log(2)).floor();
double logBase({@required int x, @required int base}) {
return log(x) / log(base);
}

int log2(num x) => (logBase(x: x.floor(), base: 2)).floor();

int roundToNearestPow10(int x) =>
pow(10, logBase(x: x, base: 10).ceil()).floor();

String isolateName(IsolateRef ref) {
// analysis_server.dart.snapshot$main
Expand Down
9 changes: 8 additions & 1 deletion packages/devtools_app/lib/src/vm_service_wrapper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,14 @@ class VmServiceWrapper implements VmService {
int offset,
int count,
}) {
return trackFuture('getObject', _vmService.getObject(isolateId, objectId));
return trackFuture(
'getObject',
_vmService.getObject(
isolateId,
objectId,
offset: offset,
count: count,
));
}

@override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,14 @@
files = (
);
inputPaths = (
"${PODS_ROOT}/Target Support Files/Pods-Runner/Pods-Runner-frameworks.sh",
"${BUILT_PRODUCTS_DIR}/file_selector_macos/file_selector_macos.framework",
"${BUILT_PRODUCTS_DIR}/url_launcher_macos/url_launcher_macos.framework",
);
name = "[CP] Embed Pods Frameworks";
outputPaths = (
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/file_selector_macos.framework",
"${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/url_launcher_macos.framework",
);
runOnlyForDeploymentPostprocessing = 0;
shellPath = /bin/sh;
Expand Down
12 changes: 9 additions & 3 deletions packages/devtools_app/test/association_variable_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ void main() {
kind: InstanceKind.kMap,
id: '123',
classRef: null,
length: 2,
associations: [
MapAssociation(
key: InstanceRef(
Expand Down Expand Up @@ -83,7 +84,8 @@ void main() {
),
isolateRef,
);
when(manager.service.getObject(any, any)).thenAnswer((_) async {
when(manager.service.getObject(any, any, offset: 0, count: 2))
.thenAnswer((_) async {
return instance;
});

Expand Down Expand Up @@ -111,6 +113,7 @@ void main() {
kind: InstanceKind.kMap,
id: '123',
classRef: null,
length: 2,
associations: [
MapAssociation(
key: InstanceRef(
Expand Down Expand Up @@ -141,7 +144,8 @@ void main() {
),
isolateRef,
);
when(manager.service.getObject(any, any)).thenAnswer((_) async {
when(manager.service.getObject(any, any, offset: 0, count: 2))
.thenAnswer((_) async {
return instance;
});

Expand All @@ -168,6 +172,7 @@ void main() {
kind: InstanceKind.kMap,
id: '123',
classRef: null,
length: 2,
associations: [
MapAssociation(
key: InstanceRef(
Expand Down Expand Up @@ -197,7 +202,8 @@ void main() {
),
isolateRef,
);
when(manager.service.getObject(any, any)).thenAnswer((_) async {
when(manager.service.getObject(any, any, offset: 0, count: 2))
.thenAnswer((_) async {
return instance;
});

Expand Down
Loading