From 68c0ebe48a46e5a52619ffa92002799d6cdaef87 Mon Sep 17 00:00:00 2001 From: Derek Xu Date: Wed, 1 Feb 2023 22:04:29 +0000 Subject: [PATCH] [VM/Service] Add optional parameters to getInstances This CL updates the VM Service spec to v4.1 and adds the optional `includeSubclasses` and `includeImplementers` parameters to the `getInstances` service procedure. This CL also adds `pkg/vm_service/test/get_instances_rpc_test.dart` which is based on `runtime/observatory/tests/service/get_instances_as_array_rpc_test.dart` TEST=CI Fixes https://github.com/dart-lang/sdk/issues/51003 Change-Id: Ia1ebec0ebeb6cba274621853e6486bab7cb7eb78 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279201 Commit-Queue: Derek Xu Reviewed-by: Ben Konyi --- pkg/vm_service/CHANGELOG.md | 5 ++ pkg/vm_service/java/version.properties | 2 +- pkg/vm_service/lib/src/vm_service.dart | 38 ++++++++-- pkg/vm_service/pubspec.yaml | 2 +- .../test/get_instances_rpc_test.dart | 74 +++++++++++++++++++ .../tests/service/get_version_rpc_test.dart | 2 +- .../tests/service_2/get_version_rpc_test.dart | 2 +- runtime/vm/service.cc | 27 +++---- runtime/vm/service.h | 2 +- runtime/vm/service/service.md | 19 +++-- 10 files changed, 140 insertions(+), 33 deletions(-) create mode 100644 pkg/vm_service/test/get_instances_rpc_test.dart diff --git a/pkg/vm_service/CHANGELOG.md b/pkg/vm_service/CHANGELOG.md index 806b07cac0a..0e5e74eab4b 100644 --- a/pkg/vm_service/CHANGELOG.md +++ b/pkg/vm_service/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 10.1.0 +- Update to version `4.1` of the spec. +- Add optional `includeSubclasses` and `includeImplementers` parameters to + `getInstances`. + ## 10.0.0 - Update to version `4.0` of the spec. - Update for incorrectly documented types for `WeakReference`'s `target`, diff --git a/pkg/vm_service/java/version.properties b/pkg/vm_service/java/version.properties index db1ef8e5fa6..96345eed22b 100644 --- a/pkg/vm_service/java/version.properties +++ b/pkg/vm_service/java/version.properties @@ -1 +1 @@ -version=4.0 +version=4.1 diff --git a/pkg/vm_service/lib/src/vm_service.dart b/pkg/vm_service/lib/src/vm_service.dart index fb22f150fa3..3e1806d6864 100644 --- a/pkg/vm_service/lib/src/vm_service.dart +++ b/pkg/vm_service/lib/src/vm_service.dart @@ -26,7 +26,7 @@ export 'snapshot_graph.dart' HeapSnapshotObjectNoData, HeapSnapshotObjectNullData; -const String vmServiceVersion = '4.0.0'; +const String vmServiceVersion = '4.1.0'; /// @optional const String optional = 'optional'; @@ -601,8 +601,7 @@ abstract class VmServiceInterface { String isolateId, String targetId, int limit); /// The `getInstances` RPC is used to retrieve a set of instances which are of - /// a specific class. This does not include instances of subclasses of the - /// given class. + /// a specific class. /// /// The order of the instances is undefined (i.e., not related to allocation /// order) and unstable (i.e., multiple invocations of this method against the @@ -617,6 +616,13 @@ abstract class VmServiceInterface { /// /// `limit` is the maximum number of instances to be returned. /// + /// If `includeSubclasses` is true, instances of subclasses of the specified + /// class will be included in the set. + /// + /// If `includeImplementers` is true, instances of implementers of the + /// specified class will be included in the set. Note that subclasses of a + /// class are also considered implementers of that class. + /// /// If `isolateId` refers to an isolate which has exited, then the `Collected` /// [Sentinel] is returned. /// @@ -625,7 +631,12 @@ abstract class VmServiceInterface { /// This method will throw a [SentinelException] in the case a [Sentinel] is /// returned. Future getInstances( - String isolateId, String objectId, int limit); + String isolateId, + String objectId, + int limit, { + bool? includeSubclasses, + bool? includeImplementers, + }); /// The `getIsolate` RPC is used to lookup an `Isolate` object by its `id`. /// @@ -1449,6 +1460,8 @@ class VmServerConnection { params!['isolateId'], params['objectId'], params['limit'], + includeSubclasses: params['includeSubclasses'], + includeImplementers: params['includeImplementers'], ); break; case 'getIsolate': @@ -1974,9 +1987,20 @@ class VmService implements VmServiceInterface { @override Future getInstances( - String isolateId, String objectId, int limit) => - _call('getInstances', - {'isolateId': isolateId, 'objectId': objectId, 'limit': limit}); + String isolateId, + String objectId, + int limit, { + bool? includeSubclasses, + bool? includeImplementers, + }) => + _call('getInstances', { + 'isolateId': isolateId, + 'objectId': objectId, + 'limit': limit, + if (includeSubclasses != null) 'includeSubclasses': includeSubclasses, + if (includeImplementers != null) + 'includeImplementers': includeImplementers, + }); @override Future getIsolate(String isolateId) => diff --git a/pkg/vm_service/pubspec.yaml b/pkg/vm_service/pubspec.yaml index adedca676a2..88d6cfaa3b0 100644 --- a/pkg/vm_service/pubspec.yaml +++ b/pkg/vm_service/pubspec.yaml @@ -1,5 +1,5 @@ name: vm_service -version: 10.0.0 +version: 10.1.0 description: >- A library to communicate with a service implementing the Dart VM service protocol. diff --git a/pkg/vm_service/test/get_instances_rpc_test.dart b/pkg/vm_service/test/get_instances_rpc_test.dart new file mode 100644 index 00000000000..a7500c6e877 --- /dev/null +++ b/pkg/vm_service/test/get_instances_rpc_test.dart @@ -0,0 +1,74 @@ +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'dart:developer'; + +import "package:test/test.dart"; +import 'package:vm_service/vm_service.dart'; + +import "common/service_test_common.dart"; +import "common/test_helper.dart"; + +class Class {} + +class Subclass extends Class {} + +class Implementor implements Class {} + +late final Class aClass; +late final Subclass aSubclass; +late final Implementor anImplementor; + +testMain() { + debugger(); + final _ = 1; + + aClass = new Class(); + aSubclass = new Subclass(); + anImplementor = new Implementor(); +} + +IsolateTest createTestThatExpectsInstanceCounts( + int numInstances, + int numInstancesWhenIncludingSubclasses, + int numInstancesWhenIncludingImplementers) { + return (VmService service, IsolateRef isolateRef) async { + final isolateId = isolateRef.id!; + final isolate = await service.getIsolate(isolateId); + final rootLib = + await service.getObject(isolateId, isolate.rootLib!.id!) as Library; + + Future instanceCount(String className, + {bool includeSubclasses = false, + bool includeImplementers = false}) async { + final result = await service.getInstances( + isolateId, + rootLib.classes!.singleWhere((cls) => cls.name == className).id!, + 10, + includeSubclasses: includeSubclasses, + includeImplementers: includeImplementers, + ); + expect(result.totalCount, result.instances!.length); + return result.totalCount!; + } + + expect(await instanceCount("Class"), numInstances); + expect(await instanceCount("Class", includeSubclasses: true), + numInstancesWhenIncludingSubclasses); + expect(await instanceCount("Class", includeImplementers: true), + numInstancesWhenIncludingImplementers); + }; +} + +final tests = [ + hasStoppedAtBreakpoint, + stoppedAtLine(25), + createTestThatExpectsInstanceCounts(0, 0, 0), + resumeIsolate, + createTestThatExpectsInstanceCounts(1, 2, 3), +]; + +main([args = const []]) async => + runIsolateTests(args, tests, 'get_instances_rpc_test.dart', + testeeConcurrent: testMain); diff --git a/runtime/observatory/tests/service/get_version_rpc_test.dart b/runtime/observatory/tests/service/get_version_rpc_test.dart index 2d1ea37c4e5..7cf155c24a6 100644 --- a/runtime/observatory/tests/service/get_version_rpc_test.dart +++ b/runtime/observatory/tests/service/get_version_rpc_test.dart @@ -12,7 +12,7 @@ var tests = [ final result = await vm.invokeRpcNoUpgrade('getVersion', {}); expect(result['type'], 'Version'); expect(result['major'], 4); - expect(result['minor'], 0); + expect(result['minor'], 1); expect(result['_privateMajor'], 0); expect(result['_privateMinor'], 0); }, diff --git a/runtime/observatory_2/tests/service_2/get_version_rpc_test.dart b/runtime/observatory_2/tests/service_2/get_version_rpc_test.dart index 5fe3ff70bfd..88d60152230 100644 --- a/runtime/observatory_2/tests/service_2/get_version_rpc_test.dart +++ b/runtime/observatory_2/tests/service_2/get_version_rpc_test.dart @@ -12,7 +12,7 @@ var tests = [ final result = await vm.invokeRpcNoUpgrade('getVersion', {}); expect(result['type'], equals('Version')); expect(result['major'], equals(4)); - expect(result['minor'], equals(0)); + expect(result['minor'], equals(1)); expect(result['_privateMajor'], equals(0)); expect(result['_privateMinor'], equals(0)); }, diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc index df7b380d76e..6304a75cfda 100644 --- a/runtime/vm/service.cc +++ b/runtime/vm/service.cc @@ -3522,25 +3522,20 @@ class GetInstancesVisitor : public ObjectGraph::Visitor { static const MethodParameter* const get_instances_params[] = { RUNNABLE_ISOLATE_PARAMETER, - NULL, + new IdParameter("objectId", /*required=*/true), + new UIntParameter("limit", /*required=*/true), + new BoolParameter("includeSubclasses", /*required=*/false), + new BoolParameter("includeImplementers", /*required=*/false), + nullptr, }; static void GetInstances(Thread* thread, JSONStream* js) { const char* object_id = js->LookupParam("objectId"); - if (object_id == NULL) { - PrintMissingParamError(js, "objectId"); - return; - } - const char* limit_cstr = js->LookupParam("limit"); - if (limit_cstr == NULL) { - PrintMissingParamError(js, "limit"); - return; - } - intptr_t limit; - if (!GetIntegerId(limit_cstr, &limit)) { - PrintInvalidParamError(js, "limit"); - return; - } + const intptr_t limit = UIntParameter::Parse(js->LookupParam("limit")); + const bool include_subclasses = + BoolParameter::Parse(js->LookupParam("includeSubclasses"), false); + const bool include_implementers = + BoolParameter::Parse(js->LookupParam("includeImplementers"), false); const Object& obj = Object::Handle(LookupHeapObject(thread, object_id, NULL)); if (obj.ptr() == Object::sentinel().ptr() || !obj.IsClass()) { @@ -3557,7 +3552,7 @@ static void GetInstances(Thread* thread, JSONStream* js) { { ObjectGraph graph(thread); HeapIterationScope iteration_scope(Thread::Current(), true); - MarkClasses(cls, false, false); + MarkClasses(cls, include_subclasses, include_implementers); graph.IterateObjects(&visitor); UnmarkClasses(); } diff --git a/runtime/vm/service.h b/runtime/vm/service.h index d0aa66991cd..b36e533d9d5 100644 --- a/runtime/vm/service.h +++ b/runtime/vm/service.h @@ -17,7 +17,7 @@ namespace dart { #define SERVICE_PROTOCOL_MAJOR_VERSION 4 -#define SERVICE_PROTOCOL_MINOR_VERSION 0 +#define SERVICE_PROTOCOL_MINOR_VERSION 1 class Array; class EmbedderServiceHandler; diff --git a/runtime/vm/service/service.md b/runtime/vm/service/service.md index 23eeb98ece5..83a52edaf2f 100644 --- a/runtime/vm/service/service.md +++ b/runtime/vm/service/service.md @@ -1,8 +1,8 @@ -# Dart VM Service Protocol 4.0 +# Dart VM Service Protocol 4.1 > Please post feedback to the [observatory-discuss group][discuss-list] -This document describes of _version 4.0_ of the Dart VM Service Protocol. This +This document describes of _version 4.1_ of the Dart VM Service Protocol. This protocol is used to communicate with a running Dart Virtual Machine. To use the Service Protocol, start the VM with the *--observe* flag. @@ -838,12 +838,13 @@ See [InboundReferences](#inboundreferences). ``` InstanceSet|Sentinel getInstances(string isolateId, string objectId, - int limit) + int limit, + bool includeSubclasses [optional], + bool includeImplementers [optional]) ``` The _getInstances_ RPC is used to retrieve a set of instances which are of a -specific class. This does not include instances of subclasses of the given -class. +specific class. The order of the instances is undefined (i.e., not related to allocation order) and unstable (i.e., multiple invocations of this method against the same class @@ -858,6 +859,13 @@ be the ID of a `Class`, otherwise an [RPC error](#rpc-error) is returned. _limit_ is the maximum number of instances to be returned. +If _includeSubclasses_ is true, instances of subclasses of the specified class +will be included in the set. + +If _includeImplementers_ is true, instances of implementers of the specified +class will be included in the set. Note that subclasses of a class are also +considered implementers of that class. + If _isolateId_ refers to an isolate which has exited, then the _Collected_ [Sentinel](#sentinel) is returned. @@ -4444,5 +4452,6 @@ version | comments 3.61 | Added `isolateGroupId` property to `@Isolate` and `Isolate`. 3.62 | Added `Set` to `InstanceKind`. 4.0 | Added `Record` and `RecordType` `InstanceKind`s, added a deprecation notice to the `decl` property of `BoundField`, added `name` property to `BoundField`, added a deprecation notice to the `parentListIndex` property of `InboundReference`, changed the type of the `parentField` property of `InboundReference` from `@Field` to `@Field\|string\|int`, added a deprecation notice to the `parentListIndex` property of `RetainingObject`, changed the type of the `parentField` property of `RetainingObject` from `string` to `string\|int`, removed the deprecated `timeSpan` property from `CpuSamples`, and removed the deprecated `timeSpan` property from `CpuSamplesEvent`. +4.1 | Added optional `includeSubclasses` and `includeImplementers` parameters to `getInstances`. [discuss-list]: https://groups.google.com/a/dartlang.org/forum/#!forum/observatory-discuss