Skip to content

Commit 4551080

Browse files
jakemac53commit-bot@chromium.org
jakemac53
authored and
commit-bot@chromium.org
committed
Add retry logic for incremental compiler initialization.
Problem: We have been having a lot of reports of failures when initializing the incremental compiler (see #38102). These appear to be the result of empty kernel files but we have added other logic to check for that when kernel files are written, which has had zero reports of throwing from users (dart-lang/build#2387). We also use the same mechanism for temp directory management as we did with analyzer, but only see this problem with the switch to kernel, and specifically the incremental compiler. Solution: Add retry logic and see if that fixes the problem. Any time there is a retry log a warning so that we don't just silently do retries all the time. This is a general defensive mechanism to cover a broad spectrum of potential failures with the incremental compiler in the wild. I don't intend to remove it as it isn't harmful, and the warning logs should be enough to encourage issues to be filed if it is happening often. I have no direct reason to believe this will actually solve the particular linked problem other than that we only see this issue when the incremental compiler is enabled. Bug: #39122, #38102 Change-Id: Iaabb4497d6da69684692c1d7b9c030c59ebc6072 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/123001 Commit-Queue: Jake Macdonald <jakemac@google.com> Reviewed-by: Johnni Winther <johnniwinther@google.com> Auto-Submit: Jake Macdonald <jakemac@google.com>
1 parent f78e546 commit 4551080

File tree

3 files changed

+198
-165
lines changed

3 files changed

+198
-165
lines changed

‎pkg/front_end/lib/src/api_unstable/modular_incremental_compilation.dart‎

Lines changed: 191 additions & 165 deletions
Original file line numberDiff line numberDiff line change
@@ -54,190 +54,216 @@ Future<InitializedCompilerState> initializeIncrementalCompiler(
5454
bool outlineOnly,
5555
bool omitPlatform: false,
5656
bool trackNeededDillLibraries: false}) async {
57-
final List<int> sdkDigest = workerInputDigests[sdkSummary];
58-
if (sdkDigest == null) {
59-
throw new StateError("Expected to get digest for $sdkSummary");
60-
}
61-
62-
Map<Uri, WorkerInputComponent> workerInputCache =
63-
oldState?.workerInputCache ?? new Map<Uri, WorkerInputComponent>();
64-
Map<Uri, Uri> workerInputCacheLibs =
65-
oldState?.workerInputCacheLibs ?? new Map<Uri, Uri>();
66-
67-
WorkerInputComponent cachedSdkInput = workerInputCache[sdkSummary];
68-
69-
IncrementalCompiler incrementalCompiler;
70-
CompilerOptions options;
71-
ProcessedOptions processedOpts;
72-
73-
if (oldState == null ||
74-
oldState.incrementalCompiler == null ||
75-
oldState.options.compileSdk != compileSdk ||
76-
oldState.incrementalCompiler.outlineOnly != outlineOnly ||
77-
!equalMaps(oldState.options.experimentalFlags, experimentalFlags) ||
78-
!equalMaps(oldState.options.environmentDefines, environmentDefines) ||
79-
!equalSets(oldState.tags, tags) ||
80-
cachedSdkInput == null ||
81-
!digestsEqual(cachedSdkInput.digest, sdkDigest)) {
82-
// No - or immediately not correct - previous state.
83-
// We'll load a new sdk, anything loaded already will have a wrong root.
84-
workerInputCache.clear();
85-
workerInputCacheLibs.clear();
86-
87-
// The sdk was either not cached or it has changed.
88-
options = new CompilerOptions()
89-
..compileSdk = compileSdk
90-
..sdkRoot = sdkRoot
91-
..sdkSummary = sdkSummary
92-
..packagesFileUri = packagesFile
93-
..librariesSpecificationUri = librariesSpecificationUri
94-
..target = target
95-
..fileSystem = fileSystem
96-
..omitPlatform = omitPlatform
97-
..environmentDefines = environmentDefines
98-
..experimentalFlags = experimentalFlags;
99-
100-
processedOpts = new ProcessedOptions(options: options);
101-
cachedSdkInput = new WorkerInputComponent(
102-
sdkDigest, await processedOpts.loadSdkSummary(null));
103-
workerInputCache[sdkSummary] = cachedSdkInput;
104-
for (Library lib in cachedSdkInput.component.libraries) {
105-
if (workerInputCacheLibs.containsKey(lib.importUri)) {
106-
throw new StateError("Duplicate sources in sdk.");
57+
bool isRetry = false;
58+
while (true) {
59+
try {
60+
final List<int> sdkDigest = workerInputDigests[sdkSummary];
61+
if (sdkDigest == null) {
62+
throw new StateError("Expected to get digest for $sdkSummary");
10763
}
108-
workerInputCacheLibs[lib.importUri] = sdkSummary;
109-
}
11064

111-
incrementalCompiler = new IncrementalCompiler.fromComponent(
112-
new CompilerContext(processedOpts),
113-
cachedSdkInput.component,
114-
outlineOnly);
115-
incrementalCompiler.trackNeededDillLibraries = trackNeededDillLibraries;
116-
} else {
117-
options = oldState.options;
118-
processedOpts = oldState.processedOpts;
119-
Component sdkComponent = cachedSdkInput.component;
120-
// Reset the state of the component.
121-
for (Library lib in sdkComponent.libraries) {
122-
lib.isExternal = cachedSdkInput.externalLibs.contains(lib.importUri);
123-
}
65+
Map<Uri, WorkerInputComponent> workerInputCache =
66+
oldState?.workerInputCache ?? new Map<Uri, WorkerInputComponent>();
67+
Map<Uri, Uri> workerInputCacheLibs =
68+
oldState?.workerInputCacheLibs ?? new Map<Uri, Uri>();
12469

125-
// Make sure the canonical name root knows about the sdk - otherwise we
126-
// won't be able to link to it when loading more outlines.
127-
sdkComponent.adoptChildren();
128-
129-
// TODO(jensj): This is - at least currently - necessary,
130-
// although it's not entirely obvious why.
131-
// It likely has to do with several outlines containing the same libraries.
132-
// Once that stops (and we check for it) we can probably remove this,
133-
// and instead only do it when about to reuse an outline in the
134-
// 'inputSummaries.add(component);' line further down.
135-
for (WorkerInputComponent cachedInput in workerInputCache.values) {
136-
cachedInput.component.adoptChildren();
137-
}
70+
WorkerInputComponent cachedSdkInput = workerInputCache[sdkSummary];
13871

139-
// Reuse the incremental compiler, but reset as needed.
140-
incrementalCompiler = oldState.incrementalCompiler;
141-
incrementalCompiler.invalidateAllSources();
142-
incrementalCompiler.trackNeededDillLibraries = trackNeededDillLibraries;
143-
options.packagesFileUri = packagesFile;
144-
options.fileSystem = fileSystem;
145-
processedOpts.clearFileSystemCache();
146-
}
72+
IncrementalCompiler incrementalCompiler;
73+
CompilerOptions options;
74+
ProcessedOptions processedOpts;
14775

148-
// Then read all the input summary components.
149-
CanonicalName nameRoot = cachedSdkInput.component.root;
150-
Map<Uri, Uri> libraryToInputDill;
151-
if (trackNeededDillLibraries) {
152-
libraryToInputDill = new Map<Uri, Uri>();
153-
}
154-
List<int> loadFromDillIndexes = new List<int>();
76+
if (oldState == null ||
77+
oldState.incrementalCompiler == null ||
78+
oldState.options.compileSdk != compileSdk ||
79+
oldState.incrementalCompiler.outlineOnly != outlineOnly ||
80+
!equalMaps(oldState.options.experimentalFlags, experimentalFlags) ||
81+
!equalMaps(oldState.options.environmentDefines, environmentDefines) ||
82+
!equalSets(oldState.tags, tags) ||
83+
cachedSdkInput == null ||
84+
!digestsEqual(cachedSdkInput.digest, sdkDigest)) {
85+
// No - or immediately not correct - previous state.
86+
// We'll load a new sdk, anything loaded already will have a wrong root.
87+
workerInputCache.clear();
88+
workerInputCacheLibs.clear();
15589

156-
// Notice that the ordering of the input summaries matter, so we need to
157-
// keep them in order.
158-
if (outputLoadedInputSummaries.length != inputSummaries.length) {
159-
throw new ArgumentError("Invalid length.");
160-
}
161-
Set<Uri> inputSummariesSet = new Set<Uri>();
162-
for (int i = 0; i < inputSummaries.length; i++) {
163-
Uri summaryUri = inputSummaries[i];
164-
inputSummariesSet.add(summaryUri);
165-
WorkerInputComponent cachedInput = workerInputCache[summaryUri];
166-
List<int> digest = workerInputDigests[summaryUri];
167-
if (digest == null) {
168-
throw new StateError("Expected to get digest for $summaryUri");
169-
}
170-
if (cachedInput == null ||
171-
cachedInput.component.root != nameRoot ||
172-
!digestsEqual(digest, cachedInput.digest)) {
173-
// Remove any old libraries from workerInputCacheLibs.
174-
Component component = cachedInput?.component;
175-
if (component != null) {
176-
for (Library lib in component.libraries) {
177-
workerInputCacheLibs.remove(lib.importUri);
90+
// The sdk was either not cached or it has changed.
91+
options = new CompilerOptions()
92+
..compileSdk = compileSdk
93+
..sdkRoot = sdkRoot
94+
..sdkSummary = sdkSummary
95+
..packagesFileUri = packagesFile
96+
..librariesSpecificationUri = librariesSpecificationUri
97+
..target = target
98+
..fileSystem = fileSystem
99+
..omitPlatform = omitPlatform
100+
..environmentDefines = environmentDefines
101+
..experimentalFlags = experimentalFlags;
102+
103+
processedOpts = new ProcessedOptions(options: options);
104+
cachedSdkInput = new WorkerInputComponent(
105+
sdkDigest, await processedOpts.loadSdkSummary(null));
106+
workerInputCache[sdkSummary] = cachedSdkInput;
107+
for (Library lib in cachedSdkInput.component.libraries) {
108+
if (workerInputCacheLibs.containsKey(lib.importUri)) {
109+
throw new StateError("Duplicate sources in sdk.");
110+
}
111+
workerInputCacheLibs[lib.importUri] = sdkSummary;
178112
}
179-
}
180113

181-
loadFromDillIndexes.add(i);
182-
} else {
183-
// Need to reset cached components so they are usable again.
184-
Component component = cachedInput.component;
185-
for (Library lib in component.libraries) {
186-
lib.isExternal = cachedInput.externalLibs.contains(lib.importUri);
187-
if (trackNeededDillLibraries) {
188-
libraryToInputDill[lib.importUri] = summaryUri;
114+
incrementalCompiler = new IncrementalCompiler.fromComponent(
115+
new CompilerContext(processedOpts),
116+
cachedSdkInput.component,
117+
outlineOnly);
118+
incrementalCompiler.trackNeededDillLibraries = trackNeededDillLibraries;
119+
} else {
120+
options = oldState.options;
121+
processedOpts = oldState.processedOpts;
122+
Component sdkComponent = cachedSdkInput.component;
123+
// Reset the state of the component.
124+
for (Library lib in sdkComponent.libraries) {
125+
lib.isExternal = cachedSdkInput.externalLibs.contains(lib.importUri);
126+
}
127+
128+
// Make sure the canonical name root knows about the sdk - otherwise we
129+
// won't be able to link to it when loading more outlines.
130+
sdkComponent.adoptChildren();
131+
132+
// TODO(jensj): This is - at least currently - necessary,
133+
// although it's not entirely obvious why.
134+
// It likely has to do with several outlines containing the same
135+
// libraries. Once that stops (and we check for it) we can probably
136+
// remove this, and instead only do it when about to reuse an outline
137+
// in the 'inputSummaries.add(component);' line further down.
138+
for (WorkerInputComponent cachedInput in workerInputCache.values) {
139+
cachedInput.component.adoptChildren();
189140
}
141+
142+
// Reuse the incremental compiler, but reset as needed.
143+
incrementalCompiler = oldState.incrementalCompiler;
144+
incrementalCompiler.invalidateAllSources();
145+
incrementalCompiler.trackNeededDillLibraries = trackNeededDillLibraries;
146+
options.packagesFileUri = packagesFile;
147+
options.fileSystem = fileSystem;
148+
processedOpts.clearFileSystemCache();
190149
}
191-
component.computeCanonicalNames(); // this isn't needed, is it?
192-
outputLoadedInputSummaries[i] = component;
193-
}
194-
}
195150

196-
for (int i = 0; i < loadFromDillIndexes.length; i++) {
197-
int index = loadFromDillIndexes[i];
198-
Uri summaryUri = inputSummaries[index];
199-
List<int> digest = workerInputDigests[summaryUri];
200-
if (digest == null) {
201-
throw new StateError("Expected to get digest for $summaryUri");
202-
}
151+
// Then read all the input summary components.
152+
CanonicalName nameRoot = cachedSdkInput.component.root;
153+
Map<Uri, Uri> libraryToInputDill;
154+
if (trackNeededDillLibraries) {
155+
libraryToInputDill = new Map<Uri, Uri>();
156+
}
157+
List<int> loadFromDillIndexes = new List<int>();
158+
159+
// Notice that the ordering of the input summaries matter, so we need to
160+
// keep them in order.
161+
if (outputLoadedInputSummaries.length != inputSummaries.length) {
162+
throw new ArgumentError("Invalid length.");
163+
}
164+
Set<Uri> inputSummariesSet = new Set<Uri>();
165+
for (int i = 0; i < inputSummaries.length; i++) {
166+
Uri summaryUri = inputSummaries[i];
167+
inputSummariesSet.add(summaryUri);
168+
WorkerInputComponent cachedInput = workerInputCache[summaryUri];
169+
List<int> digest = workerInputDigests[summaryUri];
170+
if (digest == null) {
171+
throw new StateError("Expected to get digest for $summaryUri");
172+
}
173+
if (cachedInput == null ||
174+
cachedInput.component.root != nameRoot ||
175+
!digestsEqual(digest, cachedInput.digest)) {
176+
// Remove any old libraries from workerInputCacheLibs.
177+
Component component = cachedInput?.component;
178+
if (component != null) {
179+
for (Library lib in component.libraries) {
180+
workerInputCacheLibs.remove(lib.importUri);
181+
}
182+
}
203183

204-
List<int> bytes = await fileSystem.entityForUri(summaryUri).readAsBytes();
205-
WorkerInputComponent cachedInput = new WorkerInputComponent(
206-
digest,
207-
await processedOpts.loadComponent(bytes, nameRoot,
208-
alwaysCreateNewNamedNodes: true));
209-
workerInputCache[summaryUri] = cachedInput;
210-
outputLoadedInputSummaries[index] = cachedInput.component;
211-
for (Library lib in cachedInput.component.libraries) {
212-
if (workerInputCacheLibs.containsKey(lib.importUri)) {
213-
Uri fromSummary = workerInputCacheLibs[lib.importUri];
214-
if (inputSummariesSet.contains(fromSummary)) {
215-
throw new StateError(
216-
"Asked to load several summaries that contain the same library.");
184+
loadFromDillIndexes.add(i);
217185
} else {
218-
// Library contained in old cached component. Flush that cache.
219-
Component component = workerInputCache.remove(fromSummary).component;
186+
// Need to reset cached components so they are usable again.
187+
Component component = cachedInput.component;
220188
for (Library lib in component.libraries) {
221-
workerInputCacheLibs.remove(lib.importUri);
189+
lib.isExternal = cachedInput.externalLibs.contains(lib.importUri);
190+
if (trackNeededDillLibraries) {
191+
libraryToInputDill[lib.importUri] = summaryUri;
192+
}
222193
}
194+
component.computeCanonicalNames(); // this isn't needed, is it?
195+
outputLoadedInputSummaries[i] = component;
223196
}
224-
} else {
225-
workerInputCacheLibs[lib.importUri] = summaryUri;
226197
}
227198

228-
if (trackNeededDillLibraries) {
229-
libraryToInputDill[lib.importUri] = summaryUri;
199+
for (int i = 0; i < loadFromDillIndexes.length; i++) {
200+
int index = loadFromDillIndexes[i];
201+
Uri summaryUri = inputSummaries[index];
202+
List<int> digest = workerInputDigests[summaryUri];
203+
if (digest == null) {
204+
throw new StateError("Expected to get digest for $summaryUri");
205+
}
206+
207+
List<int> bytes =
208+
await fileSystem.entityForUri(summaryUri).readAsBytes();
209+
WorkerInputComponent cachedInput = new WorkerInputComponent(
210+
digest,
211+
await processedOpts.loadComponent(bytes, nameRoot,
212+
alwaysCreateNewNamedNodes: true));
213+
workerInputCache[summaryUri] = cachedInput;
214+
outputLoadedInputSummaries[index] = cachedInput.component;
215+
for (Library lib in cachedInput.component.libraries) {
216+
if (workerInputCacheLibs.containsKey(lib.importUri)) {
217+
Uri fromSummary = workerInputCacheLibs[lib.importUri];
218+
if (inputSummariesSet.contains(fromSummary)) {
219+
throw new StateError(
220+
"Asked to load several summaries that contain the same "
221+
"library.");
222+
} else {
223+
// Library contained in old cached component. Flush that cache.
224+
Component component =
225+
workerInputCache.remove(fromSummary).component;
226+
for (Library lib in component.libraries) {
227+
workerInputCacheLibs.remove(lib.importUri);
228+
}
229+
}
230+
} else {
231+
workerInputCacheLibs[lib.importUri] = summaryUri;
232+
}
233+
234+
if (trackNeededDillLibraries) {
235+
libraryToInputDill[lib.importUri] = summaryUri;
236+
}
237+
}
230238
}
231-
}
232-
}
233239

234-
incrementalCompiler
235-
.setModulesToLoadOnNextComputeDelta(outputLoadedInputSummaries);
240+
incrementalCompiler
241+
.setModulesToLoadOnNextComputeDelta(outputLoadedInputSummaries);
236242

237-
return new InitializedCompilerState(options, processedOpts,
238-
workerInputCache: workerInputCache,
239-
workerInputCacheLibs: workerInputCacheLibs,
240-
incrementalCompiler: incrementalCompiler,
241-
tags: tags,
242-
libraryToInputDill: libraryToInputDill);
243+
return new InitializedCompilerState(options, processedOpts,
244+
workerInputCache: workerInputCache,
245+
workerInputCacheLibs: workerInputCacheLibs,
246+
incrementalCompiler: incrementalCompiler,
247+
tags: tags,
248+
libraryToInputDill: libraryToInputDill);
249+
} catch (e, s) {
250+
if (isRetry) rethrow;
251+
print('''
252+
Failed to initialize incremental compiler, throwing away old state.
253+
254+
This is likely a result of https://github.com/dart-lang/sdk/issues/38102, if
255+
you are consistently seeing this problem please see that issue.
256+
257+
The specific exception that was encountered was:
258+
259+
$e
260+
$s
261+
''');
262+
isRetry = true;
263+
oldState = null;
264+
// Artificial delay to attempt to increase the odds of recovery from
265+
// timing related issues.
266+
await new Future.delayed(const Duration(milliseconds: 50));
267+
}
268+
}
243269
}

0 commit comments

Comments
 (0)