Skip to content

Commit 63261a7

Browse files
srujzsCommit Queue
srujzs
authored and
Commit Queue
committed
[frontend_server/ddc] Add recompile-restart instruction
- Adds a new instruction intended for recompiles during a hot restart. This is desired so that DDC can differentiate between the two recompiles and emit errors for hot reload. The VM only ever recompiles during a hot reload, so they won't need this. - Adds some tests to make sure recompile-restart doesn't trigger errors that recompile will. Also tests that they're virtually similar otherwise. - Fixes a small issue in the delta inspector to use importUris instead of fileUris when computing the LibraryIndex. Change-Id: Ic375167ec72934cd380c4f538b45566bd87de433 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406200 Reviewed-by: Jens Johansen <[email protected]> Commit-Queue: Srujan Gaddam <[email protected]> Reviewed-by: Nicholas Shahan <[email protected]>
1 parent 0083128 commit 63261a7

File tree

5 files changed

+172
-30
lines changed

5 files changed

+172
-30
lines changed

‎pkg/front_end/test/spell_checking_list_code.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1452,6 +1452,7 @@ recognizing
14521452
recompilation
14531453
recompile
14541454
recompiled
1455+
recompiles
14551456
recompiling
14561457
recompute
14571458
recomputed
@@ -1533,6 +1534,7 @@ resource
15331534
respond
15341535
response
15351536
responses
1537+
restart
15361538
restoring
15371539
restriction
15381540
restricts

‎pkg/frontend_server/lib/frontend_server.dart‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,7 @@ ${argParser.usage}
263263
enum _State {
264264
READY_FOR_INSTRUCTION,
265265
RECOMPILE_LIST,
266+
RECOMPILE_RESTART_LIST,
266267
// compileExpression
267268
COMPILE_EXPRESSION_EXPRESSION,
268269
COMPILE_EXPRESSION_DEFS,
@@ -312,7 +313,11 @@ abstract class CompilerInterface {
312313

313314
/// Assuming some Dart program was previously compiled, recompile it again
314315
/// taking into account some changed(invalidated) sources.
315-
Future<void> recompileDelta({String? entryPoint});
316+
///
317+
/// If [recompileRestart] is true, recompiles assuming the frontend server was
318+
/// given a `recompile-restart` request.
319+
Future<void> recompileDelta(
320+
{String? entryPoint, bool recompileRestart = false});
316321

317322
/// Accept results of previous compilation so that next recompilation cycle
318323
/// won't recompile sources that were previously reported as changed.
@@ -692,7 +697,7 @@ class FrontendCompiler implements CompilerInterface {
692697
if (_compilerOptions.target!.name == 'dartdevc') {
693698
await writeJavaScriptBundle(results, _kernelBinaryFilename,
694699
options['filesystem-scheme'], options['dartdevc-module-format'],
695-
fullComponent: true);
700+
fullComponent: true, recompileRestart: false);
696701
}
697702
await writeDillFile(
698703
results,
@@ -829,9 +834,12 @@ class FrontendCompiler implements CompilerInterface {
829834
}
830835

831836
/// Write a JavaScript bundle containing the provided component.
837+
///
838+
/// [recompileRestart] should be true when this is part of a
839+
/// `recompile-restart` request.
832840
Future<void> writeJavaScriptBundle(KernelCompilationResults results,
833841
String filename, String fileSystemScheme, String moduleFormat,
834-
{required bool fullComponent}) async {
842+
{required bool fullComponent, required bool recompileRestart}) async {
835843
PackageConfig packageConfig = await loadPackageConfigUri(
836844
_compilerOptions.packagesFileUri ??
837845
new File('.dart_tool/package_config.json').absolute.uri);
@@ -850,11 +858,9 @@ class FrontendCompiler implements CompilerInterface {
850858
if (fullComponent) {
851859
await bundler.initialize(component, _mainSource, packageConfig);
852860
} else {
853-
await bundler.invalidate(
854-
component,
855-
_generator.lastKnownGoodResult!.component,
856-
_mainSource,
857-
packageConfig);
861+
await bundler.invalidate(component,
862+
_generator.lastKnownGoodResult!.component, _mainSource, packageConfig,
863+
recompileRestart: recompileRestart);
858864
}
859865

860866
// Create JavaScript bundler.
@@ -1018,7 +1024,8 @@ class FrontendCompiler implements CompilerInterface {
10181024
}
10191025

10201026
@override
1021-
Future<void> recompileDelta({String? entryPoint}) async {
1027+
Future<void> recompileDelta(
1028+
{String? entryPoint, bool recompileRestart = false}) async {
10221029
final String boundaryKey = generateV4UUID();
10231030
_outputStream.writeln('result $boundaryKey');
10241031
await invalidateIfInitializingFromDill();
@@ -1046,7 +1053,7 @@ class FrontendCompiler implements CompilerInterface {
10461053
try {
10471054
await writeJavaScriptBundle(results, _kernelBinaryFilename,
10481055
_options['filesystem-scheme'], _options['dartdevc-module-format'],
1049-
fullComponent: false);
1056+
fullComponent: false, recompileRestart: recompileRestart);
10501057
} catch (e) {
10511058
_outputStream.writeln('$e');
10521059
errors.add(e.toString());
@@ -1389,6 +1396,7 @@ StreamSubscription<String> listenAndCompile(CompilerInterface compiler,
13891396
case _State.READY_FOR_INSTRUCTION:
13901397
const String COMPILE_INSTRUCTION_SPACE = 'compile ';
13911398
const String RECOMPILE_INSTRUCTION_SPACE = 'recompile ';
1399+
const String RECOMPILE_RESTART_INSTRUCTION_SPACE = 'recompile-restart ';
13921400
const String NATIVE_ASSETS_INSTRUCTION_SPACE = 'native-assets ';
13931401
const String NATIVE_ASSETS_ONLY_INSTRUCTION = 'native-assets-only';
13941402
const String COMPILE_EXPRESSION_INSTRUCTION_SPACE =
@@ -1401,6 +1409,19 @@ StreamSubscription<String> listenAndCompile(CompilerInterface compiler,
14011409
await compiler.compile(entryPoint, options, generator: generator);
14021410
} else if (string == NATIVE_ASSETS_ONLY_INSTRUCTION) {
14031411
await compiler.compileNativeAssetsOnly(options, generator: generator);
1412+
} else if (string.startsWith(RECOMPILE_RESTART_INSTRUCTION_SPACE)) {
1413+
// 'recompile-restart [<entryPoint>] <boundarykey>'
1414+
// where <boundarykey> can't have spaces
1415+
final String remainder =
1416+
string.substring(RECOMPILE_RESTART_INSTRUCTION_SPACE.length);
1417+
final int spaceDelim = remainder.lastIndexOf(' ');
1418+
if (spaceDelim > -1) {
1419+
recompileEntryPoint = remainder.substring(0, spaceDelim);
1420+
boundaryKey = remainder.substring(spaceDelim + 1);
1421+
} else {
1422+
boundaryKey = remainder;
1423+
}
1424+
state = _State.RECOMPILE_RESTART_LIST;
14041425
} else if (string.startsWith(RECOMPILE_INSTRUCTION_SPACE)) {
14051426
// 'recompile [<entryPoint>] <boundarykey>'
14061427
// where <boundarykey> can't have spaces
@@ -1477,8 +1498,11 @@ StreamSubscription<String> listenAndCompile(CompilerInterface compiler,
14771498
}
14781499
break;
14791500
case _State.RECOMPILE_LIST:
1501+
case _State.RECOMPILE_RESTART_LIST:
14801502
if (string == boundaryKey) {
1481-
await compiler.recompileDelta(entryPoint: recompileEntryPoint);
1503+
await compiler.recompileDelta(
1504+
entryPoint: recompileEntryPoint,
1505+
recompileRestart: state == _State.RECOMPILE_RESTART_LIST);
14821506
state = _State.READY_FOR_INSTRUCTION;
14831507
} else {
14841508
compiler.invalidate(Uri.base.resolve(string));

‎pkg/frontend_server/lib/src/javascript_bundle.dart‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,12 @@ class IncrementalJavaScriptBundler {
8080

8181
/// Update the incremental bundler from a partial component and the last full
8282
/// component.
83-
Future<void> invalidate(
84-
Component partialComponent,
85-
Component lastFullComponent,
86-
Uri mainUri,
87-
PackageConfig packageConfig) async {
88-
if (canaryFeatures && _moduleFormat == ModuleFormat.ddc) {
83+
Future<void> invalidate(Component partialComponent,
84+
Component lastFullComponent, Uri mainUri, PackageConfig packageConfig,
85+
{required bool recompileRestart}) async {
86+
if (canaryFeatures &&
87+
_moduleFormat == ModuleFormat.ddc &&
88+
!recompileRestart) {
8989
// Find any potential hot reload rejections before updating the strongly
9090
// connected component graph.
9191
final List<String> errors = _deltaInspector.compareGenerations(

‎pkg/frontend_server/test/frontend_server_test.dart‎

Lines changed: 125 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ class _MockedCompiler implements CompilerInterface {
6767
}
6868

6969
@override
70-
Future<void> recompileDelta({String? entryPoint}) async {
70+
Future<void> recompileDelta(
71+
{String? entryPoint, bool recompileRestart = false}) async {
7172
verifyRecompileDelta(entryPoint);
7273
}
7374

@@ -2388,7 +2389,9 @@ void main(List<String> arguments, SendPort sendPort) {
23882389
// library bundle of a multi-bundle compilation.
23892390
group('recompile to JavaScript with in-body change', () {
23902391
Future<void> runTests(
2391-
{required String moduleFormat, bool canary = false}) async {
2392+
{required String moduleFormat,
2393+
bool canary = false,
2394+
bool recompileRestart = false}) async {
23922395
// Five libraries, a to e, in two library bundles, {a, b} and {c, d, e}:
23932396
// (a <-> b) -> (c <-> d <-> e)
23942397
// In body changes are performed on d and e. With advanced invalidation,
@@ -2526,7 +2529,8 @@ d() {
25262529
""");
25272530
// Trigger a recompile that invalidates 'd.dart'. The entry point
25282531
// uri (a.dart) is passed explicitly.
2529-
frontendServer.recompile(fileD.uri, entryPoint: entryPoint);
2532+
frontendServer.recompile(fileD.uri,
2533+
entryPoint: entryPoint, recompileRestart: recompileRestart);
25302534
break;
25312535
case 1:
25322536
CompilationResult result =
@@ -2562,7 +2566,8 @@ e() {
25622566
""");
25632567
// Trigger a recompile that invalidates 'd.dart'. The entry point
25642568
// uri (a.dart) is omitted.
2565-
frontendServer.recompile(fileE.uri);
2569+
frontendServer.recompile(fileE.uri,
2570+
recompileRestart: recompileRestart);
25662571
break;
25672572
case 2:
25682573
CompilationResult result =
@@ -2609,6 +2614,113 @@ e() {
26092614
test('DDC module format and canary', () async {
26102615
await runTests(moduleFormat: 'ddc', canary: true);
26112616
});
2617+
2618+
test('DDC module format and canary and recompile-restart', () async {
2619+
// Test that `recompile-restart` makes no difference for the given
2620+
// inputs.
2621+
await runTests(
2622+
moduleFormat: 'ddc', canary: true, recompileRestart: true);
2623+
});
2624+
2625+
group(
2626+
'valid change that is invalid for hot reload fails with recompile, '
2627+
'but not recompile-restart', () {
2628+
Future<void> runTests({bool recompileRestart = false}) async {
2629+
// Making a const class non-const is not valid for a hot reload.
2630+
File fileA = new File('${tempDir.path}/a.dart')
2631+
..createSync()
2632+
..writeAsStringSync('''
2633+
class A {
2634+
const A();
2635+
}
2636+
2637+
main() {}
2638+
''');
2639+
File packageConfig =
2640+
new File('${tempDir.path}/.dart_tool/package_config.json')
2641+
..createSync(recursive: true)
2642+
..writeAsStringSync('''
2643+
{
2644+
"configVersion": 2,
2645+
"packages": [
2646+
{
2647+
"name": "a",
2648+
"rootUri": "../",
2649+
"packageUri": "./"
2650+
}
2651+
]
2652+
}
2653+
''');
2654+
2655+
String entryPoint = 'package:a/a.dart';
2656+
2657+
File dillFile = new File('${tempDir.path}/app.dill');
2658+
2659+
expect(dillFile.existsSync(), false);
2660+
2661+
final List<String> args = <String>[
2662+
'--sdk-root=${sdkRoot.toFilePath()}',
2663+
'--incremental',
2664+
'--platform=${ddcPlatformKernel.path}',
2665+
'--output-dill=${dillFile.path}',
2666+
'--target=dartdevc',
2667+
// Errors are only emitted with the DDC library bundle format.
2668+
'--dartdevc-module-format=ddc',
2669+
'--dartdevc-canary',
2670+
'--packages=${packageConfig.path}',
2671+
'--emit-debug-symbols',
2672+
];
2673+
2674+
FrontendServer frontendServer = new FrontendServer();
2675+
Future<int> result = frontendServer.open(args);
2676+
frontendServer.compile(entryPoint);
2677+
int count = 0;
2678+
frontendServer.listen((Result compiledResult) {
2679+
switch (count) {
2680+
case 0:
2681+
CompilationResult result =
2682+
new CompilationResult.parse(compiledResult.status);
2683+
expect(result.errorsCount, equals(0));
2684+
2685+
frontendServer.accept();
2686+
2687+
fileA.writeAsStringSync('''
2688+
class A {
2689+
A();
2690+
}
2691+
2692+
main() {}
2693+
''');
2694+
frontendServer.recompile(fileA.uri,
2695+
entryPoint: entryPoint, recompileRestart: recompileRestart);
2696+
break;
2697+
case 1:
2698+
CompilationResult result =
2699+
new CompilationResult.parse(compiledResult.status);
2700+
expect(result.errorsCount, equals(recompileRestart ? 0 : 1));
2701+
2702+
frontendServer.accept();
2703+
frontendServer.quit();
2704+
break;
2705+
default:
2706+
break;
2707+
}
2708+
count++;
2709+
});
2710+
2711+
expect(await result, 0);
2712+
expect(count, 2);
2713+
frontendServer.close();
2714+
}
2715+
2716+
test('recompile', () async {
2717+
await runTests(recompileRestart: false);
2718+
});
2719+
2720+
test('recompile-restart', () async {
2721+
await runTests(recompileRestart: true);
2722+
});
2723+
});
26122724
});
26132725

26142726
group('compile to JavaScript all library bundles with unsound null safety',
@@ -3591,15 +3703,17 @@ class FrontendServer {
35913703
void recompile(Uri? invalidatedUri,
35923704
{String boundaryKey = 'abc',
35933705
List<Uri>? invalidatedUris,
3594-
String? entryPoint}) {
3706+
String? entryPoint,
3707+
bool recompileRestart = false}) {
35953708
invalidatedUris ??= [if (invalidatedUri != null) invalidatedUri];
35963709
outputParser.expectSources = true;
3597-
inputStreamController.add('recompile '
3598-
'${entryPoint != null ? '$entryPoint ' : ''}'
3599-
'$boundaryKey\n'
3600-
'${invalidatedUris.map((uri) => '$uri\n').join()}'
3601-
'$boundaryKey\n'
3602-
.codeUnits);
3710+
inputStreamController
3711+
.add('${recompileRestart ? 'recompile-restart ' : 'recompile '}'
3712+
'${entryPoint != null ? '$entryPoint ' : ''}'
3713+
'$boundaryKey\n'
3714+
'${invalidatedUris.map((uri) => '$uri\n').join()}'
3715+
'$boundaryKey\n'
3716+
.codeUnits);
36033717
}
36043718

36053719
/// Compiles the native assets in isolation.

‎pkg/frontend_server/test/src/javascript_bundle_test.dart‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,8 @@ void main() {
481481
new Component(libraries: [libraryA2, libraryB2, libraryC2]);
482482

483483
await javaScriptBundler.invalidate(
484-
partialComponent, testComponent, uriA, packageConfig);
484+
partialComponent, testComponent, uriA, packageConfig,
485+
recompileRestart: false);
485486

486487
final _MemorySink manifestSink = new _MemorySink();
487488
final _MemorySink codeSink = new _MemorySink();
@@ -584,7 +585,8 @@ void main() {
584585
final Component partialComponent = new Component(libraries: [libraryC2]);
585586

586587
await javaScriptBundler.invalidate(
587-
partialComponent, testComponent, uriA, packageConfig);
588+
partialComponent, testComponent, uriA, packageConfig,
589+
recompileRestart: false);
588590

589591
final _MemorySink manifestSink = new _MemorySink();
590592
final _MemorySink codeSink = new _MemorySink();

0 commit comments

Comments
 (0)