Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 4926236

Browse files
alexmarkovcommit-bot@chromium.org
authored andcommitted
[vm] Do expensive kernel metadata verification only once per kernel
In DEBUG mode during AOT compilation MetadataHelper::SetMetadataMappings may spend considerable time verifying that node offsets are sorted in kernel metadata. The problem is that there are a lot of MetadataHelpers created (several per each compiled function). This change moves verification of kernel metadata mappings to the kernel loader, so it is performed only once per kernel program. AOT gen_snapshot time on a small test: Before: 11.680s After: 9.041s Change-Id: I1c0a676b0ed28fc5cfa756ca60159f914190fac0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/103486 Commit-Queue: Alexander Markov <[email protected]> Reviewed-by: Aart Bik <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
1 parent cc2fac5 commit 4926236

File tree

3 files changed

+57
-19
lines changed

3 files changed

+57
-19
lines changed

runtime/vm/compiler/frontend/kernel_translation_helper.cc

Lines changed: 49 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,6 +1367,55 @@ void LibraryDependencyHelper::ReadUntilExcluding(Field field) {
13671367
}
13681368
}
13691369

1370+
#if defined(DEBUG)
1371+
1372+
void MetadataHelper::VerifyMetadataMappings(
1373+
const TypedDataBase& metadata_mappings) {
1374+
const intptr_t kUInt32Size = 4;
1375+
Reader reader(metadata_mappings);
1376+
if (reader.size() == 0) {
1377+
return;
1378+
}
1379+
1380+
// Scan through metadata mappings in reverse direction.
1381+
1382+
// Read metadataMappings length.
1383+
intptr_t offset = reader.size() - kUInt32Size;
1384+
const intptr_t metadata_num = reader.ReadUInt32At(offset);
1385+
1386+
if (metadata_num == 0) {
1387+
ASSERT(metadata_mappings.LengthInBytes() == kUInt32Size);
1388+
return;
1389+
}
1390+
1391+
// Read metadataMappings elements.
1392+
for (intptr_t i = 0; i < metadata_num; ++i) {
1393+
// Read nodeOffsetToMetadataOffset length.
1394+
offset -= kUInt32Size;
1395+
const intptr_t mappings_num = reader.ReadUInt32At(offset);
1396+
1397+
// Skip nodeOffsetToMetadataOffset.
1398+
offset -= mappings_num * 2 * kUInt32Size;
1399+
1400+
// Verify that node offsets are sorted.
1401+
intptr_t prev_node_offset = -1;
1402+
reader.set_offset(offset);
1403+
for (intptr_t j = 0; j < mappings_num; ++j) {
1404+
const intptr_t node_offset = reader.ReadUInt32();
1405+
const intptr_t md_offset = reader.ReadUInt32();
1406+
1407+
ASSERT(node_offset >= 0 && md_offset >= 0);
1408+
ASSERT(node_offset > prev_node_offset);
1409+
prev_node_offset = node_offset;
1410+
}
1411+
1412+
// Skip tag.
1413+
offset -= kUInt32Size;
1414+
}
1415+
}
1416+
1417+
#endif // defined(DEBUG)
1418+
13701419
MetadataHelper::MetadataHelper(KernelReaderHelper* helper,
13711420
const char* tag,
13721421
bool precompiler_only)
@@ -1386,25 +1435,6 @@ void MetadataHelper::SetMetadataMappings(intptr_t mappings_offset,
13861435
ASSERT((mappings_offset != 0) && (mappings_num != 0));
13871436
mappings_offset_ = mappings_offset;
13881437
mappings_num_ = mappings_num;
1389-
1390-
#ifdef DEBUG
1391-
// Verify that node offsets are sorted.
1392-
{
1393-
Reader reader(H.metadata_mappings());
1394-
reader.set_offset(mappings_offset);
1395-
1396-
intptr_t prev_node_offset = -1;
1397-
for (intptr_t i = 0; i < mappings_num; ++i) {
1398-
intptr_t node_offset = reader.ReadUInt32();
1399-
intptr_t md_offset = reader.ReadUInt32();
1400-
1401-
ASSERT((node_offset >= 0) && (md_offset >= 0));
1402-
ASSERT(node_offset > prev_node_offset);
1403-
prev_node_offset = node_offset;
1404-
}
1405-
}
1406-
#endif // DEBUG
1407-
14081438
last_node_offset_ = kIntptrMax;
14091439
last_mapping_index_ = 0;
14101440
}

runtime/vm/compiler/frontend/kernel_translation_helper.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,10 @@ class MetadataHelper {
803803
const char* tag,
804804
bool precompiler_only);
805805

806+
#if defined(DEBUG)
807+
static void VerifyMetadataMappings(const TypedDataBase& metadata_mappings);
808+
#endif
809+
806810
protected:
807811
// Look for metadata mapping with node offset greater or equal than the given.
808812
intptr_t FindMetadataMapping(intptr_t node_offset);

runtime/vm/kernel_loader.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,10 @@ void KernelLoader::InitializeFields(UriToSourceTable* uri_to_source_table) {
408408
program_->string_table_offset() -
409409
program_->metadata_mappings_offset()));
410410

411+
#if defined(DEBUG)
412+
MetadataHelper::VerifyMetadataMappings(metadata_mappings);
413+
#endif
414+
411415
const Array& libraries_cache =
412416
Array::Handle(Z, HashTables::New<UnorderedHashMap<SmiTraits>>(
413417
program_->library_count(), Heap::kOld));

0 commit comments

Comments
 (0)