Skip to content

Commit 2f8a59a

Browse files
jensjohaCommit Queue
authored andcommitted
[analyzer] Fix duplicated excluded globs
With 39c60bc (https://dart-review.googlesource.com/c/sdk/+/464762) it suddenly took more than 7 minutes to start the analysis server on my machine. Turns out it was caused by duplicated exclude globs. This should fix the issue (and avoid some traversing of the filesystem) and takes startup back to a more reasonable time. By applying this patch: ``` diff --git a/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart b/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart index 72a821c..0c7f8ead5d2 100644 --- a/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart +++ b/pkg/analyzer/lib/src/dart/analysis/analysis_context_collection.dart @@ -101,6 +101,10 @@ class AnalysisContextCollectionImpl implements AnalysisContextCollection { optionsFile: optionsFile, packagesFile: packagesFile, ); + for(var root in roots) { + // ignore: avoid_dynamic_calls + print("Root with ${(root as dynamic).excludedGlobs.length} globs"); + } byteStore ??= MemoryByteStore(); ``` and running ``` out/ReleaseX64/dart-sdk/bin/dart pkg/analysis_server/bin/server.dart --train-using pkg/front_end ``` so I can see the number of excluded globs in each root, before this CL it printed `Root with 19958 globs`. With this CL it prins `Root with 34 globs` (which it also did before 39c60bc). Change-Id: Ie0b5edc7eede4c28394cd3d20334f9f1ad8417d3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/465920 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]>
1 parent 6b72a9c commit 2f8a59a

File tree

2 files changed

+54
-13
lines changed

2 files changed

+54
-13
lines changed

pkg/analyzer/lib/src/dart/analysis/context_locator.dart

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -404,15 +404,19 @@ class ContextLocatorImpl {
404404
Set<String> containingRootEnabledLegacyPlugins,
405405
List<LocatedGlob> excludedGlobs,
406406
File? optionsFile,
407-
File? packagesFile,
408-
) {
407+
File? packagesFile, {
408+
File? optionsFileFromParentInSameRoot,
409+
}) {
409410
var packagesFileToUse =
410411
packagesFile ?? _getPackagesFile(folder) ?? containingRoot.packagesFile;
411412
var buildGnFile = folder.getExistingFile(file_paths.buildGn);
412413

413414
var optionsFileToUse = optionsFile;
414415
if (optionsFileToUse == null) {
415416
optionsFileToUse = folder.existingAnalysisOptionsYamlFile;
417+
// If this folder doesn't have one use the one from a parent folder if any,
418+
// that will be the one we find anyway.
419+
optionsFileToUse ??= optionsFileFromParentInSameRoot;
416420
if (optionsFileToUse == null) {
417421
var parentFolder = folder.parent;
418422
while (parentFolder != containingRoot.root) {
@@ -470,12 +474,14 @@ class ContextLocatorImpl {
470474
if (optionsFileToUse != null) {
471475
(containingRoot as ContextRootImpl).optionsFileMap[folder] =
472476
optionsFileToUse;
473-
// Add excluded globs.
474-
var excludes = _getExcludedGlobs(
475-
optionsFileToUse,
476-
containingRoot.workspace,
477-
);
478-
containingRoot.excludedGlobs.addAll(excludes);
477+
if (optionsFileToUse != optionsFileFromParentInSameRoot) {
478+
// Add excluded globs only if we found a new options file.
479+
var excludes = _getExcludedGlobs(
480+
optionsFileToUse,
481+
containingRoot.workspace,
482+
);
483+
containingRoot.excludedGlobs.addAll(excludes);
484+
}
479485
}
480486
_createContextRootsIn(
481487
roots,
@@ -487,6 +493,7 @@ class ContextLocatorImpl {
487493
excludedGlobs,
488494
optionsFile,
489495
packagesFile,
496+
optionsFileToUseForFolder: usedThisRoot ? optionsFileToUse : null,
490497
);
491498

492499
return usedThisRoot;
@@ -507,8 +514,9 @@ class ContextLocatorImpl {
507514
Set<String> containingRootEnabledLegacyPlugins,
508515
List<LocatedGlob> excludedGlobs,
509516
File? optionsFile,
510-
File? packagesFile,
511-
) {
517+
File? packagesFile, {
518+
File? optionsFileToUseForFolder,
519+
}) {
512520
bool isExcluded(Folder folder) {
513521
if (excludedFolders.contains(folder) ||
514522
folder.shortName.startsWith('.')) {
@@ -553,6 +561,7 @@ class ContextLocatorImpl {
553561
excludedGlobs,
554562
optionsFile,
555563
packagesFile,
564+
optionsFileFromParentInSameRoot: optionsFileToUseForFolder,
556565
);
557566
}
558567
}

pkg/analyzer/test/src/dart/analysis/analysis_context_collection_test.dart

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'package:analyzer/dart/analysis/features.dart';
66
import 'package:analyzer/file_system/file_system.dart';
77
import 'package:analyzer/src/dart/analysis/analysis_context_collection.dart';
88
import 'package:analyzer/src/dart/analysis/analysis_options.dart';
9+
import 'package:analyzer/src/dart/analysis/context_root.dart';
910
import 'package:analyzer/src/dart/analysis/driver_based_analysis_context.dart';
1011
import 'package:analyzer/src/dart/analysis/experiments.dart';
1112
import 'package:analyzer/src/dart/analysis/file_state.dart';
@@ -641,7 +642,11 @@ name: test
641642
name: 'test',
642643
);
643644

644-
newAnalysisOptionsYamlFile(testPackageRootPath, '');
645+
newAnalysisOptionsYamlFile(testPackageRootPath, r'''
646+
analyzer:
647+
exclude:
648+
- foo/**
649+
''');
645650
newFile('$testPackageLibPath/a.dart', '');
646651

647652
var nestedPath = '$testPackageLibPath/nested';
@@ -662,7 +667,14 @@ analyzer:
662667
newFile('$nestedNestedPath/c.dart', '');
663668
newFile('$nestedNestedPath/excluded2/d.dart', '');
664669

665-
_assertWorkspaceCollectionText(workspaceRootPath, r'''
670+
// There's still an issue here with these exclude globs being there twice:
671+
// - foo/** in /home/test
672+
// - foo in /home/test
673+
// But at least that's it.
674+
_assertWorkspaceCollectionText(
675+
workspaceRootPath,
676+
withExcludedGlobs: true,
677+
r'''
666678
contexts
667679
/home/test
668680
packagesFile: /home/test/.dart_tool/package_config.json
@@ -680,6 +692,15 @@ contexts
680692
uri: package:test/nested/nested/c.dart
681693
analysisOptions_2
682694
workspacePackage_0_0
695+
excludedGlobs
696+
foo/** in /home/test
697+
foo in /home/test
698+
foo/** in /home/test
699+
foo in /home/test
700+
excluded/** in /home/test/lib/nested
701+
excluded in /home/test/lib/nested
702+
excluded2/** in /home/test/lib/nested/nested
703+
excluded2 in /home/test/lib/nested/nested
683704
analysisOptions
684705
analysisOptions_0: /home/test/analysis_options.yaml
685706
analysisOptions_1: /home/test/lib/nested/analysis_options.yaml
@@ -690,7 +711,8 @@ workspaces
690711
pubPackages
691712
workspacePackage_0_0: PubPackage
692713
root: /home/test
693-
''');
714+
''',
715+
);
694716
}
695717

696718
test_packageConfigWorkspace_multipleAnalysisOptions_outerExclude() async {
@@ -1686,13 +1708,15 @@ workspaces
16861708
void _assertWorkspaceCollectionText(
16871709
String workspaceRootPath,
16881710
String expected, {
1711+
bool withExcludedGlobs = false,
16891712
File? optionsFile,
16901713
void Function({required AnalysisOptionsImpl analysisOptions})?
16911714
updateAnalysisOptions,
16921715
}) {
16931716
if (optionsFile != null) {
16941717
expect(optionsFile.exists, isTrue);
16951718
}
1719+
configuration.withExcludedGlobs = withExcludedGlobs;
16961720
var collection = AnalysisContextCollectionImpl(
16971721
resourceProvider: resourceProvider,
16981722
sdkPath: sdkRoot.path,
@@ -1809,6 +1833,13 @@ class _AnalysisContextCollectionPrinter {
18091833
_writeDartFile(fsState, file);
18101834
}
18111835
});
1836+
if (configuration.withExcludedGlobs && contextRoot is ContextRootImpl) {
1837+
sink.writeElements('excludedGlobs', contextRoot.excludedGlobs, (glob) {
1838+
sink.writelnWithIndent(
1839+
'${glob.glob.pattern} in ${glob.parent.posixPath}',
1840+
);
1841+
});
1842+
}
18121843
});
18131844
}
18141845

@@ -1944,4 +1975,5 @@ class _AnalysisContextCollectionPrinterConfiguration {
19441975
bool withEnabledFeatures = false;
19451976
bool withLintRules = false;
19461977
bool withOptionFilesForContext = false;
1978+
bool withExcludedGlobs = false;
19471979
}

0 commit comments

Comments
 (0)