From c192336ddebdaaacc82f26ff26855eed3818aa19 Mon Sep 17 00:00:00 2001 From: Polina Cherkasova Date: Mon, 22 May 2023 18:00:26 +0000 Subject: [PATCH] Add auto-snapshotting and usage reporting. Change-Id: I84b48cc750638ab3bcc138f2a0425442559e6d54 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304212 Bot-Commit: Rubber Stamper Commit-Queue: Polina Cherkasova Reviewed-by: Ilya Yanok Reviewed-by: Samuel Rawlins --- .../lib/src/analytics/analytics_manager.dart | 23 +++++ .../lib/src/server/driver.dart | 7 ++ .../usage_tracking/AUTOSNAPSHOTTING.md | 48 ++++++++++ .../usage_tracking/usage_tracking.dart | 88 +++++++++++++++++++ pkg/analysis_server/pubspec.yaml | 1 + pkg/analysis_server/test/test_all.dart | 2 + .../test/utilities/test_all.dart | 13 +++ .../utilities/usage_tracking/test_all.dart | 13 +++ .../usage_tracking/usage_tracking_test.dart | 44 ++++++++++ 9 files changed, 239 insertions(+) create mode 100644 pkg/analysis_server/lib/src/utilities/usage_tracking/AUTOSNAPSHOTTING.md create mode 100644 pkg/analysis_server/lib/src/utilities/usage_tracking/usage_tracking.dart create mode 100644 pkg/analysis_server/test/utilities/test_all.dart create mode 100644 pkg/analysis_server/test/utilities/usage_tracking/test_all.dart create mode 100644 pkg/analysis_server/test/utilities/usage_tracking/usage_tracking_test.dart diff --git a/pkg/analysis_server/lib/src/analytics/analytics_manager.dart b/pkg/analysis_server/lib/src/analytics/analytics_manager.dart index a283aa11f3e..c2eedd84b91 100644 --- a/pkg/analysis_server/lib/src/analytics/analytics_manager.dart +++ b/pkg/analysis_server/lib/src/analytics/analytics_manager.dart @@ -20,6 +20,7 @@ import 'package:analysis_server/src/protocol_server.dart'; import 'package:analysis_server/src/status/pages.dart'; import 'package:analyzer/dart/analysis/analysis_context.dart'; import 'package:collection/collection.dart'; +import 'package:leak_tracker/src/usage_tracking/model.dart'; import 'package:unified_analytics/unified_analytics.dart'; /// An interface for managing and reporting analytics. @@ -190,6 +191,28 @@ class AnalyticsManager { requestData.addValue('openWorkspacePaths', openWorkspacePaths.length); } + Future sendMemoryUsage(MemoryUsageEvent event) async { + final delta = event.delta; + var seconds = event.period?.inSeconds; + + assert((event.delta == null) == (event.period == null)); + + if (delta == null || seconds == null) { + await analytics.sendEvent(eventName: DashEvent.memoryInfo, eventData: { + 'rss': event.rss, + }); + return; + } + + if (seconds == 0) seconds = 1; + + await analytics.sendEvent(eventName: DashEvent.memoryInfo, eventData: { + 'rss': event.rss, + 'periodSec': seconds, + 'mbPerSec': delta / seconds, + }); + } + /// Record that the given [response] was sent to the client. void sentResponse({required Response response}) { var sendTime = DateTime.now(); diff --git a/pkg/analysis_server/lib/src/server/driver.dart b/pkg/analysis_server/lib/src/server/driver.dart index 05a8dd9618a..08f4305002a 100644 --- a/pkg/analysis_server/lib/src/server/driver.dart +++ b/pkg/analysis_server/lib/src/server/driver.dart @@ -41,6 +41,8 @@ import 'package:telemetry/crash_reporting.dart'; import 'package:telemetry/telemetry.dart' as telemetry; import 'package:unified_analytics/unified_analytics.dart'; +import '../utilities/usage_tracking/usage_tracking.dart'; + /// The [Driver] class represents a single running instance of the analysis /// server application. It is responsible for parsing command line options /// and starting the HTTP and/or stdio servers. @@ -355,6 +357,11 @@ class Driver implements ServerStarter { errorNotifier, sendPort); } + + configureMemoryUsageTracking( + arguments, + (memoryUsageEvent) => analyticsManager.sendMemoryUsage(memoryUsageEvent), + ); } void startAnalysisServer( diff --git a/pkg/analysis_server/lib/src/utilities/usage_tracking/AUTOSNAPSHOTTING.md b/pkg/analysis_server/lib/src/utilities/usage_tracking/AUTOSNAPSHOTTING.md new file mode 100644 index 00000000000..77a93f0fc5b --- /dev/null +++ b/pkg/analysis_server/lib/src/utilities/usage_tracking/AUTOSNAPSHOTTING.md @@ -0,0 +1,48 @@ +# Auto-snapshotting + +IMPORTANT: memory snapshots should not be requested from external users because they may contain PII. + +If a user reports that the process `dart:analysis_server.dart.snapshot` takes too much memory, +and the issue is hard to reproduce, you may want to request memory snapshots from the user. + +## Request numbers + +Ask user to provide memory footprint for the process `dart:analysis_server.dart.snapshot`. +If there are many instances of the process, ask for the biggest memory footprint among +the instances. + +- **Mac**: column 'Real Mem' in 'Activity Monitor' +- **Windows**: TODO: add content +- **Linux**: TODO: add content + +## Create auto-snapshotting argument + +Based on the reported and expected values, construct auto-snapshotting argument. See example in +the [test file](../../../../test/utilities/usage_tracking/usage_tracking_test.dart), the +constant `_autosnapshottingArg`. + +See explanation of parameters in +[documentation for AutoSnapshottingConfig](https://github.com/dart-lang/leak_tracker/blob/main/lib/src/usage_tracking/model.dart). + +## Instruct user to configure analyzer + +Pass the created argument to the user and instruct them to configure +analyzer. + +### For VSCode + +1. Open Settings > Extensions > Dart > Analyser +2. Add the argument to `Dart: Analyzer Additional Args` + +### For Android Studio + +1. Double-press Shift +2. Type 'Registry' into search field +3. Click 'Registry...' +4. Add the argument to the value of the key 'dart.server.additional.arguments' + +## Analyze snapshots + +Ask user to provide the collected snapshots and analyze them. + +TODO (polina-c): link DevTools documentation diff --git a/pkg/analysis_server/lib/src/utilities/usage_tracking/usage_tracking.dart b/pkg/analysis_server/lib/src/utilities/usage_tracking/usage_tracking.dart new file mode 100644 index 00000000000..c15fabc9236 --- /dev/null +++ b/pkg/analysis_server/lib/src/utilities/usage_tracking/usage_tracking.dart @@ -0,0 +1,88 @@ +// 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 'package:args/args.dart'; +import 'package:collection/collection.dart'; +import 'package:leak_tracker/src/usage_tracking/model.dart'; +import 'package:leak_tracker/src/usage_tracking/usage_tracking.dart'; + +void configureMemoryUsageTracking( + List arguments, + UsageCallback callback, +) { + final config = UsageTrackingConfig( + interval: const Duration(seconds: 1), + usageEventsConfig: UsageEventsConfig( + callback, + deltaMb: 512, + ), + autoSnapshottingConfig: parseAutoSnapshottingConfig(arguments), + ); + trackMemoryUsage(config); +} + +/// Parses the config for autosnapshotting from CLI [args]. +/// +/// See example of config in tests for this function. +/// +/// If there is no argument that starts with '--autosnapshotting=', returns null. +/// +/// In case of error throws exception. +AutoSnapshottingConfig? parseAutoSnapshottingConfig(List args) { + const argName = 'autosnapshotting'; + final arg = args.firstWhereOrNull((a) => a.contains('--$argName')); + if (arg == null) return null; + var parser = ArgParser()..addMultiOption(argName, splitCommas: true); + final parsedArgs = parser.parse([arg]); + assert(parsedArgs.options.contains(argName)); + final values = parsedArgs[argName] as List; + if (values.isEmpty) return null; + final items = Map.fromEntries(values.map((e) { + final keyValue = e.split('='); + if (keyValue.length != 2) { + throw ArgumentError( + 'Invalid auto-snapshotting config: $values.\n' + 'Expected "key-value", got "$e".', + ); + } + final keyString = keyValue[0]; + try { + final key = _Keys.values.byName(keyString); + return MapEntry(key, keyValue[1]); + } on ArgumentError { + throw ArgumentError('Invalid auto-snapshotting key: $keyString".'); + } + })); + if (!items.containsKey(_Keys.dir)) { + throw ArgumentError( + '${_Keys.dir.name} should be provided for auto-snapshotting.'); + } + return AutoSnapshottingConfig( + thresholdMb: _parseKey(_Keys.thresholdMb, items, 7000), + increaseMb: _parseKey(_Keys.increaseMb, items, 500), + directory: items[_Keys.dir]!, + directorySizeLimitMb: _parseKey(_Keys.dirLimitMb, items, 30000), + minDelayBetweenSnapshots: Duration( + seconds: _parseKey(_Keys.delaySec, items, 20), + ), + ); +} + +int _parseKey(_Keys key, Map<_Keys, String> items, int defaultValue) { + final value = items[key]; + if (value == null || value.trim().isEmpty) return defaultValue; + final result = int.tryParse(value); + if (result == null) { + throw ArgumentError( + 'Invalid auto-snapshotting value for ${key.name}: $value.'); + } + return result; +} + +enum _Keys { + thresholdMb, + increaseMb, + dir, + dirLimitMb, + delaySec, +} diff --git a/pkg/analysis_server/pubspec.yaml b/pkg/analysis_server/pubspec.yaml index c7983da0c92..fa70ec81466 100644 --- a/pkg/analysis_server/pubspec.yaml +++ b/pkg/analysis_server/pubspec.yaml @@ -16,6 +16,7 @@ dependencies: crypto: any dart_style: any http: any + leak_tracker: any linter: any meta: any path: any diff --git a/pkg/analysis_server/test/test_all.dart b/pkg/analysis_server/test/test_all.dart index bdc3ae4f66d..155d6142d13 100644 --- a/pkg/analysis_server/test/test_all.dart +++ b/pkg/analysis_server/test/test_all.dart @@ -26,6 +26,7 @@ import 'services/test_all.dart' as services; import 'socket_server_test.dart' as socket_server; import 'src/test_all.dart' as src; import 'tool/test_all.dart' as tool; +import 'utilities/test_all.dart' as utilities; import 'verify_error_fix_status_test.dart' as verify_error_fix_status; import 'verify_no_solo_test.dart' as verify_no_solo; import 'verify_sorted_test.dart' as verify_sorted; @@ -54,6 +55,7 @@ void main() { socket_server.main(); src.main(); tool.main(); + utilities.main(); verify_error_fix_status.main(); verify_no_solo.main(); verify_sorted.main(); diff --git a/pkg/analysis_server/test/utilities/test_all.dart b/pkg/analysis_server/test/utilities/test_all.dart new file mode 100644 index 00000000000..03256a01379 --- /dev/null +++ b/pkg/analysis_server/test/utilities/test_all.dart @@ -0,0 +1,13 @@ +// 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 'package:test_reflective_loader/test_reflective_loader.dart'; + +import 'usage_tracking/test_all.dart' as usage_tracking; + +void main() { + defineReflectiveSuite(() { + usage_tracking.main(); + }, name: 'utilities'); +} diff --git a/pkg/analysis_server/test/utilities/usage_tracking/test_all.dart b/pkg/analysis_server/test/utilities/usage_tracking/test_all.dart new file mode 100644 index 00000000000..6a8a1907d6c --- /dev/null +++ b/pkg/analysis_server/test/utilities/usage_tracking/test_all.dart @@ -0,0 +1,13 @@ +// 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 'package:test_reflective_loader/test_reflective_loader.dart'; + +import 'usage_tracking_test.dart' as usage_tracking; + +void main() { + defineReflectiveSuite(() { + usage_tracking.main(); + }, name: 'usage_tracking'); +} diff --git a/pkg/analysis_server/test/utilities/usage_tracking/usage_tracking_test.dart b/pkg/analysis_server/test/utilities/usage_tracking/usage_tracking_test.dart new file mode 100644 index 00000000000..3aafd2fb76c --- /dev/null +++ b/pkg/analysis_server/test/utilities/usage_tracking/usage_tracking_test.dart @@ -0,0 +1,44 @@ +// 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 'package:analysis_server/src/utilities/usage_tracking/usage_tracking.dart'; +import 'package:test/test.dart'; + +void main() { + group('parseAutoSnapshottingConfig', () { + test('parses correct config', () { + final config = parseAutoSnapshottingConfig(_argsWithSnapshotting)!; + expect(config.thresholdMb, 200); + expect(config.increaseMb, 100); + expect(config.directory, '/Users/polinach/Downloads/analyzer_snapshots'); + expect(config.directorySizeLimitMb, 10000); + expect(config.minDelayBetweenSnapshots, Duration(seconds: 20)); + }); + test('returns null for no config', () { + final config = parseAutoSnapshottingConfig(_argsNoSnapshotting); + expect(config, null); + }); + test('throws for wrong config', () { + final wrongAutosnapshottingArg = + '--autosnapshotting--wrong-configuration'; + expect( + () => parseAutoSnapshottingConfig( + [wrongAutosnapshottingArg, 'some other arg']), + throwsA(isA()), + ); + }); + }); +} + +const _argsNoSnapshotting = [ + '--sdk=C:/b/s/w/ir/x/w/sdk/sdk/', + '--train-using=C:/b/s/w/ir/x/w/sdk/pkg/compiler/lib' +]; +const _argsWithSnapshotting = [ + _autosnapshottingArg, + '--sdk=C:/b/s/w/ir/x/w/sdk/sdk/', + '--train-using=C:/b/s/w/ir/x/w/sdk/pkg/compiler/lib' +]; +// This constant is referenced in README.md for auto-snapshotting. +const _autosnapshottingArg = + '--autosnapshotting=thresholdMb=200,increaseMb=100,dir=/Users/polinach/Downloads/analyzer_snapshots,dirLimitMb=10000,delaySec=20';