From dba07581203717d99b6ea0fafb35e27cd2aeda26 Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Wed, 17 May 2023 07:43:26 +0000 Subject: [PATCH] Revert "Add auto-snapshotting and analytics for memory usage." This reverts commit 5f1e7ade45bf3a6bd8deecdec5c6247b7eddc971. Reason for revert: bots are failing Original change's description: > Add auto-snapshotting and analytics for memory usage. > > See video linked to issue: https://github.com/flutter/devtools/issues/5606 > > Change-Id: I9f22031871e30bc7160e2c49b0423fb64df62223 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/300862 > Reviewed-by: Samuel Rawlins > Reviewed-by: Jacob Richman Change-Id: I54cb3943230ab1229cc8b2c9df40e9c71765df19 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/303881 Auto-Submit: Polina Cherkasova Reviewed-by: Polina Cherkasova Reviewed-by: Brian Wilkerson Reviewed-by: Samuel Rawlins Commit-Queue: Daco Harkes --- DEPS | 2 +- .../lib/src/analytics/analytics_manager.dart | 23 ----- .../lib/src/server/driver.dart | 7 -- .../usage_tracking/AUTOSNAPSHOTTING.md | 50 ---------- .../usage_tracking/usage_tracking.dart | 97 ------------------- pkg/analysis_server/pubspec.yaml | 1 - .../usage_tracking/usage_tracking_test.dart | 51 ---------- 7 files changed, 1 insertion(+), 230 deletions(-) delete mode 100644 pkg/analysis_server/lib/src/utilities/usage_tracking/AUTOSNAPSHOTTING.md delete mode 100644 pkg/analysis_server/lib/src/utilities/usage_tracking/usage_tracking.dart delete mode 100644 pkg/analysis_server/test/utilities/usage_tracking/usage_tracking_test.dart diff --git a/DEPS b/DEPS index 9a5b5065c88..853252e35dd 100644 --- a/DEPS +++ b/DEPS @@ -173,7 +173,7 @@ vars = { "test_descriptor_rev": "23e49a21fc6b4bf3164d336c699b29d1b8bb4622", "test_process_rev": "b6a6cd5f598250c71d8deeea3d38eb821af5e932", "test_reflective_loader_rev": "d1b763f6281a46a48e1da6f0a6e8152e3480e8d2", - "tools_rev": "49da4cabaddec3c82485b15d83ddac2278f947db", + "tools_rev": "62c96040d8090bc1bb866db75d40bd1707876cf9", "typed_data_rev": "921f5c0380c6c487d8a065c45618162efa4cbd92", "usage_rev": "929a4e31f0bd4f861dd0e34d4c6f7184c751b569", "vector_math_rev": "e3de8da3e7db3b9b4f56a16061e9e480539fb08c", diff --git a/pkg/analysis_server/lib/src/analytics/analytics_manager.dart b/pkg/analysis_server/lib/src/analytics/analytics_manager.dart index c2eedd84b91..a283aa11f3e 100644 --- a/pkg/analysis_server/lib/src/analytics/analytics_manager.dart +++ b/pkg/analysis_server/lib/src/analytics/analytics_manager.dart @@ -20,7 +20,6 @@ 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. @@ -191,28 +190,6 @@ 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 08f4305002a..05a8dd9618a 100644 --- a/pkg/analysis_server/lib/src/server/driver.dart +++ b/pkg/analysis_server/lib/src/server/driver.dart @@ -41,8 +41,6 @@ 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. @@ -357,11 +355,6 @@ 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 deleted file mode 100644 index 90e0a66fab7..00000000000 --- a/pkg/analysis_server/lib/src/utilities/usage_tracking/AUTOSNAPSHOTTING.md +++ /dev/null @@ -1,50 +0,0 @@ -# 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 deleted file mode 100644 index 2f658a7dcff..00000000000 --- a/pkg/analysis_server/lib/src/utilities/usage_tracking/usage_tracking.dart +++ /dev/null @@ -1,97 +0,0 @@ -// 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 fa70ec81466..c7983da0c92 100644 --- a/pkg/analysis_server/pubspec.yaml +++ b/pkg/analysis_server/pubspec.yaml @@ -16,7 +16,6 @@ dependencies: crypto: any dart_style: any http: any - leak_tracker: any linter: any meta: any path: any 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 deleted file mode 100644 index 4d5dedf803b..00000000000 --- a/pkg/analysis_server/test/utilities/usage_tracking/usage_tracking_test.dart +++ /dev/null @@ -1,51 +0,0 @@ -// 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, 6000); - expect(config.increaseMb, 500); - expect(config.directory, '/Users/polinach/Downloads/analyzer_snapshots'); - expect(config.directorySizeLimitMb, 30000); - 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=6000,increaseMb=500,dir=/Users/polinach/Downloads/analyzer_snapshots,dirLimitMb=30000,delaySec=20';