Skip to content

Commit 4227e60

Browse files
authored
[NFC] Combine the two binary reading scans of sections into one (#7577)
Rather than scan for DWARF and code annotations, and then scan for names, it is simpler to do them in a single loop. I don't see a speedup (I guess being in cache makes it not matter), but the code is simpler at least, avoiding the two loops.
1 parent f634373 commit 4227e60

File tree

2 files changed

+12
-51
lines changed

2 files changed

+12
-51
lines changed

src/wasm-binary.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1668,7 +1668,7 @@ class WasmBinaryReader {
16681668
void readTags();
16691669

16701670
static Name escape(Name name);
1671-
void findAndReadNames();
1671+
void readNames(size_t sectionPos, size_t payloadLen);
16721672
void readFeatures(size_t payloadLen);
16731673
void readDylink(size_t payloadLen);
16741674
void readDylink0(size_t payloadLen);

src/wasm/wasm-binary.cpp

Lines changed: 11 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1939,24 +1939,21 @@ void WasmBinaryReader::preScan() {
19391939
if (sectionCode == BinaryConsts::Section::Custom) {
19401940
auto sectionName = getInlineString();
19411941

1942-
// Code annotations require code locations.
1943-
// TODO: For Branch Hinting, we could note which functions require
1944-
// code locations, as an optimization.
19451942
if (sectionName == Annotations::BranchHint) {
1943+
// Code annotations require code locations.
1944+
// TODO: For Branch Hinting, we could note which functions require
1945+
// code locations, as an optimization.
19461946
needCodeLocations = true;
1947-
// Do not break, so we keep looking for DWARF.
1948-
}
1949-
1950-
// DWARF sections contain code offsets.
1951-
if (DWARF && Debug::isDWARFSection(sectionName)) {
1947+
} else if (DWARF && Debug::isDWARFSection(sectionName)) {
1948+
// DWARF sections contain code offsets.
19521949
needCodeLocations = true;
19531950
foundDWARF = true;
1954-
break;
1951+
} else if (debugInfo &&
1952+
sectionName == BinaryConsts::CustomSections::Name) {
1953+
readNames(oldPos, payloadLen);
19551954
}
1956-
1957-
// TODO: We could stop early if we see the Code section and DWARF is
1958-
// disabled, as BranchHint must appear first, but this seems to
1959-
// make practically no difference in practice.
1955+
// TODO: We could stop early in some cases, if we've seen enough (e.g.
1956+
// seeing Code implies no BranchHint will appear, due to ordering).
19601957
}
19611958
pos = oldPos + payloadLen;
19621959
}
@@ -1973,14 +1970,6 @@ void WasmBinaryReader::preScan() {
19731970

19741971
void WasmBinaryReader::read() {
19751972
preScan();
1976-
1977-
// Skip ahead and read the name section so we know what names to use when we
1978-
// construct module elements.
1979-
// TODO: Combine this pre-scan with the one in preScan().
1980-
if (debugInfo) {
1981-
findAndReadNames();
1982-
}
1983-
19841973
readHeader();
19851974
sourceMapReader.parse(wasm);
19861975

@@ -4942,32 +4931,7 @@ class NameProcessor {
49424931

49434932
} // anonymous namespace
49444933

4945-
void WasmBinaryReader::findAndReadNames() {
4946-
// Find the names section. Skip the magic and version.
4947-
assert(pos == 0);
4948-
getInt32();
4949-
getInt32();
4950-
Index payloadLen, sectionPos;
4951-
bool found = false;
4952-
while (more()) {
4953-
uint8_t sectionCode = getInt8();
4954-
payloadLen = getU32LEB();
4955-
sectionPos = pos;
4956-
if (sectionCode == BinaryConsts::Section::Custom) {
4957-
auto sectionName = getInlineString();
4958-
if (sectionName.equals(BinaryConsts::CustomSections::Name)) {
4959-
found = true;
4960-
break;
4961-
}
4962-
}
4963-
pos = sectionPos + payloadLen;
4964-
}
4965-
if (!found) {
4966-
// No names section to read.
4967-
pos = 0;
4968-
return;
4969-
}
4970-
4934+
void WasmBinaryReader::readNames(size_t sectionPos, size_t payloadLen) {
49714935
// Read the names.
49724936
uint32_t lastType = 0;
49734937
while (pos < sectionPos + payloadLen) {
@@ -5092,9 +5056,6 @@ void WasmBinaryReader::findAndReadNames() {
50925056
if (pos != sectionPos + payloadLen) {
50935057
throwError("bad names section position change");
50945058
}
5095-
5096-
// Reset the position; we were just reading ahead.
5097-
pos = 0;
50985059
}
50995060

51005061
void WasmBinaryReader::readFeatures(size_t payloadLen) {

0 commit comments

Comments
 (0)