Skip to content

Commit 9052ec7

Browse files
scheglovCommit Queue
authored andcommitted
Fine. Short-circuit requirement checks with hashForRequirements
Introduce a fast-path in fine-grained dependency validation by comparing a precomputed per-library hash before doing field-by-field checks. When the recorded hash matches the current LibraryManifest.hashForRequirements, we skip detailed validation for that library; otherwise we fall back to the existing thorough comparisons. Why --- Validating RequirementsManifest.isSatisfied() was spending time walking multiple fields and maps even when nothing changed. The new hash captures exactly the manifest parts that the validator reads, so equality implies the same outcome. This keeps correctness while dramatically reducing the common-case cost. What changed ------------ - Add LibraryRequirements.hashForRequirements and persist it in the binary format (read/write). - Initialize per-library requirements from the current manifest via LibraryRequirements.fromManifest(), capturing the hash and exportMapId. - Extend RequirementsManifest.isSatisfied() to: * short-circuit per-library checks when hashes match, * record lightweight perf counters ("libHash"/"libDetails" and "libsHash"/"libsDetails") via OperationPerformanceImpl. - Thread the new performance parameter through callers: * AnalysisDriver._getLibraryDiagnosticsBundle() * LibraryContext._getLinkedBundleEntry() - Bump AnalysisDriver.DATA_VERSION to 564 to invalidate stale caches. Performance ----------- - getLibraryDiagnosticsBundle elapsed time: 70.105 ms → 3.557 ms (−66.548 ms, −94.93%; ~19.71× faster) across 1,646 runs. - Counters (after): apiSignature=193, libDetails=8, libHash=17,916, libsDetails=7, libsHash=1,444. Correctness ----------- - The hash is computed from exactly the manifest fields that validation consults; equal hashes guarantee the same decision. - When hashes differ, full validation runs as before. - Export combinators, re-export flags, and opaque API handling are unchanged and still enforced. Impact ------ Expect faster cache hits when resolving/validating diagnostics bundles and linked cycles, with no behavioral changes when libraries actually differ. Change-Id: Id3a631a680d21a994d4277a2d8f98040159c094d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/451820 Reviewed-by: Paul Berry <[email protected]> Commit-Queue: Konstantin Shcheglov <[email protected]>
1 parent 4bc9a47 commit 9052ec7

File tree

3 files changed

+42
-8
lines changed

3 files changed

+42
-8
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ testFineAfterLibraryAnalyzerHook;
106106
// TODO(scheglov): Clean up the list of implicitly analyzed files.
107107
class AnalysisDriver {
108108
/// The version of data format, should be incremented on every format change.
109-
static const int DATA_VERSION = 563;
109+
static const int DATA_VERSION = 564;
110110

111111
/// The number of exception contexts allowed to write. Once this field is
112112
/// zero, we stop writing any new exception contexts in this process.
@@ -1925,6 +1925,7 @@ class AnalysisDriver {
19251925
var elementFactory = libraryContext.elementFactory;
19261926
var failure = bundle.requirements.isSatisfied(
19271927
elementFactory: elementFactory,
1928+
performance: performance,
19281929
);
19291930
scheduler.eventsController.add(
19301931
events.CheckLibraryDiagnosticsRequirements(

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,10 @@ class LibraryContext {
240240
}
241241

242242
var failure = performance.run('checkRequirements', (performance) {
243-
return entry.requirements.isSatisfied(elementFactory: elementFactory);
243+
return entry.requirements.isSatisfied(
244+
elementFactory: elementFactory,
245+
performance: performance,
246+
);
244247
});
245248

246249
eventsController?.add(

pkg/analyzer/lib/src/fine/requirements.dart

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import 'package:analyzer/src/fine/manifest_id.dart';
1616
import 'package:analyzer/src/fine/manifest_item.dart';
1717
import 'package:analyzer/src/fine/requirement_failure.dart';
1818
import 'package:analyzer/src/summary2/linked_element_factory.dart';
19+
import 'package:analyzer/src/util/performance/operation_performance.dart';
1920
import 'package:analyzer/src/utilities/extensions/string.dart';
2021
import 'package:collection/collection.dart';
2122
import 'package:meta/meta.dart';
@@ -490,6 +491,15 @@ class LibraryExportRequirements {
490491
}
491492

492493
class LibraryRequirements {
494+
/// Snapshot of [LibraryManifest.hashForRequirements] at recording.
495+
///
496+
/// This serves as a fast-path check in [RequirementsManifest.isSatisfied].
497+
/// If this hash matches the current hash on the corresponding
498+
/// [LibraryManifest], it implies that the relevant portions of the library
499+
/// have not changed, allowing the detailed, field-by-field requirement
500+
/// validation for this library to be skipped.
501+
String hashForRequirements;
502+
493503
String? name;
494504
bool? isSynthetic;
495505
Uint8List? featureSet;
@@ -547,6 +557,7 @@ class LibraryRequirements {
547557
ManifestItemIdList? allDeclaredSetters;
548558

549559
LibraryRequirements({
560+
required this.hashForRequirements,
550561
required this.name,
551562
required this.isSynthetic,
552563
required this.featureSet,
@@ -581,16 +592,17 @@ class LibraryRequirements {
581592
required this.allDeclaredSetters,
582593
});
583594

584-
factory LibraryRequirements.empty() {
595+
factory LibraryRequirements.fromManifest(LibraryManifest manifest) {
585596
return LibraryRequirements(
597+
hashForRequirements: manifest.hashForRequirements,
586598
name: null,
587599
isSynthetic: null,
588600
featureSet: null,
589601
languageVersion: null,
590602
libraryMetadataId: null,
591603
exportedLibraryUris: null,
592604
exportMap: {},
593-
exportMapId: ManifestItemId.generate(),
605+
exportMapId: manifest.exportMapId,
594606
instances: {},
595607
interfaces: {},
596608
exportedExtensions: null,
@@ -620,6 +632,7 @@ class LibraryRequirements {
620632

621633
factory LibraryRequirements.read(BinaryReader reader) {
622634
return LibraryRequirements(
635+
hashForRequirements: reader.readStringUtf8(),
623636
name: reader.readOptionalStringUtf8(),
624637
isSynthetic: reader.readOptionalBool(),
625638
featureSet: reader.readOptionalUint8List(),
@@ -665,6 +678,7 @@ class LibraryRequirements {
665678
}
666679

667680
void write(BinaryWriter writer) {
681+
writer.writeStringUtf8(hashForRequirements);
668682
writer.writeOptionalStringUtf8(name);
669683
writer.writeOptionalBool(isSynthetic);
670684
writer.writeOptionalUint8List(featureSet);
@@ -867,11 +881,13 @@ class RequirementsManifest {
867881
/// are satisfied.
868882
RequirementFailure? isSatisfied({
869883
required LinkedElementFactory elementFactory,
884+
required OperationPerformanceImpl performance,
870885
}) {
871886
if (opaqueApiUses.isNotEmpty) {
872887
return OpaqueApiUseFailure(uses: opaqueApiUses);
873888
}
874889

890+
var onlyHashForLibraries = true;
875891
for (var libraryEntry in libraries.entries) {
876892
var libraryUri = libraryEntry.key;
877893
var libraryRequirements = libraryEntry.value;
@@ -881,6 +897,15 @@ class RequirementsManifest {
881897
return LibraryMissing(uri: libraryUri);
882898
}
883899

900+
if (libraryRequirements.hashForRequirements ==
901+
libraryManifest.hashForRequirements) {
902+
performance.getDataInt('libHash').increment();
903+
continue;
904+
} else {
905+
performance.getDataInt('libDetails').increment();
906+
onlyHashForLibraries = false;
907+
}
908+
884909
if (libraryRequirements.name case var expected?) {
885910
var actual = libraryManifest.name;
886911
if (expected != actual) {
@@ -1525,6 +1550,12 @@ class RequirementsManifest {
15251550
}
15261551
}
15271552

1553+
if (onlyHashForLibraries) {
1554+
performance.getDataInt('libsHash').increment();
1555+
} else {
1556+
performance.getDataInt('libsDetails').increment();
1557+
}
1558+
15281559
for (var exportRequirement in exportRequirements) {
15291560
var failure = exportRequirement.isSatisfied(
15301561
elementFactory: elementFactory,
@@ -2524,11 +2555,10 @@ class RequirementsManifest {
25242555
return result;
25252556
}
25262557

2527-
result = LibraryRequirements.empty();
2528-
result.exportMapId = element.manifest!.exportMapId;
2529-
2558+
result = LibraryRequirements.fromManifest(element.manifest!);
2559+
state._result = result;
25302560
libraries[element.uri] = result;
2531-
return state._result = result;
2561+
return result;
25322562
}
25332563

25342564
String _qualifiedMethodName(

0 commit comments

Comments
 (0)