Skip to content

Commit 07724a0

Browse files
johnniwintherCommit Queue
johnniwinther
authored and
Commit Queue
committed
[cfe] Optimize scope lookup for misses
This creates a fast path for when neither a getable nor setable is in the current scope. Change-Id: I57a07591e72737af6dbec30cff424a6df4ed5211 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/412280 Commit-Queue: Johnni Winther <[email protected]> Reviewed-by: Chloe Stefantsova <[email protected]>
1 parent 2b96b7e commit 07724a0

File tree

5 files changed

+133
-76
lines changed

5 files changed

+133
-76
lines changed

‎pkg/front_end/lib/src/base/local_scope.dart‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,13 @@ mixin LocalScopeMixin implements LookupScopeMixin, LocalScope {
7171
@override
7272
Builder? lookupGetable(String name, int charOffset, Uri fileUri) {
7373
_recordUse(name, charOffset);
74-
Builder? builder;
7574
if (_local != null) {
76-
builder = lookupGetableIn(name, charOffset, fileUri, _local!);
77-
if (builder != null) return builder;
75+
Builder? builder = lookupGetableIn(name, charOffset, fileUri, _local!);
76+
if (builder != null) {
77+
return builder;
78+
}
7879
}
79-
return builder ?? _parent?.lookupGetable(name, charOffset, fileUri);
80+
return _parent?.lookupGetable(name, charOffset, fileUri);
8081
}
8182

8283
@override
@@ -87,8 +88,13 @@ mixin LocalScopeMixin implements LookupScopeMixin, LocalScope {
8788
@override
8889
Builder? lookupSetable(String name, int charOffset, Uri fileUri) {
8990
_recordUse(name, charOffset);
90-
Builder? builder = lookupSetableIn(name, charOffset, fileUri, _local);
91-
return builder ?? _parent?.lookupSetable(name, charOffset, fileUri);
91+
if (_local != null) {
92+
Builder? builder = lookupSetableIn(name, charOffset, fileUri, _local);
93+
if (builder != null) {
94+
return builder;
95+
}
96+
}
97+
return _parent?.lookupSetable(name, charOffset, fileUri);
9298
}
9399

94100
void _recordUse(String name, int charOffset) {}

‎pkg/front_end/lib/src/base/scope.dart‎

Lines changed: 91 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,9 @@ Builder? normalizeLookup(
153153
required String classNameOrDebugName,
154154
required bool isSetter,
155155
bool forStaticAccess = false}) {
156+
if (getable == null && setable == null) {
157+
return null;
158+
}
156159
Builder? thisBuilder;
157160
Builder? otherBuilder;
158161
if (isSetter) {
@@ -230,8 +233,12 @@ mixin LookupScopeMixin implements LookupScope {
230233

231234
Builder? lookupGetableIn(
232235
String name, int charOffset, Uri fileUri, Map<String, Builder> getables) {
236+
Builder? getable = getables[name];
237+
if (getable == null) {
238+
return null;
239+
}
233240
return normalizeLookup(
234-
getable: getables[name],
241+
getable: getable,
235242
setable: null,
236243
name: name,
237244
charOffset: charOffset,
@@ -242,8 +249,13 @@ mixin LookupScopeMixin implements LookupScope {
242249

243250
Builder? lookupSetableIn(String name, int charOffset, Uri fileUri,
244251
Map<String, Builder>? getables) {
252+
Builder? getable = getables?[name];
253+
if (getable == null) {
254+
return null;
255+
}
256+
// Coverage-ignore(suite): Not run.
245257
return normalizeLookup(
246-
getable: getables?[name],
258+
getable: getable,
247259
setable: null,
248260
name: name,
249261
charOffset: charOffset,
@@ -271,27 +283,37 @@ abstract class BaseNameSpaceLookupScope implements LookupScope {
271283

272284
@override
273285
Builder? lookupGetable(String name, int charOffset, Uri fileUri) {
274-
Builder? builder = normalizeLookup(
275-
getable: _nameSpace.lookupLocalMember(name, setter: false),
276-
setable: _nameSpace.lookupLocalMember(name, setter: true),
277-
name: name,
278-
charOffset: charOffset,
279-
fileUri: fileUri,
280-
classNameOrDebugName: classNameOrDebugName,
281-
isSetter: false);
286+
Builder? getable = _nameSpace.lookupLocalMember(name, setter: false);
287+
Builder? setable = _nameSpace.lookupLocalMember(name, setter: true);
288+
Builder? builder;
289+
if (getable != null || setable != null) {
290+
builder = normalizeLookup(
291+
getable: getable,
292+
setable: setable,
293+
name: name,
294+
charOffset: charOffset,
295+
fileUri: fileUri,
296+
classNameOrDebugName: classNameOrDebugName,
297+
isSetter: false);
298+
}
282299
return builder ?? _parent?.lookupGetable(name, charOffset, fileUri);
283300
}
284301

285302
@override
286303
Builder? lookupSetable(String name, int charOffset, Uri fileUri) {
287-
Builder? builder = normalizeLookup(
288-
getable: _nameSpace.lookupLocalMember(name, setter: false),
289-
setable: _nameSpace.lookupLocalMember(name, setter: true),
290-
name: name,
291-
charOffset: charOffset,
292-
fileUri: fileUri,
293-
classNameOrDebugName: classNameOrDebugName,
294-
isSetter: true);
304+
Builder? getable = _nameSpace.lookupLocalMember(name, setter: false);
305+
Builder? setable = _nameSpace.lookupLocalMember(name, setter: true);
306+
Builder? builder;
307+
if (getable != null || setable != null) {
308+
builder = normalizeLookup(
309+
getable: getable,
310+
setable: setable,
311+
name: name,
312+
charOffset: charOffset,
313+
fileUri: fileUri,
314+
classNameOrDebugName: classNameOrDebugName,
315+
isSetter: true);
316+
}
295317
return builder ?? _parent?.lookupSetable(name, charOffset, fileUri);
296318
}
297319

@@ -330,27 +352,36 @@ abstract class AbstractTypeParameterScope implements LookupScope {
330352

331353
@override
332354
Builder? lookupGetable(String name, int charOffset, Uri fileUri) {
333-
Builder? builder = normalizeLookup(
334-
getable: getTypeParameter(name),
335-
setable: null,
336-
name: name,
337-
charOffset: charOffset,
338-
fileUri: fileUri,
339-
classNameOrDebugName: classNameOrDebugName,
340-
isSetter: false);
355+
Builder? typeParameter = getTypeParameter(name);
356+
Builder? builder;
357+
if (typeParameter != null) {
358+
builder = normalizeLookup(
359+
getable: typeParameter,
360+
setable: null,
361+
name: name,
362+
charOffset: charOffset,
363+
fileUri: fileUri,
364+
classNameOrDebugName: classNameOrDebugName,
365+
isSetter: false);
366+
}
341367
return builder ?? _parent.lookupGetable(name, charOffset, fileUri);
342368
}
343369

344370
@override
345371
Builder? lookupSetable(String name, int charOffset, Uri fileUri) {
346-
Builder? builder = normalizeLookup(
347-
getable: getTypeParameter(name),
348-
setable: null,
349-
name: name,
350-
charOffset: charOffset,
351-
fileUri: fileUri,
352-
classNameOrDebugName: classNameOrDebugName,
353-
isSetter: true);
372+
Builder? typeParameter = getTypeParameter(name);
373+
Builder? builder;
374+
if (typeParameter != null) {
375+
// Coverage-ignore-block(suite): Not run.
376+
builder = normalizeLookup(
377+
getable: typeParameter,
378+
setable: null,
379+
name: name,
380+
charOffset: charOffset,
381+
fileUri: fileUri,
382+
classNameOrDebugName: classNameOrDebugName,
383+
isSetter: true);
384+
}
354385
return builder ?? _parent.lookupSetable(name, charOffset, fileUri);
355386
}
356387

@@ -404,27 +435,37 @@ class FixedLookupScope implements LookupScope {
404435

405436
@override
406437
Builder? lookupGetable(String name, int charOffset, Uri fileUri) {
407-
Builder? builder = normalizeLookup(
408-
getable: _getables?[name],
409-
setable: _setables?[name],
410-
name: name,
411-
charOffset: charOffset,
412-
fileUri: fileUri,
413-
classNameOrDebugName: classNameOrDebugName,
414-
isSetter: false);
438+
Builder? getable = _getables?[name];
439+
Builder? setable = _setables?[name];
440+
Builder? builder;
441+
if (getable != null || setable != null) {
442+
builder = normalizeLookup(
443+
getable: getable,
444+
setable: setable,
445+
name: name,
446+
charOffset: charOffset,
447+
fileUri: fileUri,
448+
classNameOrDebugName: classNameOrDebugName,
449+
isSetter: false);
450+
}
415451
return builder ?? _parent?.lookupGetable(name, charOffset, fileUri);
416452
}
417453

418454
@override
419455
Builder? lookupSetable(String name, int charOffset, Uri fileUri) {
420-
Builder? builder = normalizeLookup(
421-
getable: _getables?[name],
422-
setable: _setables?[name],
423-
name: name,
424-
charOffset: charOffset,
425-
fileUri: fileUri,
426-
classNameOrDebugName: classNameOrDebugName,
427-
isSetter: true);
456+
Builder? getable = _getables?[name];
457+
Builder? setable = _setables?[name];
458+
Builder? builder;
459+
if (getable != null || setable != null) {
460+
builder = normalizeLookup(
461+
getable: getable,
462+
setable: setable,
463+
name: name,
464+
charOffset: charOffset,
465+
fileUri: fileUri,
466+
classNameOrDebugName: classNameOrDebugName,
467+
isSetter: true);
468+
}
428469
return builder ?? _parent?.lookupSetable(name, charOffset, fileUri);
429470
}
430471

‎pkg/front_end/lib/src/builder/builder_mixins.dart‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,20 @@ mixin DeclarationBuilderMixin implements IDeclarationBuilder {
2828
name.startsWith("_")) {
2929
return null;
3030
}
31-
Builder? declaration = normalizeLookup(
32-
getable: nameSpace.lookupLocalMember(name, setter: false),
33-
setable: nameSpace.lookupLocalMember(name, setter: true),
34-
name: name,
35-
charOffset: charOffset,
36-
fileUri: fileUri,
37-
classNameOrDebugName: this.name,
38-
isSetter: isSetter,
39-
forStaticAccess: true);
31+
Builder? getable = nameSpace.lookupLocalMember(name, setter: false);
32+
Builder? setable = nameSpace.lookupLocalMember(name, setter: true);
33+
Builder? declaration;
34+
if (getable != null || setable != null) {
35+
declaration = normalizeLookup(
36+
getable: getable,
37+
setable: setable,
38+
name: name,
39+
charOffset: charOffset,
40+
fileUri: fileUri,
41+
classNameOrDebugName: this.name,
42+
isSetter: isSetter,
43+
forStaticAccess: true);
44+
}
4045
// TODO(johnniwinther): Handle augmented extensions/extension type
4146
// declarations.
4247
return declaration;

‎pkg/front_end/lib/src/builder/class_builder.dart‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -192,15 +192,20 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
192192
name.startsWith("_")) {
193193
return null;
194194
}
195-
Builder? declaration = normalizeLookup(
196-
getable: nameSpace.lookupLocalMember(name, setter: false),
197-
setable: nameSpace.lookupLocalMember(name, setter: true),
198-
name: name,
199-
charOffset: fileOffset,
200-
fileUri: fileUri,
201-
classNameOrDebugName: this.name,
202-
isSetter: isSetter,
203-
forStaticAccess: true);
195+
Builder? getable = nameSpace.lookupLocalMember(name, setter: false);
196+
Builder? setable = nameSpace.lookupLocalMember(name, setter: true);
197+
Builder? declaration;
198+
if (getable != null || setable != null) {
199+
declaration = normalizeLookup(
200+
getable: getable,
201+
setable: setable,
202+
name: name,
203+
charOffset: fileOffset,
204+
fileUri: fileUri,
205+
classNameOrDebugName: this.name,
206+
isSetter: isSetter,
207+
forStaticAccess: true);
208+
}
204209
if (declaration == null && isAugmenting) {
205210
return origin.findStaticBuilder(
206211
name, fileOffset, fileUri, accessingLibrary,

‎pkg/front_end/test/coverage_suite_expected.dart‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
175175
),
176176
// 100.0%.
177177
"package:front_end/src/base/local_scope.dart": (
178-
hitCount: 59,
178+
hitCount: 60,
179179
missCount: 0,
180180
),
181181
// 100.0%.
@@ -210,7 +210,7 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
210210
),
211211
// 100.0%.
212212
"package:front_end/src/base/scope.dart": (
213-
hitCount: 674,
213+
hitCount: 670,
214214
missCount: 0,
215215
),
216216
// 100.0%.

0 commit comments

Comments
 (0)