From 4475267f706abaedb56e0e1da0e6725762cdae13 Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Fri, 7 Jan 2022 21:22:56 +0000 Subject: [PATCH] [ VM / Service ] Add --log_service_response_sizes= debug option Providing `--log_service_response_sizes` will cause the VM to log VM service response sizes to the provided file in CSV format. Also added `--service_response_sizes_directory` to the service test runner to allow for collecting response size data for the entire service test suite. TEST=Local Change-Id: I7aaf4ba936e2593e67d46ff9052e2130374ef461 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/226805 Reviewed-by: Siva Annamalai Commit-Queue: Ben Konyi --- pkg/test_runner/lib/src/configuration.dart | 2 + pkg/test_runner/lib/src/options.dart | 7 ++- pkg/test_runner/lib/src/test_suite.dart | 2 + .../tests/service/test_helper.dart | 13 +++++ .../tests/service_2/test_helper.dart | 13 +++++ runtime/vm/dart.cc | 2 + runtime/vm/service.cc | 53 +++++++++++++++++++ runtime/vm/service.h | 8 +++ 8 files changed, 99 insertions(+), 1 deletion(-) diff --git a/pkg/test_runner/lib/src/configuration.dart b/pkg/test_runner/lib/src/configuration.dart index 51110c0c6e6..198c01aa74f 100644 --- a/pkg/test_runner/lib/src/configuration.dart +++ b/pkg/test_runner/lib/src/configuration.dart @@ -64,6 +64,7 @@ class TestConfiguration { this.keepGeneratedFiles, this.sharedOptions, String packages, + this.serviceResponseSizesDirectory, this.suiteDirectory, this.outputDirectory, this.reproducingArguments, @@ -176,6 +177,7 @@ class TestConfiguration { return _packages; } + final String serviceResponseSizesDirectory; final String outputDirectory; final String suiteDirectory; String get babel => configuration.babel; diff --git a/pkg/test_runner/lib/src/options.dart b/pkg/test_runner/lib/src/options.dart index dccb5f4b660..142b123c721 100644 --- a/pkg/test_runner/lib/src/options.dart +++ b/pkg/test_runner/lib/src/options.dart @@ -338,7 +338,10 @@ has been specified on the command line.''', hide: true), _Option.bool('print_passing_stdout', 'Print the stdout of passing, as well as failing, tests.', - hide: true) + hide: true), + _Option('service_response_sizes_directory', + 'Log VM service response sizes in CSV files in the provided directory', + hide: true), ]; /// For printing out reproducing command lines, we don't want to add these @@ -777,6 +780,8 @@ has been specified on the command line.''', localIP: data["local_ip"] as String, sharedOptions: sharedOptions, packages: data["packages"] as String, + serviceResponseSizesDirectory: + data['service_response_sizes_directory'] as String, suiteDirectory: data["suite_dir"] as String, outputDirectory: data["output_directory"] as String, reproducingArguments: diff --git a/pkg/test_runner/lib/src/test_suite.dart b/pkg/test_runner/lib/src/test_suite.dart index 9a5d6858a48..bae578b842e 100644 --- a/pkg/test_runner/lib/src/test_suite.dart +++ b/pkg/test_runner/lib/src/test_suite.dart @@ -763,6 +763,8 @@ class StandardTestSuite extends TestSuite { ...vmOptionsList[vmOptionsVariant], ...extraVmOptions, if (emitDdsTest) '-DUSE_DDS=true', + if (configuration.serviceResponseSizesDirectory != null) + '-DSERVICE_RESPONSE_SIZES_DIR=${configuration.serviceResponseSizesDirectory}', ]; var isCrashExpected = expectations.contains(Expectation.crash); var commands = _makeCommands( diff --git a/runtime/observatory/tests/service/test_helper.dart b/runtime/observatory/tests/service/test_helper.dart index 487c09891f9..a0e0c192f92 100644 --- a/runtime/observatory/tests/service/test_helper.dart +++ b/runtime/observatory/tests/service/test_helper.dart @@ -18,6 +18,10 @@ export 'service_test_common.dart' show DDSTest, IsolateTest, VMTest; /// Determines whether DDS is enabled for this test run. const bool useDds = const bool.fromEnvironment('USE_DDS'); +/// The directory to output VM service response size data files to. +const String serviceResponseSizesDir = + const String.fromEnvironment('SERVICE_RESPONSE_SIZES_DIR'); + /// The extra arguments to use const List extraDebuggingArgs = ['--lazy-async-stacks']; @@ -178,6 +182,15 @@ class _ServiceTesteeLauncher { if (!testeeControlsServer) { fullArgs.add('--enable-vm-service:$port'); } + if (serviceResponseSizesDir != null) { + // Dump service response size details to a CSV. This feature is not used + // on the build bots and the generated output will persist after the test + // has completed. + final dir = path.prettyUri(serviceResponseSizesDir); + final testName = path.withoutExtension(path.basename(args.last)); + final logName = '${testName}_${useDds ? "" : "no_"}dds_sizes.csv'; + fullArgs.add('--log_service_response_sizes=$dir/$logName'); + } fullArgs.addAll(args); return _spawnCommon(dartExecutable, fullArgs, {}); diff --git a/runtime/observatory_2/tests/service_2/test_helper.dart b/runtime/observatory_2/tests/service_2/test_helper.dart index 86e7b131eee..6c782b82e79 100644 --- a/runtime/observatory_2/tests/service_2/test_helper.dart +++ b/runtime/observatory_2/tests/service_2/test_helper.dart @@ -20,6 +20,10 @@ export 'service_test_common.dart' show DDSTest, IsolateTest, VMTest; /// Determines whether DDS is enabled for this test run. const bool useDds = const bool.fromEnvironment('USE_DDS'); +/// The directory to output VM service response size data files to. +const String serviceResponseSizesDir = + const String.fromEnvironment('SERVICE_RESPONSE_SIZES_DIR'); + /// The extra arguments to use const List extraDebuggingArgs = ['--lazy-async-stacks']; @@ -178,6 +182,15 @@ class _ServiceTesteeLauncher { if (!testeeControlsServer) { fullArgs.add('--enable-vm-service:$port'); } + if (serviceResponseSizesDir != null) { + // Dump service response size details to a CSV. This feature is not used + // on the build bots and the generated output will persist after the test + // has completed. + final dir = path.prettyUri(serviceResponseSizesDir); + final testName = path.withoutExtension(path.basename(args.last)); + final logName = '${testName}_${useDds ? "" : "no_"}dds_sizes.csv'; + fullArgs.add('--log_service_response_sizes=$dir/$logName'); + } fullArgs.addAll(args); return _spawnCommon(dartExecutable, fullArgs, {}); diff --git a/runtime/vm/dart.cc b/runtime/vm/dart.cc index 9f78ed866dd..97953903426 100644 --- a/runtime/vm/dart.cc +++ b/runtime/vm/dart.cc @@ -343,6 +343,7 @@ char* Dart::DartInit(const uint8_t* vm_isolate_snapshot, Isolate::InitVM(); UserTags::Init(); PortMap::Init(); + Service::Init(); FreeListElement::Init(); ForwardingCorpse::Init(); Api::Init(); @@ -802,6 +803,7 @@ char* Dart::Cleanup() { ShutdownIsolate(); vm_isolate_ = NULL; ASSERT(Isolate::IsolateListLength() == 0); + Service::Cleanup(); PortMap::Cleanup(); UserTags::Cleanup(); IsolateGroup::Cleanup(); diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc index 38e0d7521ed..2f2a075c699 100644 --- a/runtime/vm/service.cc +++ b/runtime/vm/service.cc @@ -72,6 +72,56 @@ DEFINE_FLAG(bool, "Print a message when an isolate is paused but there is no " "debugger attached."); +DEFINE_FLAG( + charp, + log_service_response_sizes, + nullptr, + "Log sizes of service responses and events to a file in CSV format."); + +void* Service::service_response_size_log_file_ = nullptr; + +void Service::LogResponseSize(const char* method, JSONStream* js) { + if (service_response_size_log_file_ == nullptr) { + return; + } + Dart_FileWriteCallback file_write = Dart::file_write_callback(); + char* entry = + OS::SCreate(nullptr, "%s, %" Pd "\n", method, js->buffer()->length()); + (*file_write)(entry, strlen(entry), service_response_size_log_file_); + free(entry); +} + +void Service::Init() { + if (FLAG_log_service_response_sizes == nullptr) { + return; + } + Dart_FileOpenCallback file_open = Dart::file_open_callback(); + Dart_FileWriteCallback file_write = Dart::file_write_callback(); + Dart_FileCloseCallback file_close = Dart::file_close_callback(); + if ((file_open == nullptr) || (file_write == nullptr) || + (file_close == nullptr)) { + OS::PrintErr("Error: Could not access file callbacks."); + UNREACHABLE(); + } + ASSERT(service_response_size_log_file_ == nullptr); + service_response_size_log_file_ = + (*file_open)(FLAG_log_service_response_sizes, true); + if (service_response_size_log_file_ == nullptr) { + OS::PrintErr("Warning: Failed to open service response size log file: %s\n", + FLAG_log_service_response_sizes); + return; + } +} + +void Service::Cleanup() { + if (service_response_size_log_file_ == nullptr) { + return; + } + Dart_FileCloseCallback file_close = Dart::file_close_callback(); + (*file_close)(service_response_size_log_file_); + service_response_size_log_file_ = nullptr; +} + static void PrintInvalidParamError(JSONStream* js, const char* param) { #if !defined(PRODUCT) js->PrintError(kInvalidParams, "%s: invalid '%s' parameter: %s", js->method(), @@ -988,6 +1038,7 @@ ErrorPtr Service::InvokeMethod(Isolate* I, return T->StealStickyError(); } method->entry(T, &js); + Service::LogResponseSize(c_method_name, &js); js.PostReply(); return T->StealStickyError(); } @@ -1239,6 +1290,8 @@ void Service::PostEventImpl(Isolate* isolate, } } + Service::LogResponseSize(kind, event); + // Message is of the format [, ]. // // Build the event message in the C heap to avoid dart heap diff --git a/runtime/vm/service.h b/runtime/vm/service.h index 751c1e54b53..d6324d9d324 100644 --- a/runtime/vm/service.h +++ b/runtime/vm/service.h @@ -89,6 +89,9 @@ class StreamInfo { class Service : public AllStatic { public: + static void Init(); + static void Cleanup(); + // Handles a message which is not directed to an isolate. static ErrorPtr HandleRootMessage(const Array& message); @@ -159,6 +162,9 @@ class Service : public AllStatic { const Instance& id, const Error& error); + // Logs the size of the contents of `js` to FLAG_log_service_response_sizes. + static void LogResponseSize(const char* method, JSONStream* js); + // Enable/Disable timeline categories. // Returns True if the categories were successfully enabled, False otherwise. static bool EnableTimelineStreams(char* categories_list); @@ -250,6 +256,8 @@ class Service : public AllStatic { static Dart_GetVMServiceAssetsArchive get_service_assets_callback_; static Dart_EmbedderInformationCallback embedder_information_callback_; + static void* service_response_size_log_file_; + static const uint8_t* dart_library_kernel_; static intptr_t dart_library_kernel_len_; };