Optimize WASM plugin execution with compilation caching by goccy · Pull Request #363 · mercari/grpc-federation

goccy · 2026-03-03T01:35:20Z

Optimize WASM plugin execution with compilation caching
If result.Files does not exist, we will not run the code generator plugin

Copilot

Pull request overview

This PR optimizes WASM plugin execution by caching compiled WASM modules and their runtimes so that the expensive compilation step is performed only once per plugin path during the lifetime of a Generator instance. It also skips code generator plugin execution entirely when result.Files is empty.

Changes:

Introduced wasmPlugin (compiled module + runtime, reusable) and wasmPluginCache (path-keyed map) in wasm.go, replacing the previous one-shot evalCodeGeneratorPlugin function.
Extended Generator with a long-lived wasmPluginCache, added a Close method, and threaded the cache through all evalAllCodeGenerationPlugin call sites.
Added an early-return in CreateCodeGeneratorResponse when result.Files is empty, avoiding unnecessary plugin invocations.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`generator/wasm.go`	New `wasmPlugin` / `wasmPluginCache` types replacing the old `evalCodeGeneratorPlugin` function; handles compile-once-execute-many pattern
`generator/generator.go`	Adds `wasmPluginCache` field to `Generator`, a `Close` method for cleanup, passes cache to all `evalAllCodeGenerationPlugin` calls, and adds early-exit guard on empty `result.Files`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

generator/wasm.go

Include opt.Sha256 in the cache key so that the same path with different hash values does not bypass hash verification. Return errors from Close methods using errors.Join instead of silently discarding them. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

generator/generator.go

generator/wasm.go

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

utahta · 2026-03-03T03:16:37Z

generator/generator.go

 		fmt.Fprint(os.Stderr, validator.Format(outs))
 	}
+	if len(result.Files) == 0 {
+		return nil, nil


Since returning nil can make protoc treat the plugin as not supporting FEATURE_PROTO3_OPTIONAL, should we return an empty response with SupportedFeatures set?

- Use content-derived sha256 as cache key instead of user-provided value - Make wasmPluginCache thread-safe with sync.RWMutex - Add defer g.Close(ctx) in CLI entry point - Return SupportedFeatures even when result.Files is empty Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-03T04:48:01Z

generator/wasm.go

+func (c *wasmPluginCache) getOrCreate(ctx context.Context, opt *WasmPluginOption) (*wasmPlugin, error) {
+	wasmFile, err := os.ReadFile(opt.Path)
+	if err != nil {
+		return nil, fmt.Errorf("grpc-federation: failed to read plugin file: %s: %w", opt.Path, err)
+	}
+	hash := sha256.Sum256(wasmFile)
+	gotHash := hex.EncodeToString(hash[:])
+	if opt.Sha256 != "" && opt.Sha256 != gotHash {
+		return nil, fmt.Errorf(
+			`grpc-federation: expected plugin sha256 value is [%s] but got [%s]`,
+			opt.Sha256,
+			gotHash,
+		)
+	}
+	cacheKey := opt.Path + ":" + gotHash
+
+	c.mu.RLock()
+	if wp, ok := c.plugins[cacheKey]; ok {
+		c.mu.RUnlock()
+		return wp, nil
+	}
+	c.mu.RUnlock()


The getOrCreate method reads the entire WASM file from disk and computes its SHA-256 hash on every call, even when the plugin is already present in the cache. The cache lookup only happens after the file I/O and hashing, meaning every invocation incurs the cost of a full file read and SHA-256 computation regardless of whether the compiled module is already cached.

For a hot path (e.g., watch mode processing many files), this means repeated disk reads of potentially large WASM binaries. The optimisation intended by compilation caching is significantly undermined by this per-call file read.

Consider checking the cache keyed by opt.Path first (with a read lock), and only reading the file and computing the hash when a cache miss occurs. Alternatively, if a constant hash per path is acceptable, the SHA-256 could be cached alongside the plugin to avoid repeated computation.

Copilot · 2026-03-03T04:48:01Z

cmd/grpc-federation-generator/main.go

 		protoPath = args[0]
 	}
 	g := generator.New(cfg)
+	defer g.Close(ctx)


The error returned by g.Close(ctx) is silently discarded because defer does not propagate the return value. If closing a cached WASM runtime fails (e.g., due to a resource leak), the caller has no way to detect or log that failure.

Consider logging the error from Close if it is non-nil, similar to how other error conditions are handled in _main. For example: defer func() { if err := g.Close(ctx); err != nil { log.Printf("failed to close generator: %+v", err) } }().

Suggested change

defer g.Close(ctx)

defer func() {

if err := g.Close(ctx); err != nil {

log.Printf("failed to close generator: %+v", err)

}

}()

atkin-mc · 2026-03-03T03:35:39Z

generator/wasm.go

+type wasmPlugin struct {
+	runtime  wazero.Runtime
+	compiled wazero.CompiledModule
+	counter  atomic.Int64


Can you add a comment explaining what this counter is used for, please?

generator/generator.go

github-actions · 2026-03-03T07:09:53Z

Code Metrics Report

	main (5eeb102)	#363 (d80eab1)	+/-
Coverage	67.9%	67.8%	-0.2%
Code to Test Ratio	1:0.4	1:0.4	-0.1
Test Execution Time	6m51s	6m58s	+7s

Details

  |                     | main (5eeb102) | #363 (d80eab1) |  +/-  |
  |---------------------|----------------|----------------|-------|
- | Coverage            |          67.9% |          67.8% | -0.2% |
  |   Files             |             84 |             84 |     0 |
  |   Lines             |          15582 |          15614 |   +32 |
+ |   Covered           |          10587 |          10588 |    +1 |
- | Code to Test Ratio  |          1:0.4 |          1:0.4 |  -0.1 |
  |   Code              |          48318 |          48381 |   +63 |
  |   Test              |          20407 |          20407 |     0 |
- | Test Execution Time |          6m51s |          6m58s |   +7s |

Code coverage of files in pull request scope (31.0% → 29.4%)

Files	Coverage	+/-	Status
generator/generator.go	32.9%	+0.3%	modified
generator/wasm.go	1.5%	+1.5%	modified

Reported by octocov

optimize WASM plugin execution with compilation caching

be86cf7

Copilot AI review requested due to automatic review settings March 3, 2026 01:35

Copilot started reviewing on behalf of goccy March 3, 2026 01:35 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

generator/wasm.go Outdated Show resolved Hide resolved

generator/wasm.go Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

goccy requested a review from Copilot March 3, 2026 02:23

Copilot started reviewing on behalf of goccy March 3, 2026 02:23 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

generator/generator.go Show resolved Hide resolved

generator/wasm.go Show resolved Hide resolved

goccy requested a review from Copilot March 3, 2026 02:56

Copilot started reviewing on behalf of goccy March 3, 2026 02:57 View session

This comment has been minimized.

Sign in to view

Copilot AI reviewed Mar 3, 2026

View reviewed changes

utahta reviewed Mar 3, 2026

View reviewed changes

goccy requested a review from Copilot March 3, 2026 04:44

Copilot started reviewing on behalf of goccy March 3, 2026 04:45 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

goccy requested a review from utahta March 3, 2026 05:08

utahta approved these changes Mar 3, 2026

View reviewed changes

atkin-mc requested changes Mar 3, 2026

View reviewed changes

remove counter field

2848df0

goccy requested a review from atkin-mc March 3, 2026 07:01

atkin-mc approved these changes Mar 3, 2026

View reviewed changes

goccy merged commit 351dc4e into main Mar 3, 2026
4 checks passed

goccy deleted the optimize branch March 3, 2026 07:15

-	defer g.Close(ctx)
+	defer func() {
+		if err := g.Close(ctx); err != nil {
+			log.Printf("failed to close generator: %+v", err)
+		}
+	}()

Conversation

goccy commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

utahta Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

atkin-mc Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Mar 3, 2026

Code Metrics Report

Code coverage of files in pull request scope (31.0% → 29.4%)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants