Separate interpreter and engine per wazero.cache impl (#1022)

Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>

.github/workflows/commit.yaml

@@ -79,7 +79,7 @@ jobs:
         if: ${{ github.event_name  == 'pull_request' }}
 
       # Run tests with -race only on main branch push.
-      - run: make test go_test_options='-race'
+      - run: make test go_test_options='-timeout 10m -race'
         if: ${{ github.event_name  == 'push' }}
 
       - name: "Generate coverage report"  # only once (not per OS)

Makefile

@@ -23,7 +23,7 @@ main_sources  := $(wildcard $(filter-out %_test.go $(all_testdata) $(all_testing
 main_packages := $(sort $(foreach f,$(dir $(main_sources)),$(if $(findstring ./,$(f)),./,./$(f))))
 
 # By default, we don't run with -race as it's costly to run on every PR.
-go_test_options ?=
+go_test_options ?= -timeout 120s
 
 ensureCompilerFastest := -ldflags '-X github.com/tetratelabs/wazero/internal/integration_test/vs.ensureCompilerFastest=true'
 .PHONY: bench
@@ -177,8 +177,8 @@ build.spectest.v2: # Note: SIMD cases are placed in the "simd" subdirectory.
 
 .PHONY: test
 test:
-	@go test $(go_test_options) $$(go list ./... | grep -vE '$(spectest_v1_dir)|$(spectest_v2_dir)') -timeout 120s
+	@go test $(go_test_options) $$(go list ./... | grep -vE '$(spectest_v1_dir)|$(spectest_v2_dir)')
-	@cd internal/version/testdata && go test $(go_test_options) ./... -timeout 120s
+	@cd internal/version/testdata && go test $(go_test_options) ./...
 
 .PHONY: coverage
 coverpkg = $(subst $(space),$(comma),$(main_packages))
@@ -192,10 +192,10 @@ spectest:
 	@$(MAKE) spectest.v2
 
 spectest.v1:
-	@go test $(go_test_options) $$(go list ./... | grep $(spectest_v1_dir)) -timeout 120s
+	@go test $(go_test_options) $$(go list ./... | grep $(spectest_v1_dir))
 
 spectest.v2:
-	@go test $(go_test_options) $$(go list ./... | grep $(spectest_v2_dir)) -timeout 120s
+	@go test $(go_test_options) $$(go list ./... | grep $(spectest_v2_dir))
 
 golangci_lint_path := $(shell go env GOPATH)/bin/golangci-lint
 

cache.go

@@ -51,20 +51,24 @@ func NewCompilationCacheWithDir(dirname string) (CompilationCache, error) {
 type cache struct {
 	// eng is the engine for this cache. If the cache is configured, the engine is shared across multiple instances of
 	// Runtime, and its lifetime is not bound to them. Instead, the engine is alive until Cache.Close is called.
-	eng       wasm.Engine
+	engs      [engineKindCount]wasm.Engine
 	fileCache filecache.Cache
-	once      sync.Once
+	initOnces [engineKindCount]sync.Once
 }
 
-func (c *cache) initEngine(ne newEngine, ctx context.Context, features api.CoreFeatures) wasm.Engine {
+func (c *cache) initEngine(ek engineKind, ne newEngine, ctx context.Context, features api.CoreFeatures) wasm.Engine {
-	c.once.Do(func() { c.eng = ne(ctx, features, c.fileCache) })
+	c.initOnces[ek].Do(func() { c.engs[ek] = ne(ctx, features, c.fileCache) })
-	return c.eng
+	return c.engs[ek]
 }
 
 // Close implements the same method on the Cache interface.
 func (c *cache) Close(_ context.Context) (err error) {
-	if c.eng != nil {
+	for _, eng := range c.engs {
-		err = c.eng.Close()
+		if eng != nil {
+			if err = eng.Close(); err != nil {
+				return
+			}
+		}
 	}
 	return
 }

cache_test.go

@@ -9,7 +9,9 @@ import (
 	goruntime "runtime"
 	"testing"
 
+	"github.com/tetratelabs/wazero/internal/platform"
 	"github.com/tetratelabs/wazero/internal/testing/require"
+	"github.com/tetratelabs/wazero/internal/wasm"
 )
 
 //go:embed internal/integration_test/vs/testdata/fac.wasm
@@ -22,11 +24,17 @@ func TestCompilationCache(t *testing.T) {
 		foo, bar := getCacheSharedRuntimes(ctx, t)
 		cacheInst := foo.cache
 
+		// add interpreter first, to ensure compiler support isn't order dependent
+		eng := foo.cache.engs[engineKindInterpreter]
+		if platform.CompilerSupported() {
+			eng = foo.cache.engs[engineKindCompiler]
+		}
+
 		// Try compiling.
 		compiled, err := foo.CompileModule(ctx, facWasm)
 		require.NoError(t, err)
 		// Also check it is actually cached.
-		require.Equal(t, uint32(1), cacheInst.eng.CompiledModuleCount())
+		require.Equal(t, uint32(1), eng.CompiledModuleCount())
 		barCompiled, err := bar.CompileModule(ctx, facWasm)
 		require.NoError(t, err)
 
@@ -47,12 +55,12 @@ func TestCompilationCache(t *testing.T) {
 		require.NoError(t, err)
 		err = bar.Close(ctx)
 		require.NoError(t, err)
-		require.Equal(t, uint32(1), cacheInst.eng.CompiledModuleCount())
+		require.Equal(t, uint32(1), eng.CompiledModuleCount())
 
 		// Close the cache, and ensure the engine is closed.
 		err = cacheInst.Close(ctx)
 		require.NoError(t, err)
-		require.Equal(t, uint32(0), cacheInst.eng.CompiledModuleCount())
+		require.Equal(t, uint32(0), eng.CompiledModuleCount())
 	})
 
 	// Even when cache is configured, compiled host modules must be different as that's the way
@@ -162,3 +170,26 @@ func requireChdirToTemp(t *testing.T) (string, string) {
 	require.NoError(t, os.Chdir(tmpDir))
 	return tmpDir, oldwd
 }
+
+func TestCache_Close(t *testing.T) {
+	t.Run("all engines", func(t *testing.T) {
+		c := &cache{engs: [engineKindCount]wasm.Engine{&mockEngine{}, &mockEngine{}}}
+		err := c.Close(testCtx)
+		require.NoError(t, err)
+		for i := engineKind(0); i < engineKindCount; i++ {
+			require.True(t, c.engs[i].(*mockEngine).closed)
+		}
+	})
+	t.Run("only interp", func(t *testing.T) {
+		c := &cache{engs: [engineKindCount]wasm.Engine{nil, &mockEngine{}}}
+		err := c.Close(testCtx)
+		require.NoError(t, err)
+		require.True(t, c.engs[engineKindInterpreter].(*mockEngine).closed)
+	})
+	t.Run("only compiler", func(t *testing.T) {
+		c := &cache{engs: [engineKindCount]wasm.Engine{&mockEngine{}, nil}}
+		err := c.Close(testCtx)
+		require.NoError(t, err)
+		require.True(t, c.engs[engineKindCompiler].(*mockEngine).closed)
+	})
+}

config.go

@@ -131,7 +131,7 @@ type runtimeConfig struct {
 	enabledFeatures       api.CoreFeatures
 	memoryLimitPages      uint32
 	memoryCapacityFromMax bool
-	isInterpreter         bool
+	engineKind            engineKind
 	dwarfDisabled         bool // negative as defaults to enabled
 	newEngine             newEngine
 	cache                 CompilationCache
@@ -145,6 +145,14 @@ var engineLessConfig = &runtimeConfig{
 	dwarfDisabled:         false,
 }
 
+type engineKind int
+
+const (
+	engineKindCompiler engineKind = iota
+	engineKindInterpreter
+	engineKindCount
+)
+
 // NewRuntimeConfigCompiler compiles WebAssembly modules into
 // runtime.GOARCH-specific assembly for optimal performance.
 //
@@ -161,6 +169,7 @@ var engineLessConfig = &runtimeConfig{
 // NewRuntimeConfigInterpreter if needed.
 func NewRuntimeConfigCompiler() RuntimeConfig {
 	ret := engineLessConfig.clone()
+	ret.engineKind = engineKindCompiler
 	ret.newEngine = compiler.NewEngine
 	return ret
 }
@@ -168,7 +177,7 @@ func NewRuntimeConfigCompiler() RuntimeConfig {
 // NewRuntimeConfigInterpreter interprets WebAssembly modules instead of compiling them into assembly.
 func NewRuntimeConfigInterpreter() RuntimeConfig {
 	ret := engineLessConfig.clone()
-	ret.isInterpreter = true
+	ret.engineKind = engineKindInterpreter
 	ret.newEngine = interpreter.NewEngine
 	return ret
 }

config_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/tetratelabs/wazero/api"
 	"github.com/tetratelabs/wazero/internal/fstest"
+	"github.com/tetratelabs/wazero/internal/platform"
 	internalsys "github.com/tetratelabs/wazero/internal/sys"
 	testfs "github.com/tetratelabs/wazero/internal/testing/fs"
 	"github.com/tetratelabs/wazero/internal/testing/require"
@@ -620,6 +621,19 @@ func Test_compiledModule_Close(t *testing.T) {
 	}
 }
 
+func TestNewRuntimeConfig(t *testing.T) {
+	c, ok := NewRuntimeConfig().(*runtimeConfig)
+	require.True(t, ok)
+	// Should be cloned from the source.
+	require.NotEqual(t, engineLessConfig, c)
+	// Ensures if the correct engine is selected.
+	if platform.CompilerSupported() {
+		require.Equal(t, engineKindCompiler, c.engineKind)
+	} else {
+		require.Equal(t, engineKindInterpreter, c.engineKind)
+	}
+}
+
 // requireSysContext ensures wasm.NewContext doesn't return an error, which makes it usable in test matrices.
 func requireSysContext(
 	t *testing.T,

runtime.go

@@ -116,7 +116,7 @@ func NewRuntimeWithConfig(ctx context.Context, rConfig RuntimeConfig) Runtime {
 	if c := config.cache; c != nil {
 		// If the Cache is configured, we share the engine.
 		cacheImpl = c.(*cache)
-		engine = cacheImpl.initEngine(config.newEngine, ctx, config.enabledFeatures)
+		engine = cacheImpl.initEngine(config.engineKind, config.newEngine, ctx, config.enabledFeatures)
 	} else {
 		// Otherwise, we create a new engine.
 		engine = config.newEngine(ctx, config.enabledFeatures, nil)
@@ -128,7 +128,6 @@ func NewRuntimeWithConfig(ctx context.Context, rConfig RuntimeConfig) Runtime {
 		enabledFeatures:       config.enabledFeatures,
 		memoryLimitPages:      config.memoryLimitPages,
 		memoryCapacityFromMax: config.memoryCapacityFromMax,
-		isInterpreter:         config.isInterpreter,
 		dwarfDisabled:         config.dwarfDisabled,
 	}
 }
@@ -140,7 +139,6 @@ type runtime struct {
 	enabledFeatures       api.CoreFeatures
 	memoryLimitPages      uint32
 	memoryCapacityFromMax bool
-	isInterpreter         bool
 	dwarfDisabled         bool
 }
 

runtime_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/tetratelabs/wazero/experimental"
 	"github.com/tetratelabs/wazero/internal/filecache"
 	"github.com/tetratelabs/wazero/internal/leb128"
+	"github.com/tetratelabs/wazero/internal/platform"
 	"github.com/tetratelabs/wazero/internal/testing/require"
 	"github.com/tetratelabs/wazero/internal/version"
 	"github.com/tetratelabs/wazero/internal/wasm"
@@ -702,12 +703,20 @@ func (e *mockEngine) Close() (err error) {
 func TestNewRuntime_concurrent(t *testing.T) {
 	const num = 100
 	var wg sync.WaitGroup
-	config := NewRuntimeConfig().WithCompilationCache(NewCompilationCache())
+	c := NewCompilationCache()
+	// If available, uses two engine configurations for the single compilation cache.
+	configs := [2]RuntimeConfig{NewRuntimeConfigInterpreter().WithCompilationCache(c)}
+	if platform.CompilerSupported() {
+		configs[1] = NewRuntimeConfigCompiler().WithCompilationCache(c)
+	} else {
+		configs[1] = NewRuntimeConfigInterpreter().WithCompilationCache(c)
+	}
 	wg.Add(num)
 	for i := 0; i < num; i++ {
+		i := i
 		go func() {
 			defer wg.Done()
-			r := NewRuntimeWithConfig(testCtx, config)
+			r := NewRuntimeWithConfig(testCtx, configs[i%2])
 			err := r.Close(testCtx)
 			require.NoError(t, err)
 		}()