Skip to content

Commit a9d2363

Browse files
jensjohaCommit Queue
authored andcommitted
[analyzer] Don't collect analytics if not using it; cache getTopLevelDeclarations results in DartFixContext
Sometimes - on responding to `edit.getFixes` requests, the method `getTopLevelDeclarations` is called several times with the same parameter and caching has then sometimes sped things up. I then also noticed that it caused `reportAnalysisAnalytics` to be called many times (e.g. 73 times for one `edit.getFixes` request) each time spending time collecting data which - on passing to `analyticsManager.analysisComplete` - was just thrown away because it only saves the first one. Combined these changes have been observed to reduce the response time on `edit.getFixes` by up (down?) to ~20% (e.g. from 5 seconds to 1 second). In response to http://b/407797012 where more data is also available. Unfortunately I haven't been able to reproduce this is a benchmark (yet anyway). I don't know if it needs slower slower reads, an extreme amount of files, several contexts or something else entirely. Change-Id: Id214b0e1b7d8bbef7f6d956408f17fab45618b78 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/420323 Reviewed-by: Phil Quitslund <[email protected]> Commit-Queue: Johnni Winther <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent 8085a97 commit a9d2363

File tree

3 files changed

+18
-1
lines changed

3 files changed

+18
-1
lines changed

pkg/analysis_server/lib/src/analysis_server.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,9 @@ abstract class AnalysisServer {
917917
/// Report analytics data related to the number and size of files that were
918918
/// analyzed.
919919
void reportAnalysisAnalytics() {
920+
if (!analyticsManager.needsAnslysisCompleteCall()) {
921+
return;
922+
}
920923
var packagesFileMap = <String, File?>{};
921924
var immediateFileCount = 0;
922925
var immediateFileLineCount = 0;

pkg/analysis_server/lib/src/analytics/analytics_manager.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,8 @@ class AnalyticsManager {
220220
requestData.addValue(openWorkspacePathsKey, openWorkspacePaths.length);
221221
}
222222

223+
bool needsAnslysisCompleteCall() => _contextStructure == null;
224+
223225
Future<void> sendMemoryUsage(MemoryUsageEvent event) async {
224226
var delta = event.delta;
225227
var seconds = event.period?.inSeconds;

pkg/analysis_server_plugin/lib/edit/fix/dart_fix_context.dart

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ class DartFixContext implements FixContext {
4040
/// The workspace in which the fix contributor operates.
4141
final ChangeWorkspace workspace;
4242

43+
/// Cache of previously computed [getTopLevelDeclarations] responses.
44+
///
45+
/// It's been observed that the same request is fired multiple times for at
46+
/// least some getFixes requsts. Caching the response can speed up such
47+
/// requests.
48+
final Map<String, Future<Map<LibraryElement2, Element2>>>
49+
_cachedTopLevelDeclarations = {};
50+
4351
@override
4452
final AnalysisError error;
4553

@@ -58,7 +66,11 @@ class DartFixContext implements FixContext {
5866
///
5967
/// For getters and setters the corresponding top-level variable is returned.
6068
Future<Map<LibraryElement2, Element2>> getTopLevelDeclarations(String name) {
61-
return TopLevelDeclarations(unitResult).withName(name);
69+
var cachedResult = _cachedTopLevelDeclarations[name];
70+
if (cachedResult != null) return cachedResult;
71+
var result = TopLevelDeclarations(unitResult).withName(name);
72+
_cachedTopLevelDeclarations[name] = result;
73+
return result;
6274
}
6375

6476
/// Returns libraries with extensions that declare non-static public

0 commit comments

Comments
 (0)