[analyzer] Don't ask for subtypes for the same elements several times when searching for references

When asking for all references of a member it starts by finding all
supertypes for the class, then for each supertype (including itself)
(that has a member with the same name as the one we're querying for)
it searches for all subtypes.

We recall that when getting all subtypes, it really gets all direct
subtypes, then for each of those, their direct subtypes etc.
Naturally, then, with getting all subtypes for all supertypes we
will often ask for all direct subtypes of the same class several times.

In fact, it's O(n^2):

As an example, if asking for references to `foo` on this
(in a file in analyzer, with 8 workspaces):
```
class X0 { void foo() { print('hello'); } }
class X1 extends X0 { void foo() { print('hello'); } }
class X2 extends X1 { void foo() { print('hello'); } }
[...]
class X149 extends X148 { void foo() { print('hello'); } }
```
we ask for subtypes 90600 (150 classes * 151 / 2 * 8 workspaces) times.

This CL stops that from happening and in the example from above we
"only" asks for subtypes 1200 (150 classes * 8 workspaces) times.

For the `newFile` from `AbstractSingleUnitTest` example, for instance,
we go from 28,264 asks to 17,432 asks (a ~38% reduction).

Non-first runtimes when asking for references:

Before:
0:00:03.074853
0:00:03.021881
0:00:03.034707
0:00:03.115596
0:00:03.032574

After:
0:00:02.223978
0:00:02.149937
0:00:02.150236
0:00:02.104704
0:00:02.175859

Difference at 95.0% confidence
        -0.894979 +/- 0.060283
        -29.2867% +/- 1.97266%
        (Student's t, pooled s = 0.0413338)

Change-Id: Id792a595e74de01c7186ab1263c38728f051f603
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/272623
Commit-Queue: Jens Johansen <jensj@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Jens Johansen 2022-12-05 08:03:24 +00:00 committed by Commit Queue
parent a464fc354b
commit b812517fac
8 changed files with 109 additions and 51 deletions

View file

@ -61,10 +61,11 @@ class ImplementationHandler
}
final needsMember = helper.findMemberElement(interfaceElement) != null;
Set<InterfaceElement> allSubtypes = await performance.runAsync(
"searchAllSubtypes",
var allSubtypes = <InterfaceElement>{};
await performance.runAsync(
"appendAllSubtypes",
(performance) => server.searchEngine
.searchAllSubtypes(interfaceElement, performance: performance));
.appendAllSubtypes(interfaceElement, allSubtypes, performance));
final locations = performance.run(
"filter and get location",
(_) => allSubtypes

View file

@ -20,6 +20,7 @@ import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/dart/analysis/session_helper.dart';
import 'package:analyzer/src/generated/java_core.dart';
import 'package:analyzer/src/generated/source.dart';
import 'package:analyzer/src/util/performance/operation_performance.dart';
/// Checks if creating a method with the given [name] in [interfaceElement] will
/// cause any conflicts.
@ -275,7 +276,9 @@ class _CreateClassMemberValidator extends _BaseClassMemberValidator {
Future<RefactoringStatus> validate() async {
_checkClassAlreadyDeclares();
// do chained computations
var subClasses = await searchEngine.searchAllSubtypes(interfaceElement);
var subClasses = <InterfaceElement>{};
await searchEngine.appendAllSubtypes(
interfaceElement, subClasses, OperationPerformanceImpl("<root>"));
// check shadowing of class names
if (interfaceElement.name == name) {
result.addError(
@ -363,7 +366,9 @@ class _RenameClassMemberValidator extends _BaseClassMemberValidator {
_checkClassAlreadyDeclares();
// do chained computations
await _prepareReferences();
var subClasses = await searchEngine.searchAllSubtypes(interfaceElement);
var subClasses = <InterfaceElement>{};
await searchEngine.appendAllSubtypes(
interfaceElement, subClasses, OperationPerformanceImpl("<root>"));
// check shadowing of class names
for (var element in elements) {
var enclosingElement = element.enclosingElement;

View file

@ -105,23 +105,25 @@ Future<Set<ClassMemberElement>> getHierarchyMembers(
...enclosingElement.allSupertypes.map((e) => e.element),
enclosingElement,
];
var subClasses = <InterfaceElement>{};
for (var superClass in searchClasses) {
// ignore if super- class does not declare member
if (getClassMembers(superClass, name).isEmpty) {
continue;
}
// check all sub- classes
var subClasses = await performance.runAsync(
"searchAllSubtypes",
(performance) => searchEngine.searchAllSubtypes(superClass,
performance: performance));
await performance.runAsync(
"appendAllSubtypes",
(performance) => searchEngine.appendAllSubtypes(
superClass, subClasses, performance));
subClasses.add(superClass);
for (var subClass in subClasses) {
var subClassMembers = getChildren(subClass, name);
for (var member in subClassMembers) {
if (member is ClassMemberElement) {
result.add(member);
}
}
for (var subClass in subClasses) {
var subClassMembers = getChildren(subClass, name);
for (var member in subClassMembers) {
if (member is ClassMemberElement) {
result.add(member);
}
}
}

View file

@ -70,17 +70,20 @@ class MatchKind {
/// The interface [SearchEngine] defines the behavior of objects that can be
/// used to search for various pieces of information.
abstract class SearchEngine {
/// Adds all subtypes of the given [type] into [allSubtypes].
///
/// If [allSubtypes] already contains an element it is assumed that it
/// contains the entire subtree and the element won't be search on further.
///
/// [type] - the [InterfaceElement] being subtyped by the found matches.
Future<void> appendAllSubtypes(InterfaceElement type,
Set<InterfaceElement> allSubtypes, OperationPerformanceImpl performance);
/// If the [type] has subtypes, return the set of names of members which these
/// subtypes declare, possibly empty. If the [type] does not have subtypes,
/// return `null`.
Future<Set<String>?> membersOfSubtypes(InterfaceElement type);
/// Returns all subtypes of the given [type].
///
/// [type] - the [InterfaceElement] being subtyped by the found matches.
Future<Set<InterfaceElement>> searchAllSubtypes(InterfaceElement type,
{OperationPerformanceImpl? performance});
/// Returns declarations of class members with the given name.
///
/// [name] - the name being declared by the found matches.

View file

@ -17,6 +17,29 @@ class SearchEngineImpl implements SearchEngine {
SearchEngineImpl(this._drivers);
@override
Future<void> appendAllSubtypes(
InterfaceElement type,
Set<InterfaceElement> allSubtypes,
OperationPerformanceImpl performance) async {
var searchEngineCache = SearchEngineCache();
Future<void> addSubtypes(InterfaceElement type) async {
var directResults = await performance.runAsync(
"_searchDirectSubtypes",
(performance) =>
_searchDirectSubtypes(type, searchEngineCache, performance));
for (var directResult in directResults) {
var directSubtype = directResult.enclosingElement as InterfaceElement;
if (allSubtypes.add(directSubtype)) {
await addSubtypes(directSubtype);
}
}
}
await addSubtypes(type);
}
@override
Future<Set<String>?> membersOfSubtypes(InterfaceElement type) async {
var drivers = _drivers.toList();
@ -53,33 +76,6 @@ class SearchEngineImpl implements SearchEngine {
return members;
}
@override
Future<Set<InterfaceElement>> searchAllSubtypes(
InterfaceElement type, {
// TODO(jensj): Possibly make this required.
OperationPerformanceImpl? performance,
}) async {
var nnPerformance = performance ?? OperationPerformanceImpl("<root>");
var allSubtypes = <InterfaceElement>{};
var searchEngineCache = SearchEngineCache();
Future<void> addSubtypes(InterfaceElement type) async {
var directResults = await nnPerformance.runAsync(
"_searchDirectSubtypes",
(performance) =>
_searchDirectSubtypes(type, searchEngineCache, performance));
for (var directResult in directResults) {
var directSubtype = directResult.enclosingElement as InterfaceElement;
if (allSubtypes.add(directSubtype)) {
await addSubtypes(directSubtype);
}
}
}
await addSubtypes(type);
return allSubtypes;
}
@override
Future<List<SearchMatch>> searchMemberDeclarations(String name) async {
var allDeclarations = <SearchMatch>[];

View file

@ -5,6 +5,7 @@
import 'package:analysis_server/src/services/search/hierarchy.dart';
import 'package:analysis_server/src/services/search/search_engine_internal.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/util/performance/operation_performance.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
@ -144,6 +145,43 @@ class C extends B {
}
}
Future<void> test_getHierarchyMembers_linear_number_of_calls() async {
const count = 150;
const last = count - 1;
StringBuffer sb = StringBuffer();
for (int i = 0; i < count; i++) {
if (i == 0) {
sb.writeln("class X0 { void foo() { print('hello'); } }");
} else {
sb.writeln(
"class X$i extends X${i - 1} { void foo() { print('hello'); } }");
}
}
await _indexTestUnit(sb.toString());
var classLast = findElement.class_('X$last');
ClassMemberElement member =
classLast.methods.where((element) => element.name == "foo").single;
OperationPerformanceImpl performance = OperationPerformanceImpl("<root>");
var result = await performance.runAsync(
"getHierarchyMembers",
(performance) => getHierarchyMembers(searchEngine, member,
performance: performance));
expect(result, hasLength(count));
var worklist = <OperationPerformance>[];
worklist.add(performance);
while (worklist.isNotEmpty) {
var performance = worklist.removeLast();
expect(
performance.count,
lessThanOrEqualTo(count + 1),
reason: performance.toString(),
);
worklist.addAll(performance.children);
}
}
Future<void> test_getHierarchyMembers_methods() async {
await _indexTestUnit('''
class A {

View file

@ -9,6 +9,7 @@ import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/src/test_utilities/find_element.dart';
import 'package:analyzer/src/test_utilities/find_node.dart';
import 'package:analyzer/src/test_utilities/package_config_file_builder.dart';
import 'package:analyzer/src/util/performance/operation_performance.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
@ -209,7 +210,9 @@ class C implements B {}
var element = findElement.class_('T');
var subtypes = await searchEngine.searchAllSubtypes(element);
var subtypes = <InterfaceElement>{};
await searchEngine.appendAllSubtypes(
element, subtypes, OperationPerformanceImpl("<root>"));
expect(subtypes, hasLength(3));
_assertContainsClass(subtypes, 'A');
_assertContainsClass(subtypes, 'B');
@ -233,7 +236,9 @@ class C extends B {}
await resolveFile2('$aaaRootPath/lib/a.dart');
var element = findElement.class_('T');
var subtypes = await searchEngine.searchAllSubtypes(element);
var subtypes = <InterfaceElement>{};
await searchEngine.appendAllSubtypes(
element, subtypes, OperationPerformanceImpl("<root>"));
expect(subtypes, hasLength(3));
_assertContainsClass(subtypes, 'A');
_assertContainsClass(subtypes, 'B');
@ -255,7 +260,9 @@ mixin E implements C {}
var element = findElement.class_('T');
var subtypes = await searchEngine.searchAllSubtypes(element);
var subtypes = <InterfaceElement>{};
await searchEngine.appendAllSubtypes(
element, subtypes, OperationPerformanceImpl("<root>"));
expect(subtypes, hasLength(5));
_assertContainsClass(subtypes, 'A');
_assertContainsClass(subtypes, 'B');

View file

@ -9,6 +9,9 @@ abstract class OperationPerformance {
/// The child operations, might be empty.
List<OperationPerformance> get children;
/// The number of times this child has been started/run.
int get count;
/// The data attachments, for non-timing data, e.g. how many files were read,
/// or how many bytes were processed.
List<OperationPerformanceData> get data;
@ -84,6 +87,9 @@ class OperationPerformanceImpl implements OperationPerformance {
return _children;
}
@override
int get count => _count;
@override
List<OperationPerformanceData<Object>> get data {
return _data.values.toList();