Opportunisticly emit compact imports by tlively · Pull Request #8926 · WebAssembly/binaryen

tlively · 2026-07-22T03:12:47Z

In the binary writer when compact imports are enabled, opportunistically look for pairs of adjacent imports that share both their module names and import types, or alternatively just their module names. When a pair of matching imports is found, create a run of compact imports that includes as many subsequent imports as possible. Adding a pass that will reorder imports to purposefully put similar imports together is left as future work.

Update the parser for imports to support both forms of compact imports. Because imports are no longer 1:1 with import statements, the locations of imported items the parser collects and re-parses in various phases must now be the locations of importdesc productions rather than import productions. Since e.g. a function importdesc cannot be differentiated from a function definition with an empty body without context, we must add a mechanism for the parser context to tell the parser whether or not it is parsing an import.

When e.g. a function is declared in the text format, it can either be with a normal `func` production (which may or may not have an inline import declaration) or it can be an `import` production. Previously the parser would just store the location of the declaration but not the kind of production it used. In later parser phases where the declaration has to be parsed again, we would just try both productions to see which one worked. This is rather hacky and brittle and will not play nicely with the text format for compact imports, so refactor to explicitly track how the declaration should be parsed.

kripken · 2026-07-23T15:34:10Z

+    if (const auto* fa = std::get_if<Function*>(&a)) {
+      auto* fb = std::get<Function*>(b);
+      return (*fa)->type.isExact() == fb->type.isExact() &&
+             (*fa)->type.getHeapType() == fb->type.getHeapType();


Why isn't this just (*fa)->type == fb->type?

No reason, that would be a good simplification.

kripken · 2026-07-23T15:37:13Z

+      return (*ma)->initial == mb->initial && (*ma)->max == mb->max &&
+             (*ma)->hasMax() == mb->hasMax() && (*ma)->shared == mb->shared &&
+             (*ma)->is64() == mb->is64() &&
+             (*ma)->pageSizeLog2 == mb->pageSizeLog2;


We have MemoryUtils::isSubType - we can check a <= b && b <= a here? (maybe adding a helper for it in MemoryUtils?)

Sure, or if that method isn't appropriate due to the runtime interaction Steven mentioned, I can create another helper.

kripken · 2026-07-23T15:39:18Z

+      auto* tb = std::get<Table*>(b);
+      return (*ta)->type == tb->type && (*ta)->initial == tb->initial &&
+             (*ta)->max == tb->max && (*ta)->hasMax() == tb->hasMax() &&
+             (*ta)->is64() == tb->is64();


We have RuntimeTable::isSubType, which we could use like above? Though perhaps first with moving isSubType to TableUtils?)

(in these two comments I am trying to avoid duplicating this logic across the codebase, which would make later refactors harder/error prone)

FWIW isSubType is in RuntimeTable because it's not possible to statically determine whether one table is a subtype of another. table.grow will affect the subtyping relationship and it would be wrong to try to compare two table definitions for subtyping (this is a bug we previously had in the interpreter).

This is checking if two table definitions are equivalent statically, so I think we could keep this logic separate or introduce a new helper function for it.

Oh, good point. Perhaps we can share some of the logic between those, though?

kripken · 2026-07-23T15:41:38Z

 ;; Build to a binary wasm.
 ;;
-;; RUN: wasm-opt %s -o %t.wasm -q -all
+;; RUN: wasm-opt %s -o %t.wasm -q -all --disable-compact-imports


Why is this needed?

-all makes the output Wasm contain compact imports, but the V8 run later in this file does not enable compact imports. We could alternatively have fixed this by enabled compact imports in V8, which I suppose would have the benefit of showing that V8 parses them correctly. I'll change this.

is --wasm-staging not enough, btw, for v8 to support compact imports? That would be more general if so.

Turns out --wasm-staging is not enough. I'll specifically enable --wasm-compact-imports.

kripken · 2026-07-23T15:42:55Z

+        wasm_bytes = self.get_binary(wat, ['--enable-compact-imports'])
+        self.assertIn(b'\x03env\x00\x7e', wasm_bytes)
+        self.assertIn(b'\x04math\x00\x7f', wasm_bytes)
+        self.assertIn(b'\x06single\x02m1', wasm_bytes)


Maybe it would be good to add a code size test here, something like 1,000 procedurally-generated imports with the same module, and seeing how much smaller the binary size is with the feature enabled?

Sure, I guess that would catch accidental regressions.

stevenfontanella · 2026-07-23T17:47:51Z

+template<class... Ts> struct Overloaded : Ts... {
+  using Ts::operator()...;
+};
+template<class... Ts> Overloaded(Ts...) -> Overloaded<Ts...>;
+


This already exists in utilities.h.

Ah, thanks! I thought I had seen it before.

stevenfontanella · 2026-07-23T17:53:30Z

+  ModuleUtils::iterImportedFunctions(
+    *wasm, [&](Function* func) { imports.push_back(func); });
+  ModuleUtils::iterImportedGlobals(
+    *wasm, [&](Global* global) { imports.push_back(global); });
+  ModuleUtils::iterImportedTags(*wasm,
+                                [&](Tag* tag) { imports.push_back(tag); });
+  ModuleUtils::iterImportedMemories(
+    *wasm, [&](Memory* memory) { imports.push_back(memory); });
+  ModuleUtils::iterImportedTables(
+    *wasm, [&](Table* table) { imports.push_back(table); });


This can be simplified to

ModuleUtils::iterImports(*wasm, [&](ImportItem import) { imports.push_back(import); }

…-imports

stevenfontanella

LGTM if the fuzzer is happy

stevenfontanella · 2026-07-24T19:16:52Z

+    if (a.index() != b.index()) {
+      return false;
    }
-    o << U32LEB(kind) << U32LEB(getTypeIndex(func->type.getHeapType()));
-  });
-  ModuleUtils::iterImportedGlobals(*wasm, [&](Global* global) {
-    writeImportHeader(global);
-    o << U32LEB(int32_t(ExternalKind::Global));
-    writeType(global->type);
-    o << U32LEB(global->mutable_);
-  });
-  ModuleUtils::iterImportedTags(*wasm, [&](Tag* tag) {
-    writeImportHeader(tag);
-    o << U32LEB(int32_t(ExternalKind::Tag));
-    o << uint8_t(0); // Reserved 'attribute' field. Always 0.
-    o << U32LEB(getTypeIndex(tag->type));
-  });
-  ModuleUtils::iterImportedMemories(*wasm, [&](Memory* memory) {
-    writeImportHeader(memory);
-    o << U32LEB(int32_t(ExternalKind::Memory));
-    writeResizableLimits(memory->initial,
-                         memory->max,
-                         memory->hasMax(),
-                         memory->shared,
-                         memory->is64(),
-                         memory->pageSizeLog2);
-  });
-  ModuleUtils::iterImportedTables(*wasm, [&](Table* table) {
-    writeImportHeader(table);
-    o << U32LEB(int32_t(ExternalKind::Table));
-    writeType(table->type);
-    writeResizableLimits(table->initial,
-                         table->max,
-                         table->hasMax(),
-                         /*shared=*/false,
-                         table->is64());
-  });
+    if (const auto* fa = std::get_if<Function*>(&a)) {
+      auto* fb = std::get<Function*>(b);
+      return (*fa)->type == fb->type;
+    }
+    if (const auto* ga = std::get_if<Global*>(&a)) {
+      auto* gb = std::get<Global*>(b);
+      return (*ga)->type == gb->type && (*ga)->mutable_ == gb->mutable_;
+    }
+    if (const auto* ta = std::get_if<Tag*>(&a)) {
+      auto* tb = std::get<Tag*>(b);
+      return (*ta)->type == tb->type;
+    }
+    if (const auto* ma = std::get_if<Memory*>(&a)) {
+      auto* mb = std::get<Memory*>(b);
+      return MemoryUtils::sameType(**ma, *mb);
+    }
+    if (const auto* ta = std::get_if<Table*>(&a)) {
+      auto* tb = std::get<Table*>(b);
+      return TableUtils::sameType(**ta, *tb);
+    }
+    return false;
+  };


nit: I think this comes out cleaner as std::visit:

return std::visit( overloaded{ [](Function* fa, Function* fb) { return fa->type == fb->type; }, [](Global* ga, Global* gb) { return ga->type == gb->type && ga->mutable_ == gb->mutable_; }, [](Tag* ta, Tag* tb) { return ta->type == tb->type; }, [](Memory* ma, Memory* mb) { return MemoryUtils::sameType(*ma, *mb); }, [](Table* ta, Table* tb) { return TableUtils::sameType(*ta, *tb); }, [](const auto&, const auto&) { return false; } }, a, b);

stevenfontanella · 2026-07-24T19:19:22Z

+
+
+class CompactImportsTest(utils.BinaryenTestCase):
+    def get_binary(self, wat_str, flags=[]):


nit: maybe avoid the mutable default param (flags)? Although there's no bug here since flags is never mutated, I think it's better to avoid it in case the code changes in the future or gets copy-pasted.

https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments

stevenfontanella · 2026-07-24T19:29:25Z

+      case ImportGroup::SharedAll: {
+        const auto& first = imports[group.start];
+        writeInlineString(getModule(first).view());
+        writeInlineString("");


What is this for? Looks like it's the same as just o << 0?

Yes, but having an empty name here is how the encoding of compact imports is specified.

stevenfontanella · 2026-07-24T19:31:18Z

+        writeInlineString(getModule(item).view());
+        writeInlineString(getBase(item).view());
+        writeImportDesc(item);
+        continue;


All of the cases continue, so maybe we can change it to break to make the control flow more clear

kripken · 2026-07-24T19:50:25Z

 ;; Run in node.
 ;;
-;; RUN: v8 %S/../../../scripts/fuzz_shell.js -- %t.wasm | filecheck %s
+;; RUN: v8 --wasm-compact-imports %S/../../../scripts/fuzz_shell.js -- %t.wasm | filecheck %s


How about removing this and adding explicit features to wasm-opt, above? That will make sense in the long term, while this will get obsolete and might need more changes for other future features.

kripken · 2026-07-24T19:51:42Z

+        with_compact = self.get_binary(wat, ['--enable-compact-imports'])
+        without_compact = self.get_binary(wat, ['--disable-compact-imports'])
+        self.assertEqual(len(with_compact), 2030)
+        self.assertEqual(len(without_compact), 8021)


I would maybe just check the ratio rather than an exact measurement? something like without/with >= 3.9

kripken

lgtm otherwise

tlively added 7 commits July 21, 2026 18:34

[WIP SLOP] Opportunisticly emit compact imports

8bec206

Merge branch 'prefactor-compact-imports-text' into compact-imports-text

9dde504

Merge branch 'compact-imports-text' into opportunistic-compact-imports

a48434a

disable compact imports in fuzz_shell test

8b85234

update

3872ec1

tlively changed the title ~~[WIP SLOP] Opportunisticly emit compact imports~~ Opportunisticly emit compact imports Jul 23, 2026

tlively marked this pull request as ready for review July 23, 2026 01:56

tlively requested a review from a team as a code owner July 23, 2026 01:56

tlively requested review from kripken and removed request for a team July 23, 2026 01:56

kripken reviewed Jul 23, 2026

View reviewed changes

stevenfontanella reviewed Jul 23, 2026

View reviewed changes

tlively added 7 commits July 23, 2026 12:01

Merge remote-tracking branch 'origin/main' into compact-imports-text

fe0bde1

Merge branch 'compact-imports-text' into opportunistic-compact-imports

c0a7ee9

address feedback

cb7e598

remove incorrect assertion

1809915

try to fix build

0d52404

fix ruff errors

b787866

fix tests

3e565b3

Base automatically changed from compact-imports-text to main July 24, 2026 18:23

Merge remote-tracking branch 'origin/main' into opportunistic-compact…

aab737c

…-imports

tlively requested review from kripken and stevenfontanella July 24, 2026 18:38

stevenfontanella approved these changes Jul 24, 2026

View reviewed changes

kripken reviewed Jul 24, 2026

View reviewed changes

kripken approved these changes Jul 24, 2026

View reviewed changes

tlively added 5 commits July 24, 2026 13:52

more std::visit

73835f5

no optional python list param

3e4db28

continue => break

1091343

explicit features

a2b7f79

assert ratio

6309acc

tlively enabled auto-merge (squash) July 24, 2026 21:13

tlively disabled auto-merge July 24, 2026 21:18

kripken approved these changes Jul 24, 2026

View reviewed changes

tlively merged commit 51db223 into main Jul 24, 2026
16 checks passed

tlively deleted the opportunistic-compact-imports branch July 24, 2026 22:02



		class CompactImportsTest(utils.BinaryenTestCase):
		def get_binary(self, wat_str, flags=[]):

Uh oh!

Conversation

tlively commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevenfontanella left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kripken left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tlively commented Jul 22, 2026 •

edited

Loading