Add a way to fetch values for getters for an object through the VM Service(/DDS?)

[DanTup]

In VS Code's debug adapter and the new SDK DAP implementation, there is a setting that allows showing values of getters in debug tooltips/watch window/etc.. This works by walking up the class hierarchy getting all of the names of any getters at each level and then sending evaluate requests for each.

This code seems fragile and involves looking at an internal field to identify the getters (f.json?['_kind'] == 'GetterFunction'). It would be better to have a more official way to do this (and perhaps in a single request).

sdk/pkg/dds/lib/src/dap/protocol_converter.dart

Lines 444 to 475 in 248e943

	/// Collect a list of all getter names for [classRef] and its super classes.
	///
	/// This is used to show/evaluate getters in debug views like hovers and
	/// variables/watch panes.
	Future<Set<String>> _getterNamesForClassHierarchy(
	ThreadInfo thread,
	vm.ClassRef? classRef,
	) async {
	final getterNames = <String>{};
	final service = _adapter.vmService;
	while (service != null && classRef != null) {
	final classResponse =
	await service.getObject(thread.isolate.id!, classRef.id!);
	if (classResponse is! vm.Class) {
	break;
	}
	final functions = classResponse.functions;
	if (functions != null) {
	final instanceFields = functions.where((f) =>
	// TODO(dantup): Update this to use something better that bkonyi is
	// adding to the protocol.
	f.json?['_kind'] == 'GetterFunction' &&
	!(f.isStatic ?? false) &&
	!(f.isConst ?? false));
	getterNames.addAll(instanceFields.map((f) => f.name!));
	}
	classRef = classResponse.superClass;
	}
	return getterNames;
	}

@bkonyi (I think @jacob314 may also have been interested in this?)

[jacob314]

Fyi @annagrin

[jacob314]

Having https://github.com/dart-lang/sdk/blob/master/runtime/vm/service/service.md#function surface whether a Function is a getter or not SGTM. I think DDC is able to internally disambiguate methods from getters because it needs to bind "this" when calling "load" on a Function but not on a getter.

[jacob314]

Fyi @nshahan @grouma

[nshahan]

@annagrin I believe we could collect all the getters from the symbols data generated by DDC when we have it working but at the moment we don't have any markers for getters/setters/methods. They all just appear as functions.

[annagrin]

It looks like we have a couple of possible solutions:

(1) Simple approach (keeping the code @DanTup posted in the issue, and making it more reliable)

• VM service protocol should have a field in vm.Function type marking it as getter/setter
• ddc should store getter/setter info in debug symbols
• dwds should implement that marking using the symbols

Note that debug symbols work in ddc and dwds is currently in progress.

(2) Improvement for efficiency

As far as understand, currently all UIs are displaying instances in various places (watch window, variables, expression evaluation window, hover over) using the following logic:

(@DanTup please correct me if I am wrong)

• call getObject on the instance ID, get an vm.Instance back
• use instance.fields that contain vm.BoundFields (field names and their values)
• call .toString on the value
• if a field is expanded, use the same process on the field's instance ID

We could use a similar approach for properties: have a way to request properties on an instance, and push the implementation of this to dwds instead of the UI. For example, dwds could evaluate each property, or collect them into a map for evaluation, or maybe skip the evaluation altogether if it knows what JS code corresponds to dart code for the property.

Note that on the web the evaluation of properties is potentially slower than getObject, so we may want to separate getting properties into another API so we can only get the properties when needed.

@bkonyi does (1) or (2) seem reasonable from the vm service API (or DDS API) perspective? Approach (2) or any API that lets the UI get the properties in smallest number of API calls is preferable.

From dwds/ddc side, both need debug symbols to implement properly (but @jacob314 has an idea for a workaround). Symbols work is currently in progress:
#40273

[DanTup]

(@DanTup please correct me if I am wrong)

• call getObject on the instance ID, get an vm.Instance back
• use instance.fields that contain vm.BoundFields (field names and their values)
• call .toString on the value
• if a field is expanded, use the same process on the field's instance ID

Yep, that sounds right - although calling toString() is configurable (evaluateToStringInDebugViews setting) and we cap the number of toStrings() we'll call in a single parent request (so if you expand a large array or object we don't send a huge number of calls - although it's probably a little inconsistent that we don't do any sort of capping on getters in the same way).

[annagrin]

@DanTup thanks for confirmation! I added the implementation in dwds part of this issue to my 'object inspection' project.
Of course, this would need vm_service/DDS support as well.

[bkonyi]

Adding some form of field to Function for getter/setters SGTM (especially if we're peeking at private fields in contexts where we shouldn't be... :-)).

[bkonyi]

Thinking about this some more, would it be feasible to just use evaluate to just evaluate the getter against an instance?

[DanTup]

That's basically what we're doing today, and as long as the VM doesn't mind is sending lots of parallel requests for them, that's probably fine.

The real issue is getting the list of getters to do that. We have to walk up the class hierarchy to get the getter from each level (and use f.json?['_kind'] == 'GetterFunction' to know which are getters).

So I guess the important part of the request is getting the list of getters for an instance efficiently (and without using private fields).

[bkonyi]

@DanTup is this still something you're interested in? @Function now has isGetter and isSetter properties, so there's no longer a need to access any private properties.

Are we encountering any performance issues with the current implementation?

[DanTup]

We have moved over to isGetter in DAP (5e255e0).

I haven't seen/had reports of performance issues (nor have I done any timing), although I suspect most people wouldn't raise issues if expanding objects is just a little slow. Whether this is the place to focus attention if we want to improve debugging performance though, I'm not sure - I suspect there are other things that would show up slower than this.

[bkonyi]

Thanks for the feedback. I'm going to drop this to P3 then since it's not something that needs immediate attention.