Repository: tetratelabs/wazero
Pull request #1270 (2023-03-22 08:49:35)

Removes unnecessary .Index field of FunctionInstance


Patch:
diff --git a/internal/engine/compiler/engine.go b/internal/engine/compiler/engine.go
index 51b1583a..7bf7dffd 100644
--- a/internal/engine/compiler/engine.go
+++ b/internal/engine/compiler/engine.go
@@ -558,7 +558,7 @@ func (e *engine) NewModuleEngine(name string, module *wasm.Module, functions []w
 
 	imported := int(module.ImportFuncCount())
 	for i, f := range functions[:imported] {
-		cf := f.Module.Engine.(*moduleEngine).functions[f.Idx]
+		cf := f.Module.Engine.(*moduleEngine).functions[f.Definition.Index()]
 		me.functions[i] = cf
 	}
 
@@ -595,7 +595,7 @@ func (e *moduleEngine) FunctionInstanceReference(funcIndex wasm.Index) wasm.Refe
 func (e *moduleEngine) NewCallEngine(_ *wasm.CallContext, f *wasm.FunctionInstance) (ce wasm.CallEngine, err error) {
 	// Note: The input parameters are pre-validated, so a compiled function is only absent on close. Updates to
 	// code on close aren't locked, neither is this read.
-	compiled := &e.functions[f.Idx]
+	compiled := &e.functions[f.Definition.Index()]
 
 	initStackSize := initialStackSize
 	if initialStackSize < compiled.parent.stackPointerCeil {
@@ -621,7 +621,7 @@ func (e *moduleEngine) LookupFunction(t *wasm.TableInstance, typeId wasm.Functio
 		err = wasmruntime.ErrRuntimeIndirectCallTypeMismatch
 		return
 	}
-	idx = tf.source.Idx
+	idx = tf.source.Definition.Index()
 
 	return
 }
diff --git a/internal/engine/interpreter/interpreter.go b/internal/engine/interpreter/interpreter.go
index 64a91b51..6804def0 100644
--- a/internal/engine/interpreter/interpreter.go
+++ b/internal/engine/interpreter/interpreter.go
@@ -267,7 +267,7 @@ func (e *engine) NewModuleEngine(name string, module *wasm.Module, functions []w
 
 	imported := int(module.ImportFuncCount())
 	for i, f := range functions[:imported] {
-		cf := f.Module.Engine.(*moduleEngine).functions[f.Idx]
+		cf := f.Module.Engine.(*moduleEngine).functions[f.Definition.Index()]
 		me.functions[i] = cf
 	}
 
@@ -743,7 +743,7 @@ func (e *moduleEngine) FunctionInstanceReference(funcIndex wasm.Index) wasm.Refe
 func (e *moduleEngine) NewCallEngine(_ *wasm.CallContext, f *wasm.FunctionInstance) (ce wasm.CallEngine, err error) {
 	// Note: The input parameters are pre-validated, so a compiled function is only absent on close. Updates to
 	// code on close aren't locked, neither is this read.
-	compiled := &e.functions[f.Idx]
+	compiled := &e.functions[f.Definition.Index()]
 	return e.newCallEngine(f, compiled), nil
 }
 
@@ -764,7 +764,7 @@ func (e *moduleEngine) LookupFunction(t *wasm.TableInstance, typeId wasm.Functio
 		err = wasmruntime.ErrRuntimeIndirectCallTypeMismatch
 		return
 	}
-	idx = tf.source.Idx
+	idx = tf.source.Definition.Index()
 
 	return
 }
diff --git a/internal/wasm/call_context.go b/internal/wasm/call_context.go
index 5a9c74f8..2db75788 100644
--- a/internal/wasm/call_context.go
+++ b/internal/wasm/call_context.go
@@ -15,8 +15,7 @@ import (
 var _ api.Module = &CallContext{}
 
 func NewCallContext(s *Store, instance *ModuleInstance, sys *internalsys.Context) *CallContext {
-	zero := uint64(0)
-	return &CallContext{memory: instance.Memory, module: instance, s: s, Sys: sys, Closed: &zero}
+	return &CallContext{memory: instance.Memory, module: instance, s: s, Sys: sys, Closed: 0}
 }
 
 // CallContext is a function call context bound to a module. This is important as one module's functions can call
@@ -53,7 +52,7 @@ type CallContext struct {
 	//
 	// Note: Exclusively reading and updating this with atomics guarantees cross-goroutine observations.
 	// See /RATIONALE.md
-	Closed *uint64
+	Closed uint64
 
 	// CodeCloser is non-nil when the code should be closed after this module.
 	CodeCloser api.Closer
@@ -61,7 +60,7 @@ type CallContext struct {
 
 // FailIfClosed returns a sys.ExitError if CloseWithExitCode was called.
 func (m *CallContext) FailIfClosed() (err error) {
-	if closed := atomic.LoadUint64(m.Closed); closed != 0 {
+	if closed := atomic.LoadUint64(&m.Closed); closed != 0 {
 		return sys.NewExitError(m.module.Name, uint32(closed>>32)) // Unpack the high order bits as the exit code.
 	}
 	return nil
@@ -149,7 +148,7 @@ func (m *CallContext) closeWithExitCode(ctx context.Context, exitCode uint32) (e
 
 func (m *CallContext) setExitCode(exitCode uint32) bool {
 	closed := uint64(1) + uint64(exitCode)<<32 // Store exitCode as high-order bits.
-	return atomic.CompareAndSwapUint64(m.Closed, 0, closed)
+	return atomic.CompareAndSwapUint64(&m.Closed, 0, closed)
 }
 
 // ensureResourcesClosed ensures that resources assigned to CallContext is released.
diff --git a/internal/wasm/module.go b/internal/wasm/module.go
index b5514177..15e93971 100644
--- a/internal/wasm/module.go
+++ b/internal/wasm/module.go
@@ -604,7 +604,6 @@ func (m *ModuleInstance) BuildFunctions(mod *Module, importedFunctions []*Functi
 		fns[offset] = FunctionInstance{
 			TypeID:     m.TypeIDs[section],
 			Module:     m,
-			Idx:        d.index,
 			Type:       d.funcType,
 			Definition: d,
 		}
diff --git a/internal/wasm/store.go b/internal/wasm/store.go
index e7fedb6a..98262587 100644
--- a/internal/wasm/store.go
+++ b/internal/wasm/store.go
@@ -114,9 +114,6 @@ type (
 		// TypeID is assigned by a store for FunctionType.
 		TypeID FunctionTypeID
 
-		// Idx holds the index of this function instance in the function index (beginning with imports).
-		Idx Index
-
 		// Definition is known at compile time.
 		Definition api.FunctionDefinition
 	}


Test patch:
diff --git a/internal/engine/compiler/compiler_controlflow_test.go b/internal/engine/compiler/compiler_controlflow_test.go
index f33308b1..c46bba90 100644
--- a/internal/engine/compiler/compiler_controlflow_test.go
+++ b/internal/engine/compiler/compiler_controlflow_test.go
@@ -840,14 +840,13 @@ func TestCompiler_compileCall(t *testing.T) {
 
 		c, _, err := compiler.compile()
 		require.NoError(t, err)
-		index := wasm.Index(i)
 		me.functions = append(me.functions, function{
 			parent:                &code{codeSegment: c},
 			codeInitialAddress:    uintptr(unsafe.Pointer(&c[0])),
 			moduleInstanceAddress: uintptr(unsafe.Pointer(env.moduleInstance)),
 		})
 		env.module().Functions = append(env.module().Functions,
-			wasm.FunctionInstance{Type: &targetFunctionType, Idx: index})
+			wasm.FunctionInstance{Type: &targetFunctionType})
 	}
 
 	// Now we start building the caller's code.
diff --git a/internal/wasm/call_context_test.go b/internal/wasm/call_context_test.go
index 99619410..4718401e 100644
--- a/internal/wasm/call_context_test.go
+++ b/internal/wasm/call_context_test.go
@@ -93,7 +93,7 @@ func TestCallContext_Close(t *testing.T) {
 				// Closing should not err.
 				require.NoError(t, tc.closer(ctx, m))
 
-				require.Equal(t, tc.expectedClosed, *m.Closed)
+				require.Equal(t, tc.expectedClosed, m.Closed)
 
 				// Verify our intended side-effect
 				require.Nil(t, s.Module(moduleName))
@@ -197,7 +197,7 @@ func TestCallContext_CallDynamic(t *testing.T) {
 				// Closing should not err.
 				require.NoError(t, tc.closer(ctx, m))
 
-				require.Equal(t, tc.expectedClosed, *m.Closed)
+				require.Equal(t, tc.expectedClosed, m.Closed)
 
 				// Verify our intended side-effect
 				require.Nil(t, s.Module(moduleName))
@@ -259,7 +259,7 @@ func TestCallContext_CallDynamic(t *testing.T) {
 func TestCallContext_CloseModuleOnCanceledOrTimeout(t *testing.T) {
 	s := newStore()
 	t.Run("timeout", func(t *testing.T) {
-		cc := &CallContext{Closed: new(uint64), module: &ModuleInstance{Name: "test"}, s: s}
+		cc := &CallContext{Closed: 0, module: &ModuleInstance{Name: "test"}, s: s}
 		const duration = time.Second
 		ctx, cancel := context.WithTimeout(context.Background(), duration)
 		defer cancel()
@@ -272,7 +272,7 @@ func TestCallContext_CloseModuleOnCanceledOrTimeout(t *testing.T) {
 	})
 
 	t.Run("cancel", func(t *testing.T) {
-		cc := &CallContext{Closed: new(uint64), module: &ModuleInstance{Name: "test"}, s: s}
+		cc := &CallContext{Closed: 0, module: &ModuleInstance{Name: "test"}, s: s}
 		ctx, cancel := context.WithCancel(context.Background())
 		done := cc.CloseModuleOnCanceledOrTimeout(context.WithValue(ctx, struct{}{}, 1)) // Wrapping arbitrary context.
 		cancel()
@@ -287,7 +287,7 @@ func TestCallContext_CloseModuleOnCanceledOrTimeout(t *testing.T) {
 	})
 
 	t.Run("timeout over cancel", func(t *testing.T) {
-		cc := &CallContext{Closed: new(uint64), module: &ModuleInstance{Name: "test"}, s: s}
+		cc := &CallContext{Closed: 0, module: &ModuleInstance{Name: "test"}, s: s}
 		const duration = time.Second
 		ctx, cancel := context.WithCancel(context.Background())
 		defer cancel()
@@ -300,7 +300,7 @@ func TestCallContext_CloseModuleOnCanceledOrTimeout(t *testing.T) {
 	})
 
 	t.Run("cancel over timeout", func(t *testing.T) {
-		cc := &CallContext{Closed: new(uint64), module: &ModuleInstance{Name: "test"}, s: s}
+		cc := &CallContext{Closed: 0, module: &ModuleInstance{Name: "test"}, s: s}
 		ctx, cancel := context.WithCancel(context.Background())
 		// Wrap the timeout context by cancel context.
 		var timeoutDone context.CancelFunc
@@ -317,7 +317,7 @@ func TestCallContext_CloseModuleOnCanceledOrTimeout(t *testing.T) {
 	})
 
 	t.Run("cancel works", func(t *testing.T) {
-		cc := &CallContext{Closed: new(uint64), module: &ModuleInstance{Name: "test"}, s: s}
+		cc := &CallContext{Closed: 0, module: &ModuleInstance{Name: "test"}, s: s}
 		cancelChan := make(chan struct{})
 		var wg sync.WaitGroup
 		wg.Add(1)
@@ -332,7 +332,7 @@ func TestCallContext_CloseModuleOnCanceledOrTimeout(t *testing.T) {
 	})
 
 	t.Run("no close on all resources canceled", func(t *testing.T) {
-		cc := &CallContext{Closed: new(uint64), module: &ModuleInstance{Name: "test"}, s: s}
+		cc := &CallContext{Closed: 0, module: &ModuleInstance{Name: "test"}, s: s}
 		cancelChan := make(chan struct{})
 		close(cancelChan)
 		ctx, cancel := context.WithCancel(context.Background())
@@ -349,7 +349,7 @@ func TestCallContext_CloseWithCtxErr(t *testing.T) {
 	s := newStore()
 
 	t.Run("context canceled", func(t *testing.T) {
-		cc := &CallContext{Closed: new(uint64), module: &ModuleInstance{Name: "test"}, s: s}
+		cc := &CallContext{Closed: 0, module: &ModuleInstance{Name: "test"}, s: s}
 		ctx, cancel := context.WithCancel(context.Background())
 		cancel()
 
@@ -360,7 +360,7 @@ func TestCallContext_CloseWithCtxErr(t *testing.T) {
 	})
 
 	t.Run("context timeout", func(t *testing.T) {
-		cc := &CallContext{Closed: new(uint64), module: &ModuleInstance{Name: "test"}, s: s}
+		cc := &CallContext{Closed: 0, module: &ModuleInstance{Name: "test"}, s: s}
 		duration := time.Second
 		ctx, cancel := context.WithTimeout(context.Background(), duration)
 		defer cancel()
@@ -374,7 +374,7 @@ func TestCallContext_CloseWithCtxErr(t *testing.T) {
 	})
 
 	t.Run("no error", func(t *testing.T) {
-		cc := &CallContext{Closed: new(uint64), module: &ModuleInstance{Name: "test"}, s: s}
+		cc := &CallContext{Closed: 0, module: &ModuleInstance{Name: "test"}, s: s}
 
 		cc.CloseWithCtxErr(context.Background())