-
Notifications
You must be signed in to change notification settings - Fork 871
Opportunisticly emit compact imports #8926
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
08634b4
8bec206
e401ce0
9dde504
a48434a
8b85234
3872ec1
fe0bde1
c0a7ee9
cb7e598
1809915
0d52404
b787866
3e565b3
aab737c
73835f5
3e4db28
1091343
a2b7f79
6309acc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| from scripts.test import shared | ||
|
|
||
| from . import utils | ||
|
|
||
|
|
||
| class CompactImportsTest(utils.BinaryenTestCase): | ||
| def get_binary(self, wat_str, flags): | ||
| cmd = shared.WASM_OPT + ['-o', '-'] + flags | ||
| p = shared.run_process( | ||
| cmd, input=wat_str, check=True, capture_output=True, decode_output=False, | ||
| ) | ||
| return p.stdout | ||
|
|
||
| def test_shared_all_encoding(self): | ||
| wat = '''(module | ||
| (type $sig (func (param i32) (result i32))) | ||
| (import "env" "f1" (func (type $sig))) | ||
| (import "env" "f2" (func (type $sig))) | ||
| )''' | ||
| wasm_bytes = self.get_binary(wat, ['--enable-compact-imports']) | ||
| # 0x7E is CompactImportsSharedAll | ||
| self.assertIn(b'\x03env\x00\x7e', wasm_bytes) | ||
|
|
||
| def test_shared_module_encoding(self): | ||
| wat = '''(module | ||
| (type $sig1 (func (param i32) (result i32))) | ||
| (type $sig2 (func (param f64) (result f64))) | ||
| (import "env" "f1" (func (type $sig1))) | ||
| (import "env" "f2" (func (type $sig2))) | ||
| )''' | ||
| wasm_bytes = self.get_binary(wat, ['--enable-compact-imports']) | ||
| # 0x7F is CompactImportsSharedModule | ||
| self.assertIn(b'\x03env\x00\x7f', wasm_bytes) | ||
|
|
||
| def test_disabled_compact_imports(self): | ||
| wat = '''(module | ||
| (type $sig (func (param i32) (result i32))) | ||
| (import "env" "f1" (func (type $sig))) | ||
| (import "env" "f2" (func (type $sig))) | ||
| )''' | ||
| wasm_bytes = self.get_binary(wat, ['--disable-compact-imports']) | ||
| self.assertNotIn(b'\x03env\x00\x7e', wasm_bytes) | ||
| self.assertNotIn(b'\x03env\x00\x7f', wasm_bytes) | ||
| self.assertIn(b'\x03env\x02f1', wasm_bytes) | ||
| self.assertIn(b'\x03env\x02f2', wasm_bytes) | ||
|
|
||
| def test_mixed_import_patterns(self): | ||
| wat = '''(module | ||
| (type $sig1 (func (param i32) (result i32))) | ||
| (type $sig2 (func (param f64) (result f64))) | ||
| ;; Run 1: SharedAll for "env" | ||
| (import "env" "f1" (func (type $sig1))) | ||
| (import "env" "f2" (func (type $sig1))) | ||
| ;; Run 2: SharedModule for "math" (different types) | ||
| (import "math" "sin" (func (type $sig1))) | ||
| (import "math" "sqrt" (func (type $sig2))) | ||
| ;; Run 3: Single import for "single" | ||
| (import "single" "m1" (memory 1 2)) | ||
| )''' | ||
| 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) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I guess that would catch accidental regressions. |
||
|
|
||
| def test_identical_imports_size_reduction(self): | ||
| imports = '\n'.join(['(import "env" "f" (func (type $sig)))'] * 1000) | ||
| wat = f'''(module | ||
| (type $sig (func (param i32) (result i32))) | ||
| {imports} | ||
| )''' | ||
| with_compact = self.get_binary(wat, ['--enable-compact-imports']) | ||
| without_compact = self.get_binary(wat, ['--disable-compact-imports']) | ||
| self.assertTrue(len(without_compact) / len(with_compact) > 3.9) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for? Looks like it's the same as just
o << 0?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but having an empty name here is how the encoding of compact imports is specified.