dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[vm] Remove unused kernel bytes in case of compile-time errors

Browse Source 
In case of compile-time errors kernel service has been serializing kernel AST anyway and has been passing kernel bytes to the VM.
VM does not use those kernel bytes except a few unit tests.

This change removes the unnecessary kernel serialization and freeing
of kernel bytes and cleans up unit tests.

TEST=ci

Change-Id: Ic2464e50f56227bc998df53dd2b594c1abcf9468
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/436360
Commit-Queue: Alexander Markov <alexmarkov@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
This commit is contained in:
Alexander Markov
2025-06-23 10:01:09 -07:00
committed by Commit Queue
parent 66ba0cf14f
commit 36087ec8f7
10 changed files with 63 additions and 174 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/vm/bin/kernel_service.dart
+10 -28
View File
@@ -643,7 +643,7 @@ Future _processExpressionCompilationRequest(request) async {
port.send(
new CompilationResult.errors([
"No platform found to initialize incremental compiler.",
], null).toResponse(),
]).toResponse(),
);
return;
}
@@ -687,7 +687,7 @@ Future _processExpressionCompilationRequest(request) async {
new CompilationResult.errors([
"Error when trying to create a compiler for expression compilation: "
"'$e'.",
], null).toResponse(),
]).toResponse(),
);
return;
}
@@ -698,7 +698,7 @@ Future _processExpressionCompilationRequest(request) async {
port.send(
new CompilationResult.errors([
"No incremental compiler available for this isolate.",
], null).toResponse(),
]).toResponse(),
);
return;
}
@@ -724,9 +724,7 @@ Future _processExpressionCompilationRequest(request) async {
);
if (procedure == null) {
port.send(
new CompilationResult.errors(["Invalid scope."], null).toResponse(),
);
port.send(new CompilationResult.errors(["Invalid scope."]).toResponse());
return;
}
@@ -736,7 +734,7 @@ Future _processExpressionCompilationRequest(request) async {
if (compiler.errorsPlain.isNotEmpty) {
// TODO(sigmund): the compiler prints errors to the console, so we
// shouldn't print those messages again here.
result = new CompilationResult.errors(compiler.errorsPlain, null);
result = new CompilationResult.errors(compiler.errorsPlain);
} else {
Component component = createExpressionEvaluationComponent(procedure);
result = new CompilationResult.ok(serializeComponent(component));
@@ -899,7 +897,7 @@ Future _processLoadRequest(request) async {
port.send(
new CompilationResult.errors([
"No incremental compiler available for this isolate.",
], null).toResponse(),
]).toResponse(),
);
return;
}
@@ -1024,19 +1022,7 @@ Future _processLoadRequest(request) async {
...(enableColors) ? compiler.errorsColorized : compiler.errorsPlain,
...nativeAssetsErrors.map((e) => e.message),
];
final component = compilerResult.component;
if (component != null) {
result = new CompilationResult.errors(
errors,
serializeComponent(
component,
filter: (lib) => !loadedLibraries.contains(lib),
nativeAssetsComponent: nativeAssetsComponent,
),
);
} else {
result = new CompilationResult.errors(errors, null);
}
result = new CompilationResult.errors(errors);
} else {
// We serialize the component excluding vm_platform.dill because the VM has
// these sources built-in. Everything loaded as a summary in
@@ -1076,7 +1062,7 @@ Future _processLoadRequest(request) async {
inputFileUri,
inputFileUri,
null,
new CompilationResult.errors(<String>["unknown tag"], null).payload,
new CompilationResult.errors(<String>["unknown tag"]).payload,
]);
}
}
@@ -1288,8 +1274,7 @@ abstract class CompilationResult {
factory CompilationResult.ok(Uint8List? bytes) = _CompilationOk;
factory CompilationResult.errors(List<String> errors, Uint8List? bytes) =
_CompilationError;
factory CompilationResult.errors(List<String> errors) = _CompilationError;
factory CompilationResult.crash(Object exception, StackTrace stack) =
_CompilationCrash;
@@ -1332,10 +1317,9 @@ abstract class _CompilationFail extends CompilationResult {
}
class _CompilationError extends _CompilationFail {
final Uint8List? bytes;
final List<String> errors;
_CompilationError(this.errors, this.bytes);
_CompilationError(this.errors);
@override
Status get status => Status.error;
@@ -1344,8 +1328,6 @@ class _CompilationError extends _CompilationFail {
String get errorString => errors.join('\n');
String toString() => "_CompilationError(${errorString})";
List toResponse() => [status.index, payload, bytes];
}
class _CompilationCrash extends _CompilationFail {
pkg/vm/test/kernel_service_test.dart
+2 -3
View File
@@ -169,11 +169,10 @@ Future<kernel_service.Status> singleShotCompile(
}
return kernel_service.Status.ok;
} else if (status == kernel_service.Status.error.index) {
expectLength(m, 3);
expectLength(m, 2);
final String errors = m[1];
final List<int> bytes = m[2];
if (verbose) {
print("Compiled with errors --- $errors and ${bytes.length} bytes dill");
print("Compiled with errors --- $errors");
}
return kernel_service.Status.error;
} else if (status == kernel_service.Status.crash.index) {
runtime/bin/dfe.cc
-3
View File
@@ -213,18 +213,15 @@ void DFE::CompileAndReadScript(const char* script_uri,
*exit_code = 0;
break;
case Dart_KernelCompilationStatus_Error:
free(result.kernel);
*error = result.error; // Copy error message.
*exit_code = kCompilationErrorExitCode;
break;
case Dart_KernelCompilationStatus_Crash:
free(result.kernel);
*error = result.error; // Copy error message.
*exit_code = kDartFrontendErrorExitCode;
break;
case Dart_KernelCompilationStatus_Unknown:
case Dart_KernelCompilationStatus_MsgFailed:
free(result.kernel);
*error = result.error; // Copy error message.
*exit_code = kErrorExitCode;
break;
runtime/vm/dart_api_impl_test.cc
+2 -4
View File
@@ -8299,8 +8299,7 @@ TEST_CASE(DartAPI_Multiroot_Valid) {
int sourcefiles_count = sizeof(sourcefiles) / sizeof(Dart_SourceFile);
lib = TestCase::LoadTestScriptWithDFE(
sourcefiles_count, sourcefiles, nullptr, /* finalize= */ true,
/* incrementally= */ true, /* allow_compile_errors= */ false,
"foo:///main.dart",
/* incrementally= */ true, "foo:///main.dart",
/* multiroot_filepaths= */ "/bar,/baz",
/* multiroot_scheme= */ "foo");
EXPECT_VALID(lib);
@@ -8340,8 +8339,7 @@ TEST_CASE(DartAPI_Multiroot_FailWhenUriIsWrong) {
int sourcefiles_count = sizeof(sourcefiles) / sizeof(Dart_SourceFile);
lib = TestCase::LoadTestScriptWithDFE(
sourcefiles_count, sourcefiles, nullptr, /* finalize= */ true,
/* incrementally= */ true, /* allow_compile_errors= */ false,
"foo1:///main.dart",
/* incrementally= */ true, "foo1:///main.dart",
/* multiroot_filepaths= */ "/bar,/baz",
/* multiroot_scheme= */ "foo");
EXPECT_ERROR(lib,
runtime/vm/isolate_reload.cc
-3
View File
@@ -1184,9 +1184,6 @@ char* IsolateGroupReloadContext::CompileToKernel(bool force_reload,
/*multiroot_scheme=*/nullptr);
}
if (retval.status != Dart_KernelCompilationStatus_Ok) {
if (retval.kernel != nullptr) {
free(retval.kernel);
}
return retval.error;
}
*kernel_buffer = retval.kernel;
runtime/vm/isolate_reload_test.cc
+33 -24
View File
@@ -614,17 +614,16 @@ TEST_CASE(IsolateReload_LibraryImportAdded) {
" return max(3, 4);\n"
"}\n";
const char* kReloadScript =
const char* kScript2 =
"import 'dart:math';\n"
"main() {\n"
" return max(3, 4);\n"
"}\n";
Dart_Handle lib = TestCase::LoadTestScriptWithErrors(kScript);
EXPECT_VALID(lib);
EXPECT_ERROR(SimpleInvokeError(lib, "main"), "max");
Dart_Handle lib = TestCase::LoadTestScript(kScript, nullptr);
EXPECT_ERROR(lib, "Compilation failed");
lib = TestCase::ReloadTestScript(kReloadScript);
lib = TestCase::LoadTestScript(kScript2, nullptr);
EXPECT_VALID(lib);
EXPECT_EQ(4, SimpleInvoke(lib, "main"));
}
@@ -1124,8 +1123,8 @@ TEST_CASE(IsolateReload_LibraryHide) {
const char* kImportScript = "importedFunc() => 'a';\n";
TestCase::AddTestLib("test:lib1", kImportScript);
// Import 'test:lib1' with importedFunc hidden. Will result in an
// error.
// Import 'test:lib1' with importedFunc hidden. Will result in a
// compile-time error.
const char* kScript =
"import 'test:lib1' hide importedFunc;\n"
"main() {\n"
@@ -1134,18 +1133,17 @@ TEST_CASE(IsolateReload_LibraryHide) {
// Dart_Handle result;
Dart_Handle lib = TestCase::LoadTestScriptWithErrors(kScript);
EXPECT_VALID(lib);
EXPECT_ERROR(SimpleInvokeError(lib, "main"), "importedFunc");
Dart_Handle lib = TestCase::LoadTestScript(kScript, nullptr);
EXPECT_ERROR(lib, "Compilation failed");
// Import 'test:lib1'.
const char* kReloadScript =
const char* kScript2 =
"import 'test:lib1';\n"
"main() {\n"
" return importedFunc();\n"
"}\n";
lib = TestCase::ReloadTestScript(kReloadScript);
lib = TestCase::LoadTestScript(kScript2, nullptr);
EXPECT_VALID(lib);
EXPECT_STREQ("a", SimpleInvokeStr(lib, "main"));
}
@@ -1157,7 +1155,7 @@ TEST_CASE(IsolateReload_LibraryShow) {
TestCase::AddTestLib("test:lib1", kImportScript);
// Import 'test:lib1' with importedIntFunc visible. Will result in
// an error when 'main' is invoked.
// a compile-time error.
const char* kScript =
"import 'test:lib1' show importedIntFunc;\n"
"main() {\n"
@@ -1168,17 +1166,12 @@ TEST_CASE(IsolateReload_LibraryShow) {
" return importedIntFunc();\n"
"}\n";
Dart_Handle lib = TestCase::LoadTestScriptWithErrors(kScript);
EXPECT_VALID(lib);
// Works.
EXPECT_EQ(4, SimpleInvoke(lib, "mainInt"));
// Results in an error.
EXPECT_ERROR(SimpleInvokeError(lib, "main"), "importedFunc");
Dart_Handle lib = TestCase::LoadTestScript(kScript, nullptr);
EXPECT_ERROR(lib, "Compilation failed");
// Import 'test:lib1' with importedFunc visible. Will result in
// an error when 'mainInt' is invoked.
const char* kReloadScript =
// a compile-time error.
const char* kScript2 =
"import 'test:lib1' show importedFunc;\n"
"main() {\n"
" return importedFunc();\n"
@@ -1188,8 +1181,24 @@ TEST_CASE(IsolateReload_LibraryShow) {
" return importedIntFunc();\n"
"}\n";
lib = TestCase::ReloadTestScript(kReloadScript);
EXPECT_ERROR(lib, "importedIntFunc");
lib = TestCase::LoadTestScript(kScript2, nullptr);
EXPECT_ERROR(lib, "Compilation failed");
// Both imports 'test:lib1' are visible.
// Should be successful.
const char* kScript3 =
"import 'test:lib1' show importedFunc, importedIntFunc;\n"
"main() {\n"
" return importedFunc();\n"
"}\n"
"@pragma('vm:entry-point', 'call')\n"
"mainInt() {\n"
" return importedIntFunc();\n"
"}\n";
lib = TestCase::LoadTestScript(kScript3, nullptr);
EXPECT_VALID(lib);
EXPECT_EQ(4, SimpleInvoke(lib, "mainInt"));
}
// Verifies that we clear the ICs for the functions live on the stack in a way
runtime/vm/kernel_isolate.cc
-3
View File
@@ -988,9 +988,6 @@ class KernelCompilationRequest : public ValueObject {
if (result_.status == Dart_KernelCompilationStatus_Ok) {
LoadKernelFromResponse(response[1]);
} else {
if (result_.status == Dart_KernelCompilationStatus_Error) {
LoadKernelFromResponse(response[2]);
}
// This is an error.
ASSERT(response[1]->type == Dart_CObject_kString);
result_.error = Utils::StrDup(response[1]->value.as_string);
runtime/vm/source_report_test.cc
+2 -59
View File
@@ -10,15 +10,11 @@ namespace dart {
#ifndef PRODUCT
static ObjectPtr ExecuteScript(const char* script, bool allow_errors = false) {
static ObjectPtr ExecuteScript(const char* script) {
Dart_Handle lib;
{
TransitionVMToNative transition(Thread::Current());
if (allow_errors) {
lib = TestCase::LoadTestScriptWithErrors(script, nullptr);
} else {
lib = TestCase::LoadTestScript(script, nullptr);
}
lib = TestCase::LoadTestScript(script, nullptr);
EXPECT_VALID(lib);
Dart_Handle result = Dart_Invoke(lib, NewString("main"), 0, nullptr);
EXPECT_VALID(result);
@@ -315,59 +311,6 @@ ISOLATE_UNIT_TEST_CASE(SourceReport_Coverage_UnusedClass_ForceCompile) {
buffer);
}
ISOLATE_UNIT_TEST_CASE(SourceReport_Coverage_UnusedClass_ForceCompileError) {
// WARNING: This MUST be big enough for the serialized JSON string.
const int kBufferSize = 1024;
char buffer[kBufferSize];
const char* kScript =
"helper0() {}\n"
"class Unused {\n"
" helper1() { helper0()+ }\n" // syntax error
"}\n"
"main() {\n"
" helper0();\n"
"}";
Library& lib = Library::Handle();
lib ^= ExecuteScript(kScript, true);
ASSERT(!lib.IsNull());
const Script& script =
Script::Handle(lib.LookupScript(String::Handle(String::New("test-lib"))));
SourceReport report(SourceReport::kCoverage, SourceReport::kForceCompile);
JSONStream js;
js.set_id_zone(thread->isolate()->EnsureDefaultServiceIdZone());
report.PrintJSON(&js, script);
const char* json_str = js.ToCString();
ASSERT(strlen(json_str) < kBufferSize);
ElideJSONSubstring("classes", json_str, buffer);
ElideJSONSubstring("libraries", buffer, buffer);
EXPECT_STREQ(
"{\"type\":\"SourceReport\",\"ranges\":["
// UnusedClass has a syntax error.
"{\"scriptIndex\":0,\"startPos\":30,\"endPos\":53,\"compiled\":false,"
"\"error\":{\"type\":\"@Error\",\"_vmType\":\"LanguageError\","
"\"kind\":\"LanguageError\",\"id\":\"objects\\/0\\/0\","
"\"message\":\"'file:\\/\\/\\/test-lib': error: "
"\\/test-lib:3:26: "
"Error: This couldn't be parsed.\\n"
" helper1() { helper0()+ }\\n ^\"}},"
// helper0 is compiled.
"{\"scriptIndex\":0,\"startPos\":0,\"endPos\":11,\"compiled\":true,"
"\"coverage\":{\"hits\":[0],\"misses\":[]}},"
// One range with two hits (main).
"{\"scriptIndex\":0,\"startPos\":57,\"endPos\":79,\"compiled\":true,"
"\"coverage\":{\"hits\":[57,68],\"misses\":[]}}],"
// Only one script in the script table.
"\"scripts\":[{\"type\":\"@Script\",\"fixedId\":true,\"id\":\"\","
"\"uri\":\"file:\\/\\/\\/test-lib\",\"_kind\":\"kernel\"}]}",
buffer);
}
ISOLATE_UNIT_TEST_CASE(SourceReport_Coverage_LibrariesAlreadyCompiled) {
// WARNING: This MUST be big enough for the serialized JSON string.
const int kBufferSize = 1024;
runtime/vm/unit_test.cc
+13 -36
View File
@@ -299,7 +299,6 @@ char* TestCase::CompileTestScriptWithDFE(const char* url,
const uint8_t** kernel_buffer,
intptr_t* kernel_buffer_size,
bool incrementally,
bool allow_compile_errors,
const char* multiroot_filepaths,
const char* multiroot_scheme) {
// clang-format off
@@ -313,8 +312,8 @@ char* TestCase::CompileTestScriptWithDFE(const char* url,
// clang-format on
return CompileTestScriptWithDFE(
url, sizeof(sourcefiles) / sizeof(Dart_SourceFile), sourcefiles,
kernel_buffer, kernel_buffer_size, incrementally, allow_compile_errors,
multiroot_filepaths, multiroot_scheme);
kernel_buffer, kernel_buffer_size, incrementally, multiroot_filepaths,
multiroot_scheme);
}
char* TestCase::CompileTestScriptWithDFE(const char* url,
@@ -323,7 +322,6 @@ char* TestCase::CompileTestScriptWithDFE(const char* url,
const uint8_t** kernel_buffer,
intptr_t* kernel_buffer_size,
bool incrementally,
bool allow_compile_errors,
const char* multiroot_filepaths,
const char* multiroot_scheme) {
Zone* zone = Thread::Current()->zone();
@@ -340,32 +338,26 @@ char* TestCase::CompileTestScriptWithDFE(const char* url,
}
}
return ValidateCompilationResult(zone, result, kernel_buffer,
kernel_buffer_size, allow_compile_errors);
kernel_buffer_size);
}
char* TestCase::ValidateCompilationResult(
Zone* zone,
Dart_KernelCompilationResult compilation_result,
const uint8_t** kernel_buffer,
intptr_t* kernel_buffer_size,
bool allow_compile_errors) {
if (!allow_compile_errors &&
(compilation_result.status != Dart_KernelCompilationStatus_Ok)) {
intptr_t* kernel_buffer_size) {
if (compilation_result.status != Dart_KernelCompilationStatus_Ok) {
ASSERT(compilation_result.kernel == nullptr);
char* result =
OS::SCreate(zone, "Compilation failed %s", compilation_result.error);
free(compilation_result.error);
if (compilation_result.kernel != nullptr) {
free(const_cast<uint8_t*>(compilation_result.kernel));
}
*kernel_buffer = nullptr;
*kernel_buffer_size = 0;
return result;
}
ASSERT(compilation_result.error == nullptr);
*kernel_buffer = compilation_result.kernel;
*kernel_buffer_size = compilation_result.kernel_size;
if (compilation_result.error != nullptr) {
free(compilation_result.error);
}
if (kernel_buffer == nullptr) {
return OS::SCreate(zone, "front end generated a nullptr kernel file");
}
@@ -425,25 +417,15 @@ static intptr_t BuildSourceFilesArray(
return num_test_libs + 1;
}
Dart_Handle TestCase::LoadTestScriptWithErrors(
const char* script,
Dart_NativeEntryResolver resolver,
const char* lib_url,
bool finalize_classes) {
return LoadTestScript(script, resolver, lib_url, finalize_classes, true);
}
Dart_Handle TestCase::LoadTestScript(const char* script,
Dart_NativeEntryResolver resolver,
const char* lib_url,
bool finalize_classes,
bool allow_compile_errors) {
bool finalize_classes) {
LoadIsolateReloadTestLibIfNeeded(script);
Dart_SourceFile* sourcefiles = nullptr;
intptr_t num_sources = BuildSourceFilesArray(&sourcefiles, script, lib_url);
Dart_Handle result =
LoadTestScriptWithDFE(num_sources, sourcefiles, resolver,
finalize_classes, true, allow_compile_errors);
Dart_Handle result = LoadTestScriptWithDFE(num_sources, sourcefiles, resolver,
finalize_classes, true);
delete[] sourcefiles;
return result;
}
@@ -465,7 +447,7 @@ Dart_Handle TestCase::LoadTestLibrary(const char* lib_uri,
char* error = TestCase::CompileTestScriptWithDFE(
sourcefiles[0].uri, sourcefiles_count, sourcefiles, &kernel_buffer,
&kernel_buffer_size, true);
if ((kernel_buffer == nullptr) && (error != nullptr)) {
if (error != nullptr) {
return Dart_NewApiError(error);
}
@@ -492,7 +474,6 @@ Dart_Handle TestCase::LoadTestScriptWithDFE(int sourcefiles_count,
Dart_NativeEntryResolver resolver,
bool finalize,
bool incrementally,
bool allow_compile_errors,
const char* entry_script_uri,
const char* multiroot_filepaths,
const char* multiroot_scheme) {
@@ -504,9 +485,8 @@ Dart_Handle TestCase::LoadTestScriptWithDFE(int sourcefiles_count,
char* error = TestCase::CompileTestScriptWithDFE(
entry_script_uri != nullptr ? entry_script_uri : sourcefiles[0].uri,
sourcefiles_count, sourcefiles, &kernel_buffer, &kernel_buffer_size,
incrementally, allow_compile_errors, multiroot_filepaths,
multiroot_scheme);
if ((kernel_buffer == nullptr) && error != nullptr) {
incrementally, multiroot_filepaths, multiroot_scheme);
if (error != nullptr) {
return Dart_NewApiError(error);
}
@@ -620,9 +600,6 @@ Dart_Handle TestCase::ReloadTestScript(const char* script) {
if (compilation_result.status != Dart_KernelCompilationStatus_Ok) {
Dart_Handle result = Dart_NewApiError(compilation_result.error);
free(compilation_result.error);
if (compilation_result.kernel != nullptr) {
free(const_cast<uint8_t*>(compilation_result.kernel));
}
return result;
}
runtime/vm/unit_test.h
+1 -11
View File
@@ -330,7 +330,6 @@ class TestCase : TestCaseBase {
const uint8_t** kernel_buffer,
intptr_t* kernel_buffer_size,
bool incrementally = true,
bool allow_compile_errors = false,
const char* multiroot_filepaths = nullptr,
const char* multiroot_scheme = nullptr);
static char* CompileTestScriptWithDFE(
@@ -340,19 +339,12 @@ class TestCase : TestCaseBase {
const uint8_t** kernel_buffer,
intptr_t* kernel_buffer_size,
bool incrementally = true,
bool allow_compile_errors = false,
const char* multiroot_filepaths = nullptr,
const char* multiroot_scheme = nullptr);
static Dart_Handle LoadTestScript(
const char* script,
Dart_NativeEntryResolver resolver,
const char* lib_uri = RESOLVED_USER_TEST_URI,
bool finalize = true,
bool allow_compile_errors = false);
static Dart_Handle LoadTestScriptWithErrors(
const char* script,
Dart_NativeEntryResolver resolver = nullptr,
const char* lib_uri = RESOLVED_USER_TEST_URI,
bool finalize = true);
static Dart_Handle LoadTestLibrary(
const char* lib_uri,
@@ -364,7 +356,6 @@ class TestCase : TestCaseBase {
Dart_NativeEntryResolver resolver = nullptr,
bool finalize = true,
bool incrementally = true,
bool allow_compile_errors = false,
const char* entry_script_uri = nullptr,
const char* multiroot_filepaths = nullptr,
const char* multiroot_scheme = nullptr);
@@ -434,8 +425,7 @@ class TestCase : TestCaseBase {
static char* ValidateCompilationResult(Zone* zone,
Dart_KernelCompilationResult result,
const uint8_t** kernel_buffer,
intptr_t* kernel_buffer_size,
bool allow_compile_errors);
intptr_t* kernel_buffer_size);
RunEntry* const run_;
};