Skip to content

Commit

Permalink
refactor(go): stop emitting unused fields in go structs (#2612)
Browse files Browse the repository at this point in the history
Go struct fields used to be emitted for all properties of any type,
including for go structs merely there to act as a proxy to a JS object.
Those properties were never accessed (neither set not read), and could
have been cause for confusion. They additionally needlessly increased
the memory footprint of such data types.

This changes the code generation to no longer emit fields on go structs
that are intended as proxies to JS objects, instead embedding the base
type(s) (super-class and/or super-interfaces) to properly inherit their
methods without duplicating every declaration; and uses a `byte`
placeholder when needed to ensure no 0-width struct is ever generated
(go would have those take 0 bytes of memory, and as a consequence, any
such vlaue is deemed equal to any other, which is not what we want).

In cases when a member is inherited from more than one parent, that
member is re-declared on the descendant type, as otherwise the promotion
is ambiguous.

This implied making a change to the method registration functions, such
that each type (class and interface) registers a proxy `maker`
function that is used by the runtime library to properly initialize jsii
proxy instances  of objects without requiring user intervention. This
makes it safe to operate even when embedding data structures from other
libraries. Additionally, it enables hiding the JS proxy structs from the
exported APIs, which further reduces the risk of mis-use.
  • Loading branch information
RomainMuller committed Feb 28, 2021
1 parent 1f8f022 commit adfe8b9
Show file tree
Hide file tree
Showing 17 changed files with 3,799 additions and 3,690 deletions.
28 changes: 14 additions & 14 deletions packages/@jsii/go-runtime/jsii-calc-test/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestMain(m *testing.M) {

// Only uses first argument as initial value. This is just a convenience for
// tests that want to assert against the initialValue
func initCalculator(initialValue float64) calc.CalculatorIface {
func initCalculator(initialValue float64) calc.Calculator {
return calc.NewCalculator(calc.CalculatorProps{
InitialValue: initialValue,
MaximumValue: math.MaxFloat64,
Expand All @@ -43,7 +43,7 @@ func TestCalculator(t *testing.T) {
t.Run("Property access", func(t *testing.T) {
expected := float64(10)
calculator := initCalculator(expected)
actual := calculator.GetValue()
actual := calculator.Value()
if actual != expected {
t.Errorf("Expected: %f; Actual %f;", expected, actual)
}
Expand All @@ -54,7 +54,7 @@ func TestCalculator(t *testing.T) {
var newVal float64 = 12345
currentProps := calclib.NewNumber(newVal)
calculator.SetCurr(currentProps)
actual := calculator.GetValue()
actual := calculator.Value()
if newVal != actual {
t.Errorf("Expected: %f; Actual %f;", newVal, actual)
}
Expand All @@ -65,7 +65,7 @@ func TestCalculator(t *testing.T) {
calculator := initCalculator(initial)
calculator.Mul(factor)
expectedProduct := initial * factor
actualProduct := calculator.GetValue()
actualProduct := calculator.Value()
if actualProduct != expectedProduct {
t.Errorf("Expected quotient: %f; Actual %f;", expectedProduct, actualProduct)
}
Expand Down Expand Up @@ -108,7 +108,7 @@ func TestUpcasingReflectable(t *testing.T) {
key, val := "key1", "value1"
delegate[key] = val
upReflectable := calc.NewUpcasingReflectable(delegate)
entries := upReflectable.GetEntries()
entries := upReflectable.Entries()

if len(entries) != 1 {
t.Errorf("Entries expected to have length of: 1; Actual: %d", len(entries))
Expand All @@ -127,7 +127,7 @@ func TestAllTypes(t *testing.T) {
t.Run("Array property", func(t *testing.T) {
expected1, expected2 := "val1", "val2"
allTypes.SetArrayProperty([]string{expected1, expected2})
actual := allTypes.GetArrayProperty()
actual := allTypes.ArrayProperty()
actual1, actual2 := actual[0], actual[1]

if actual1 != expected1 || actual2 != expected2 {
Expand All @@ -141,7 +141,7 @@ func TestAllTypes(t *testing.T) {
expected[key] = val
allTypes.SetAnyProperty(expected)

actual := allTypes.GetAnyProperty()
actual := allTypes.AnyProperty()
actualVal := reflect.ValueOf(actual)
switch actualVal.Kind() {
case reflect.Map:
Expand Down Expand Up @@ -177,19 +177,19 @@ func TestEnumRoundtrip(t *testing.T) {

func TestOptionalEnums(t *testing.T) {
allTypes := calc.NewAllTypes()
actual := allTypes.GetOptionalEnumValue()
actual := allTypes.OptionalEnumValue()
if actual != "" {
t.Error("Expected value to be nil")
}

allTypes.SetOptionalEnumValue(calc.StringEnum_B)
actual = allTypes.GetOptionalEnumValue()
actual = allTypes.OptionalEnumValue()
if actual != calc.StringEnum_B {
t.Errorf("Expected StringEnum.B. Actual: %s", actual)
}

allTypes.SetOptionalEnumValue("")
actual = allTypes.GetOptionalEnumValue()
actual = allTypes.OptionalEnumValue()
if actual != "" {
t.Error("Expected value to be nil")
}
Expand All @@ -207,21 +207,21 @@ func TestReturnsSpecialParam(t *testing.T) {

func TestMaps(t *testing.T) {
allTypes := calc.NewAllTypes()
actual := allTypes.GetMapProperty()
actual := allTypes.MapProperty()
if len(actual) != 0 {
t.Errorf("Expected length of empty map to be 0. Got: %d", len(actual))
}

question := "The answer to the ultimate question of life, the universe, and everything"
answer := calclib.NewNumber(42)
allTypes.SetMapProperty(map[string]calclib.NumberIface{
allTypes.SetMapProperty(map[string]calclib.Number{
question: answer,
})
actual = allTypes.GetMapProperty()
actual = allTypes.MapProperty()
if len(actual) != 1 {
t.Errorf("Expected length of empty map to be 1. Got: %d", len(actual))
}
if actual[question].GetValue() != answer.GetValue() {
if actual[question].Value() != answer.Value() {
t.Errorf("Expected to have the value %v in there, got: %v", answer, actual[question])
}
}
60 changes: 52 additions & 8 deletions packages/@jsii/go-runtime/jsii-runtime-go/kernel/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/aws/jsii-runtime-go/embedded"
"io"
"io/ioutil"
"os"
"os/exec"
"reflect"
"runtime"
"sync"

"github.com/aws/jsii-runtime-go/embedded"
"github.com/aws/jsii-runtime-go/typeregistry"
)

Expand All @@ -37,7 +38,7 @@ type client struct {
tmpdir string

types typeregistry.TypeRegistry
objects map[interface{}]string
objects map[reflect.Value]string
}

// GetClient returns a singleton client instance, initializing one the first
Expand Down Expand Up @@ -83,11 +84,8 @@ func CloseClient() {
// newClient starts the kernel child process and verifies the "hello" message
// was correct.
func newClient() (*client, error) {
// Initialize map of object instances
objmap := make(map[interface{}]string)

clientinstance := &client{
objects: objmap,
objects: make(map[reflect.Value]string),
types: typeregistry.NewTypeRegistry(),
}

Expand Down Expand Up @@ -200,11 +198,53 @@ func (c *client) Types() typeregistry.TypeRegistry {
return c.types
}

func (c *client) RegisterInstance(instance interface{}, instanceID string) error {
func (c *client) RegisterInstance(instance reflect.Value, instanceID string) error {
instance = reflect.Indirect(instance)
if instance.Kind() == reflect.Interface {
instance = reflect.ValueOf(instance.Interface()).Elem()
}

if existing, found := c.objects[instance]; found && existing != instanceID {
return fmt.Errorf("attempted to register %v as %s, but it was already registered as %s", instance, instanceID, existing)
}

var findAliases func(v reflect.Value) []reflect.Value
findAliases = func(v reflect.Value) (res []reflect.Value) {
v = reflect.Indirect(v)
t := v.Type()
numField := t.NumField()
for i := 0; i < numField; i++ {
f := t.Field(i)
if f.Name == "_" {
// Ignore any padding
continue
}
if !f.Anonymous {
// Ignore any non-anonymous field
continue
}
fv := reflect.Indirect(v.Field(i))
if fv.Kind() == reflect.Interface {
fv = reflect.ValueOf(fv.Interface()).Elem()
}

res = append(res, fv)
res = append(res, findAliases(fv)...)
}
return
}

aliases := findAliases(instance)
for _, alias := range aliases {
if existing, found := c.objects[alias]; found && existing != instanceID {
return fmt.Errorf("value %v is embedded in %s, but was already assigned %s", alias, instanceID, existing)
}
}

c.objects[instance] = instanceID
for _, alias := range aliases {
c.objects[alias] = instanceID
}
return nil
}

Expand All @@ -226,7 +266,11 @@ func (c *client) response(res kernelResponse) error {

}

func (c *client) FindObjectRef(obj interface{}) (refid string, ok bool) {
func (c *client) FindObjectRef(obj reflect.Value) (refid string, ok bool) {
obj = reflect.Indirect(obj)
if obj.Kind() == reflect.Interface {
obj = reflect.ValueOf(obj.Interface()).Elem()
}
refid, ok = c.objects[obj]
return
}
Expand Down
11 changes: 5 additions & 6 deletions packages/@jsii/go-runtime/jsii-runtime-go/kernel/conversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ func (c *client) CastAndSetToPtr(ptr interface{}, data interface{}) {
if fields, isStruct := c.Types().StructFields(ptrVal.Type()); isStruct {
for _, field := range fields {
got, err := c.Get(GetRequest{
API: "get",
Property: field.Tag.Get("json"),
ObjRef: ref,
API: "get",
Property: field.Tag.Get("json"),
ObjRef: ref,
})
if err != nil {
panic(err)
Expand All @@ -33,9 +33,8 @@ func (c *client) CastAndSetToPtr(ptr interface{}, data interface{}) {
}

// If return data is jsii object references, add to objects table.
if concreteType, err := c.Types().ConcreteTypeFor(ptrVal.Type()); err == nil {
ptrVal.Set(reflect.New(concreteType))
if err = c.RegisterInstance(ptrVal.Interface(), ref.InstanceID); err != nil {
if err := c.Types().InitJsiiProxy(ptrVal); err == nil {
if err = c.RegisterInstance(ptrVal, ref.InstanceID); err != nil {
panic(err)
}
} else {
Expand Down
72 changes: 55 additions & 17 deletions packages/@jsii/go-runtime/jsii-runtime-go/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ func Load(name string, version string, tarball []byte) {
}
}

// RegisterClass associates a class fully qualified name to the specified struct
// type, and class interface. Panics if class is not a struct, iface is not an
// interface, or if the provided fqn was already used to register a different
// RegisterClass associates a class fully qualified name to the specified class
// interface, and proxy struct. Panics if class is not an go interface, proxy is
// not a go struct, or if the provided fqn was already used to register a different
// type.
func RegisterClass(fqn FQN, class reflect.Type, iface reflect.Type) {
func RegisterClass(fqn FQN, class reflect.Type, maker func() interface{}) {
client := kernel.GetClient()
if err := client.Types().RegisterClass(api.FQN(fqn), class, iface); err != nil {
if err := client.Types().RegisterClass(api.FQN(fqn), class, maker); err != nil {
panic(err)
}
}
Expand All @@ -70,12 +70,11 @@ func RegisterEnum(fqn FQN, enum reflect.Type, members map[string]interface{}) {
}

// RegisterInterface associates an interface's fully qualified name to the
// specified interface type, and proxy struct. Panics if iface is not an
// interface, proxy is not a struct, or if the provided fqn was already used to
// register a different type.
func RegisterInterface(fqn FQN, iface reflect.Type, proxy reflect.Type) {
// specified interface type, and proxy maker function. Panics if iface is not an
// interface, or if the provided fqn was already used to register a different type.
func RegisterInterface(fqn FQN, iface reflect.Type, maker func() interface{}) {
client := kernel.GetClient()
if err := client.Types().RegisterInterface(api.FQN(fqn), iface, proxy); err != nil {
if err := client.Types().RegisterInterface(api.FQN(fqn), iface, maker); err != nil {
panic(err)
}
}
Expand All @@ -90,11 +89,50 @@ func RegisterStruct(fqn FQN, strct reflect.Type) {
}
}

// InitJsiiProxy initializes a jsii proxy instance at the provided pointer.
// Panics if the pointer cannot be initialized to a proxy instance (i.e: the
// element of it is not a registered jsii interface or class type).
func InitJsiiProxy(ptr interface{}) {
ptrVal := reflect.ValueOf(ptr).Elem()
if err := kernel.GetClient().Types().InitJsiiProxy(ptrVal); err != nil {
panic(err)
}
}

// Create will construct a new JSII object within the kernel runtime. This is
// called by jsii object constructors.
func Create(fqn FQN, args []interface{}, interfaces []FQN, overrides []Override, ret interface{}) {
func Create(fqn FQN, args []interface{}, interfaces []FQN, overrides []Override, inst interface{}) {
client := kernel.GetClient()

instVal := reflect.ValueOf(inst).Elem()
instType := instVal.Type()
numField := instType.NumField()
for i := 0; i < numField; i++ {
field := instType.Field(i)
if !field.Anonymous {
continue
}
switch field.Type.Kind() {
case reflect.Interface:
fieldVal := instVal.Field(i)
if !fieldVal.IsNil() {
continue
}
if err := client.Types().InitJsiiProxy(fieldVal); err != nil {
panic(err)
}

case reflect.Struct:
fieldVal := instVal.Field(i)
if !fieldVal.IsZero() {
continue
}
if err := client.Types().InitJsiiProxy(fieldVal); err != nil {
panic(err)
}
}
}

var interfaceFQNs []api.FQN
for _, iface := range interfaces {
interfaceFQNs = append(interfaceFQNs, api.FQN(iface))
Expand All @@ -116,7 +154,7 @@ func Create(fqn FQN, args []interface{}, interfaces []FQN, overrides []Override,
panic(err)
}

if err = client.RegisterInstance(ret, res.InstanceID); err != nil {
if err = client.RegisterInstance(instVal, res.InstanceID); err != nil {
panic(err)
}
}
Expand All @@ -127,7 +165,7 @@ func Invoke(obj interface{}, method string, args []interface{}, hasReturn bool,
client := kernel.GetClient()

// Find reference to class instance in client
refid, found := client.FindObjectRef(obj)
refid, found := client.FindObjectRef(reflect.ValueOf(obj))

if !found {
panic("No Object Found")
Expand Down Expand Up @@ -178,10 +216,10 @@ func Get(obj interface{}, property string, ret interface{}) {
client := kernel.GetClient()

// Find reference to class instance in client
refid, found := client.FindObjectRef(obj)
refid, found := client.FindObjectRef(reflect.ValueOf(obj))

if !found {
panic("No Object Found")
panic(fmt.Errorf("no object reference found for %v", obj))
}

res, err := client.Get(kernel.GetRequest{
Expand Down Expand Up @@ -223,7 +261,7 @@ func Set(obj interface{}, property string, value interface{}) {
client := kernel.GetClient()

// Find reference to class instance in client
refid, found := client.FindObjectRef(obj)
refid, found := client.FindObjectRef(reflect.ValueOf(obj))

if !found {
panic("No Object Found")
Expand Down Expand Up @@ -286,7 +324,7 @@ func castPtrToRef(data interface{}) interface{} {
return result

case reflect.Ptr:
valref, valHasRef := client.FindObjectRef(data)
valref, valHasRef := client.FindObjectRef(reflect.ValueOf(data))
if valHasRef {
return api.ObjectRef{InstanceID: valref}
}
Expand Down
Loading

0 comments on commit adfe8b9

Please sign in to comment.