perf: add wazero compilation cache and wasm guard improvements#2956
Merged
perf: add wazero compilation cache and wasm guard improvements#2956
Conversation
Implement all findings from Go Fan report (#2829): 1. Process-level compilation cache: shared globalCompilationCache eliminates redundant JIT compilation across WasmGuard instances. 2. CompilationCache field in WasmGuardOptions: allows injection of custom caches for testing or per-deployment tuning. 3. TestMain with cache cleanup: properly closes the global cache after the test suite runs. 4. Consistent Close() error handling: use errors.Join to surface both module and runtime close errors instead of silently swallowing module errors. 5. Buffer retry strategy comment: documents the -2 error code convention and adaptive buffer growth protocol. 6. Remove redundant ExportedFunction("label_agent") check in LabelAgent: already verified at construction time and again inside callWasmFunction. Closes #2829 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves wazero-based WASM guard performance and reliability by introducing a shared compilation cache, making the cache injectable via options, and tightening/clarifying runtime behavior (Close error reporting, retry strategy documentation, and removing a redundant export check).
Changes:
- Added a process-wide
wazero.CompilationCacheand plumbed it into runtime construction, with an override inWasmGuardOptions. - Improved
WasmGuard.Close()to return both module and runtime close errors viaerrors.Join. - Added documentation for the adaptive output-buffer retry protocol and updated tests with a
TestMaincache lifecycle hook plus cache-related tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
internal/guard/wasm.go |
Adds global + injectable compilation cache, improves Close() error surfacing, documents buffer retry convention, removes redundant export check. |
internal/guard/wasm_test.go |
Adds TestMain to close the global cache after tests and introduces compilation-cache-focused tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+20
to
+24
| func TestMain(m *testing.M) { | ||
| code := m.Run() | ||
| globalCompilationCache.Close(context.Background()) | ||
| os.Exit(code) | ||
| } |
There was a problem hiding this comment.
TestMain closes globalCompilationCache but ignores the returned error. If Close can fail (e.g., for future disk-backed caches or internal cleanup), the test process would still exit with a success code. Consider checking the error and failing the test run (e.g., printing to stderr and using a non-zero exit code) so cache lifecycle issues are surfaced.
- Add DisableCompilationCache bool to WasmGuardOptions for explicit opt-out of caching (avoids unbounded memory in long-lived processes) - Check and surface error from globalCompilationCache.Close() in TestMain - Replace unobservable cache tests with disk-backed assertions that verify cache artifacts are actually written to the expected directory Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c5576df to
d75033c
Compare
This was referenced Mar 31, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Go Fan report (#2829) identified several improvement opportunities in the wazero/WASM guard implementation: missing compilation cache, inconsistent
Close()error handling, undocumented buffer retry protocol, and a redundant function export check.Changes
All 6 findings addressed:
High Priority
WasmGuardre-JITs the WASM binaryglobalCompilationCache(package-level, goroutine-safe) shared across all instancesCompilationCachefield toWasmGuardOptionsMedium Priority
TestMainfor cache lifecycleTestMainthat closes the global cache after testsLow Priority (Quick Wins)
Close()swallows module errors, only returns runtime errorserrors.Jointo surface both-2error code, adaptive growth) is undocumentedcallWasmFunctionExportedFunction("label_agent")checked redundantly inLabelAgentcallWasmFunctionFiles Changed
internal/guard/wasm.gointernal/guard/wasm_test.goTesting
make agent-finishedpasses. All changes are backward-compatible — existing callers that passnilopts automatically benefit from the shared cache.Closes #2829