Make __dict__ modifiable (#331)

trotterdylan · web-flow · commit f1446cd91c75 · 2017-06-19T12:53:31.000-07:00
This requires serializing access to Object.dict via atomic operations.
From some quick benchmarking, this does not seem to make a significant
difference to attribute access times.
diff --git a/runtime/builtin_types.go b/runtime/builtin_types.go
@@ -342,19 +342,13 @@ func builtinDir(f *Frame, args Args, kwargs KWArgs) (*Object, *BaseException) {
 	}
 	d := NewDict()
 	o := args[0]
-	if o.dict != nil {
-		raised := seqForEach(f, o.dict.ToObject(), func(k *Object) *BaseException {
-			return d.SetItem(f, k, None)
-		})
-		if raised != nil {
+	if dict := o.Dict(); dict != nil {
+		if raised := d.Update(f, dict.ToObject()); raised != nil {
 			return nil, raised
 		}
 	}
 	for _, t := range o.typ.mro {
-		raised := seqForEach(f, t.dict.ToObject(), func(k *Object) *BaseException {
-			return d.SetItem(f, k, None)
-		})
-		if raised != nil {
+		if raised := d.Update(f, t.Dict().ToObject()); raised != nil {
 			return nil, raised
 		}
 	}
diff --git a/runtime/builtin_types_test.go b/runtime/builtin_types_test.go
@@ -53,7 +53,7 @@ func TestBuiltinDelAttr(t *testing.T) {
 
 func TestBuiltinFuncs(t *testing.T) {
 	f := NewRootFrame()
-	objectDir := ObjectType.dict.Keys(f)
+	objectDir := ObjectType.Dict().Keys(f)
 	objectDir.Sort(f)
 	fooType := newTestClass("Foo", []*Type{ObjectType}, newStringDict(map[string]*Object{"bar": None}))
 	fooTypeDir := NewList(objectDir.elems...)
diff --git a/runtime/core_test.go b/runtime/core_test.go
@@ -1164,6 +1164,20 @@ func TestToNative(t *testing.T) {
 	}
 }
 
+func BenchmarkGetAttr(b *testing.B) {
+	f := NewRootFrame()
+	attr := NewStr("bar")
+	fooType := newTestClass("Foo", []*Type{ObjectType}, NewDict())
+	foo := newObject(fooType)
+	if raised := SetAttr(f, foo, attr, NewInt(123).ToObject()); raised != nil {
+		panic(raised)
+	}
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		mustNotRaise(GetAttr(f, foo, attr, nil))
+	}
+}
+
 // SetAttr is tested in TestObjectSetAttr.
 
 func exceptionsAreEquivalent(e1 *BaseException, e2 *BaseException) bool {
diff --git a/runtime/frame.go b/runtime/frame.go
@@ -70,7 +70,7 @@ func (f *Frame) release() {
 		// TODO: Track cache depth and release memory.
 		f.frameCache, f.back = f, f.frameCache
 		// Clear pointers early.
-		f.dict = nil
+		f.setDict(nil)
 		f.globals = nil
 		f.code = nil
 	} else if f.back != nil {
diff --git a/runtime/native.go b/runtime/native.go
@@ -434,7 +434,7 @@ func getNativeType(rtype reflect.Type) *Type {
 				d[name] = newNativeField(name, i, t)
 			}
 		}
-		t.dict = newStringDict(d)
+		t.setDict(newStringDict(d))
 		// This cannot fail since we're defining simple classes.
 		if err := prepareType(t); err != "" {
 			logFatal(err)
diff --git a/runtime/native_test.go b/runtime/native_test.go
@@ -479,11 +479,11 @@ func TestNewNativeFieldChecksInstanceType(t *testing.T) {
 	}
 
 	// When its field property is assigned to a different type
-	property, raised := native.typ.dict.GetItemString(f, "foo")
+	property, raised := native.typ.Dict().GetItemString(f, "foo")
 	if raised != nil {
 		t.Fatal("Unexpected exception:", raised)
 	}
-	if raised := IntType.dict.SetItemString(f, "foo", property); raised != nil {
+	if raised := IntType.Dict().SetItemString(f, "foo", property); raised != nil {
 		t.Fatal("Unexpected exception:", raised)
 	}
 
diff --git a/runtime/object.go b/runtime/object.go
@@ -17,6 +17,7 @@ package grumpy
 import (
 	"fmt"
 	"reflect"
+	"sync/atomic"
 	"unsafe"
 )
 
@@ -39,7 +40,7 @@ var (
 // Object represents Python 'object' objects.
 type Object struct {
 	typ  *Type `attr:"__class__"`
-	dict *Dict `attr:"__dict__"`
+	dict *Dict
 	ref  *WeakRef
 }
 
@@ -50,7 +51,7 @@ func newObject(t *Type) *Object {
 	}
 	o := (*Object)(unsafe.Pointer(reflect.New(t.basis).Pointer()))
 	o.typ = t
-	o.dict = dict
+	o.setDict(dict)
 	return o
 }
 
@@ -66,7 +67,13 @@ func (o *Object) Call(f *Frame, args Args, kwargs KWArgs) (*Object, *BaseExcepti
 
 // Dict returns o's object dict, aka __dict__.
 func (o *Object) Dict() *Dict {
-	return o.dict
+	p := (*unsafe.Pointer)(unsafe.Pointer(&o.dict))
+	return (*Dict)(atomic.LoadPointer(p))
+}
+
+func (o *Object) setDict(d *Dict) {
+	p := (*unsafe.Pointer)(unsafe.Pointer(&o.dict))
+	atomic.StorePointer(p, unsafe.Pointer(d))
 }
 
 // String returns a string representation of o, e.g. for debugging.
@@ -109,8 +116,9 @@ func objectDelAttr(f *Frame, o *Object, name *Str) *BaseException {
 		}
 	}
 	deleted := false
-	if o.dict != nil {
-		deleted, raised = o.dict.DelItem(f, name.ToObject())
+	d := o.Dict()
+	if d != nil {
+		deleted, raised = d.DelItem(f, name.ToObject())
 		if raised != nil {
 			return raised
 		}
@@ -138,7 +146,7 @@ func objectGetAttribute(f *Frame, o *Object, name *Str) (*Object, *BaseException
 		}
 	}
 	// Look in the object's dict.
-	if d := o.dict; d != nil {
+	if d := o.Dict(); d != nil {
 		value, raised := d.GetItem(f, name.ToObject())
 		if value != nil || raised != nil {
 			return value, raised
@@ -208,8 +216,8 @@ func objectSetAttr(f *Frame, o *Object, name *Str, value *Object) *BaseException
 			return typeSet.Fn(f, typeAttr, o, value)
 		}
 	}
-	if o.dict != nil {
-		if raised := o.dict.SetItem(f, name.ToObject(), value); raised == nil || !raised.isInstance(KeyErrorType) {
+	if d := o.Dict(); d != nil {
+		if raised := d.SetItem(f, name.ToObject(), value); raised == nil || !raised.isInstance(KeyErrorType) {
 			return nil
 		}
 	}
@@ -220,6 +228,7 @@ func initObjectType(dict map[string]*Object) {
 	ObjectType.typ = TypeType
 	dict["__reduce__"] = objectReduceFunc
 	dict["__reduce_ex__"] = newBuiltinFunction("__reduce_ex__", objectReduceEx).ToObject()
+	dict["__dict__"] = newProperty(newBuiltinFunction("_get_dict", objectGetDict).ToObject(), newBuiltinFunction("_set_dict", objectSetDict).ToObject(), nil).ToObject()
 	ObjectType.slots.DelAttr = &delAttrSlot{objectDelAttr}
 	ObjectType.slots.GetAttribute = &getAttributeSlot{objectGetAttribute}
 	ObjectType.slots.Hash = &unaryOpSlot{objectHash}
@@ -306,8 +315,8 @@ func objectReduceCommon(f *Frame, args Args) (*Object, *BaseException) {
 		newArgs = append(newArgs, toTupleUnsafe(extraNewArgs).elems...)
 	}
 	dict := None
-	if o.dict != nil {
-		dict = o.dict.ToObject()
+	if d := o.Dict(); d != nil {
+		dict = d.ToObject()
 	}
 	// For proto >= 2 include list and dict items.
 	listItems := None
@@ -334,3 +343,29 @@ func objectReduceCommon(f *Frame, args Args) (*Object, *BaseException) {
 	}
 	return NewTuple5(newFunc, NewTuple(newArgs...).ToObject(), dict, listItems, dictItems).ToObject(), nil
 }
+
+func objectGetDict(f *Frame, args Args, _ KWArgs) (*Object, *BaseException) {
+	if raised := checkMethodArgs(f, "_get_dict", args, ObjectType); raised != nil {
+		return nil, raised
+	}
+	o := args[0]
+	d := o.Dict()
+	if d == nil {
+		format := "'%s' object has no attribute '__dict__'"
+		return nil, f.RaiseType(AttributeErrorType, fmt.Sprintf(format, o.typ.Name()))
+	}
+	return args[0].Dict().ToObject(), nil
+}
+
+func objectSetDict(f *Frame, args Args, _ KWArgs) (*Object, *BaseException) {
+	if raised := checkMethodArgs(f, "_set_dict", args, ObjectType, DictType); raised != nil {
+		return nil, raised
+	}
+	o := args[0]
+	if o.Type() == ObjectType {
+		format := "'%s' object has no attribute '__dict__'"
+		return nil, f.RaiseType(AttributeErrorType, fmt.Sprintf(format, o.typ.Name()))
+	}
+	o.setDict(toDictUnsafe(args[1]))
+	return None, nil
+}
diff --git a/runtime/object_test.go b/runtime/object_test.go
@@ -105,7 +105,7 @@ func TestObjectDelAttr(t *testing.T) {
 	})
 	dellerType := newTestClass("Deller", []*Type{ObjectType}, newStringDict(map[string]*Object{
 		"__get__": newBuiltinFunction("__get__", func(f *Frame, args Args, kwargs KWArgs) (*Object, *BaseException) {
-			attr, raised := args[1].dict.GetItemString(f, "attr")
+			attr, raised := args[1].Dict().GetItemString(f, "attr")
 			if raised != nil {
 				return nil, raised
 			}
@@ -115,7 +115,7 @@ func TestObjectDelAttr(t *testing.T) {
 			return attr, nil
 		}).ToObject(),
 		"__delete__": newBuiltinFunction("__delete__", func(f *Frame, args Args, kwargs KWArgs) (*Object, *BaseException) {
-			deleted, raised := args[1].dict.DelItemString(f, "attr")
+			deleted, raised := args[1].Dict().DelItemString(f, "attr")
 			if raised != nil {
 				return nil, raised
 			}
@@ -127,7 +127,7 @@ func TestObjectDelAttr(t *testing.T) {
 	}))
 	fooType := newTestClass("Foo", []*Type{ObjectType}, newStringDict(map[string]*Object{"deller": newObject(dellerType)}))
 	foo := newObject(fooType)
-	if raised := foo.dict.SetItemString(NewRootFrame(), "attr", NewInt(123).ToObject()); raised != nil {
+	if raised := foo.Dict().SetItemString(NewRootFrame(), "attr", NewInt(123).ToObject()); raised != nil {
 		t.Fatal(raised)
 	}
 	cases := []invokeTestCase{
@@ -180,13 +180,13 @@ func TestObjectGetAttribute(t *testing.T) {
 		"barsetter": setter,
 	}))
 	foo := newObject(fooType)
-	if raised := foo.dict.SetItemString(NewRootFrame(), "fooattr", True.ToObject()); raised != nil {
+	if raised := foo.Dict().SetItemString(NewRootFrame(), "fooattr", True.ToObject()); raised != nil {
 		t.Fatal(raised)
 	}
-	if raised := foo.dict.SetItemString(NewRootFrame(), "barattr", NewInt(-1).ToObject()); raised != nil {
+	if raised := foo.Dict().SetItemString(NewRootFrame(), "barattr", NewInt(-1).ToObject()); raised != nil {
 		t.Fatal(raised)
 	}
-	if raised := foo.dict.SetItemString(NewRootFrame(), "barsetter", NewStr("NOT setter").ToObject()); raised != nil {
+	if raised := foo.Dict().SetItemString(NewRootFrame(), "barsetter", NewStr("NOT setter").ToObject()); raised != nil {
 		t.Fatal(raised)
 	}
 	cases := []invokeTestCase{
@@ -205,6 +205,52 @@ func TestObjectGetAttribute(t *testing.T) {
 	}
 }
 
+func TestObjectGetDict(t *testing.T) {
+	fooType := newTestClass("Foo", []*Type{ObjectType}, NewDict())
+	foo := newObject(fooType)
+	if raised := SetAttr(NewRootFrame(), foo, NewStr("bar"), NewInt(123).ToObject()); raised != nil {
+		panic(raised)
+	}
+	fun := wrapFuncForTest(func(f *Frame, o *Object) (*Object, *BaseException) {
+		return GetAttr(f, o, NewStr("__dict__"), nil)
+	})
+	cases := []invokeTestCase{
+		{args: wrapArgs(newObject(ObjectType)), wantExc: mustCreateException(AttributeErrorType, "'object' object has no attribute '__dict__'")},
+		{args: wrapArgs(newObject(fooType)), want: NewDict().ToObject()},
+		{args: wrapArgs(foo), want: newStringDict(map[string]*Object{"bar": NewInt(123).ToObject()}).ToObject()},
+	}
+	for _, cas := range cases {
+		if err := runInvokeTestCase(fun, &cas); err != "" {
+			t.Error(err)
+		}
+	}
+}
+
+func TestObjectSetDict(t *testing.T) {
+	fooType := newTestClass("Foo", []*Type{ObjectType}, NewDict())
+	testDict := newStringDict(map[string]*Object{"bar": NewInt(123).ToObject()})
+	fun := wrapFuncForTest(func(f *Frame, o, val *Object) (*Object, *BaseException) {
+		if raised := SetAttr(f, o, NewStr("__dict__"), val); raised != nil {
+			return nil, raised
+		}
+		d := o.Dict()
+		if d == nil {
+			return None, nil
+		}
+		return d.ToObject(), nil
+	})
+	cases := []invokeTestCase{
+		{args: wrapArgs(newObject(ObjectType), NewDict()), wantExc: mustCreateException(AttributeErrorType, "'object' object has no attribute '__dict__'")},
+		{args: wrapArgs(newObject(fooType), testDict), want: testDict.ToObject()},
+		{args: wrapArgs(newObject(fooType), 123), wantExc: mustCreateException(TypeErrorType, "'_set_dict' requires a 'dict' object but received a 'int'")},
+	}
+	for _, cas := range cases {
+		if err := runInvokeTestCase(fun, &cas); err != "" {
+			t.Error(err)
+		}
+	}
+}
+
 func TestObjectNew(t *testing.T) {
 	foo := makeTestType("Foo", ObjectType)
 	foo.flags &= ^typeFlagInstantiable
@@ -335,7 +381,7 @@ func TestObjectSetAttr(t *testing.T) {
 	})
 	setterType := newTestClass("Setter", []*Type{ObjectType}, newStringDict(map[string]*Object{
 		"__get__": newBuiltinFunction("__get__", func(f *Frame, args Args, kwargs KWArgs) (*Object, *BaseException) {
-			item, raised := args[1].dict.GetItemString(f, "attr")
+			item, raised := args[1].Dict().GetItemString(f, "attr")
 			if raised != nil {
 				return nil, raised
 			}
@@ -345,7 +391,7 @@ func TestObjectSetAttr(t *testing.T) {
 			return item, nil
 		}).ToObject(),
 		"__set__": newBuiltinFunction("__set__", func(f *Frame, args Args, kwargs KWArgs) (*Object, *BaseException) {
-			if raised := args[1].dict.SetItemString(f, "attr", NewTuple(args.makeCopy()...).ToObject()); raised != nil {
+			if raised := args[1].Dict().SetItemString(f, "attr", NewTuple(args.makeCopy()...).ToObject()); raised != nil {
 				return nil, raised
 			}
 			return None, nil
diff --git a/runtime/super.go b/runtime/super.go
@@ -74,7 +74,7 @@ func superGetAttribute(f *Frame, o *Object, name *Str) (*Object, *BaseException)
 		}
 		// Now do normal mro lookup from the successor type.
 		for ; i < n; i++ {
-			dict := mro[i].dict
+			dict := mro[i].Dict()
 			res, raised := dict.GetItem(f, name.ToObject())
 			if raised != nil {
 				return nil, raised
diff --git a/runtime/type.go b/runtime/type.go
@@ -172,7 +172,7 @@ func prepareBuiltinType(typ *Type, init builtinTypeInit) {
 			}
 		}
 	}
-	typ.dict = newStringDict(dict)
+	typ.setDict(newStringDict(dict))
 	if err := prepareType(typ); err != "" {
 		logFatal(err)
 	}
@@ -287,7 +287,7 @@ func (t *Type) Name() string {
 
 // FullName returns t's fully qualified name including the module.
 func (t *Type) FullName(f *Frame) (string, *BaseException) {
-	moduleAttr, raised := t.dict.GetItemString(f, "__module__")
+	moduleAttr, raised := t.Dict().GetItemString(f, "__module__")
 	if raised != nil {
 		return "", raised
 	}
@@ -313,7 +313,7 @@ func (t *Type) isSubclass(super *Type) bool {
 
 func (t *Type) mroLookup(f *Frame, name *Str) (*Object, *BaseException) {
 	for _, t := range t.mro {
-		v, raised := t.dict.GetItem(f, name.ToObject())
+		v, raised := t.Dict().GetItem(f, name.ToObject())
 		if v != nil || raised != nil {
 			return v, raised
 		}
diff --git a/runtime/type_test.go b/runtime/type_test.go
@@ -26,7 +26,7 @@ func TestNewClass(t *testing.T) {
 	strBasisStructFunc := func(o *Object) *strBasisStruct { return (*strBasisStruct)(o.toPointer()) }
 	fooType := newBasisType("Foo", reflect.TypeOf(strBasisStruct{}), strBasisStructFunc, StrType)
 	defer delete(basisTypes, fooType.basis)
-	fooType.dict = NewDict()
+	fooType.setDict(NewDict())
 	prepareType(fooType)
 	cases := []struct {
 		wantBasis reflect.Type
@@ -66,7 +66,7 @@ func TestNewBasisType(t *testing.T) {
 	if typ.Type() != TypeType {
 		t.Errorf("got %q, want a type", typ.Type().Name())
 	}
-	if typ.dict != nil {
+	if typ.Dict() != nil {
 		t.Error("type's dict was expected to be nil")
 	}
 	wantBases := []*Type{ObjectType}
@@ -151,7 +151,7 @@ func TestPrepareType(t *testing.T) {
 	for _, cas := range cases {
 		typ := newBasisType("Foo", cas.basis, cas.basisFunc, cas.base)
 		defer delete(basisTypes, cas.basis)
-		typ.dict = NewDict()
+		typ.setDict(NewDict())
 		prepareType(typ)
 		cas.wantMro[0] = typ
 		if !reflect.DeepEqual(typ.mro, cas.wantMro) {
@@ -334,7 +334,7 @@ func TestTypeGetAttribute(t *testing.T) {
 	//   __metaclass__ = BarMeta
 	// bar = Bar()
 	barType := &Type{Object: Object{typ: barMetaType}, name: "Bar", basis: fooType.basis, bases: []*Type{fooType}}
-	barType.dict = newTestDict("bar", "Bar's bar", "foo", 101, "barsetter", setter, "barmetasetter", "NOT setter")
+	barType.setDict(newTestDict("bar", "Bar's bar", "foo", 101, "barsetter", setter, "barmetasetter", "NOT setter"))
 	bar := newObject(barType)
 	prepareType(barType)
 	cases := []invokeTestCase{

Original file line number	Diff line number	Diff line change
`@@ -342,19 +342,13 @@ func builtinDir(f Frame, args Args, kwargs KWArgs) (Object, *BaseException) {`
`342`	`342`	`}`
`343`	`343`	`d := NewDict()`
`344`	`344`	`o := args[0]`
`345`		`- if o.dict != nil {`
`346`		`- raised := seqForEach(f, o.dict.ToObject(), func(k Object) BaseException {`
`347`		`- return d.SetItem(f, k, None)`
`348`		`- })`
`349`		`- if raised != nil {`
	`345`	`+ if dict := o.Dict(); dict != nil {`
	`346`	`+ if raised := d.Update(f, dict.ToObject()); raised != nil {`
`350`	`347`	`return nil, raised`
`351`	`348`	`}`
`352`	`349`	`}`
`353`	`350`	`for _, t := range o.typ.mro {`
`354`		`- raised := seqForEach(f, t.dict.ToObject(), func(k Object) BaseException {`
`355`		`- return d.SetItem(f, k, None)`
`356`		`- })`
`357`		`- if raised != nil {`
	`351`	`+ if raised := d.Update(f, t.Dict().ToObject()); raised != nil {`
`358`	`352`	`return nil, raised`
`359`	`353`	`}`
`360`	`354`	`}`
Original file line number	Diff line number	Diff line change
`@@ -434,7 +434,7 @@ func getNativeType(rtype reflect.Type) *Type {`
`434`	`434`	`d[name] = newNativeField(name, i, t)`
`435`	`435`	`}`
`436`	`436`	`}`
`437`		`- t.dict = newStringDict(d)`
	`437`	`+ t.setDict(newStringDict(d))`
`438`	`438`	`// This cannot fail since we're defining simple classes.`
`439`	`439`	`if err := prepareType(t); err != "" {`
`440`	`440`	`logFatal(err)`
Original file line number	Diff line number	Diff line change
`@@ -479,11 +479,11 @@ func TestNewNativeFieldChecksInstanceType(t *testing.T) {`
`479`	`479`	`}`
`480`	`480`
`481`	`481`	`// When its field property is assigned to a different type`
`482`		`- property, raised := native.typ.dict.GetItemString(f, "foo")`
	`482`	`+ property, raised := native.typ.Dict().GetItemString(f, "foo")`
`483`	`483`	`if raised != nil {`
`484`	`484`	`t.Fatal("Unexpected exception:", raised)`
`485`	`485`	`}`
`486`		`- if raised := IntType.dict.SetItemString(f, "foo", property); raised != nil {`
	`486`	`+ if raised := IntType.Dict().SetItemString(f, "foo", property); raised != nil {`
`487`	`487`	`t.Fatal("Unexpected exception:", raised)`
`488`	`488`	`}`
`489`	`489`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ func superGetAttribute(f Frame, o Object, name Str) (Object, *BaseException)`
`74`	`74`	`}`
`75`	`75`	`// Now do normal mro lookup from the successor type.`
`76`	`76`	`for ; i < n; i++ {`
`77`		`- dict := mro[i].dict`
	`77`	`+ dict := mro[i].Dict()`
`78`	`78`	`res, raised := dict.GetItem(f, name.ToObject())`
`79`	`79`	`if raised != nil {`
`80`	`80`	`return nil, raised`
Original file line number	Diff line number	Diff line change
`@@ -172,7 +172,7 @@ func prepareBuiltinType(typ *Type, init builtinTypeInit) {`
`172`	`172`	`}`
`173`	`173`	`}`
`174`	`174`	`}`
`175`		`- typ.dict = newStringDict(dict)`
	`175`	`+ typ.setDict(newStringDict(dict))`
`176`	`176`	`if err := prepareType(typ); err != "" {`
`177`	`177`	`logFatal(err)`
`178`	`178`	`}`
`@@ -287,7 +287,7 @@ func (t *Type) Name() string {`
`287`	`287`
`288`	`288`	`// FullName returns t's fully qualified name including the module.`
`289`	`289`	`func (t Type) FullName(f Frame) (string, *BaseException) {`
`290`		`- moduleAttr, raised := t.dict.GetItemString(f, "__module__")`
	`290`	`+ moduleAttr, raised := t.Dict().GetItemString(f, "__module__")`
`291`	`291`	`if raised != nil {`
`292`	`292`	`return "", raised`
`293`	`293`	`}`
`@@ -313,7 +313,7 @@ func (t Type) isSubclass(super Type) bool {`
`313`	`313`
`314`	`314`	`func (t Type) mroLookup(f Frame, name Str) (Object, *BaseException) {`
`315`	`315`	`for _, t := range t.mro {`
`316`		`- v, raised := t.dict.GetItem(f, name.ToObject())`
	`316`	`+ v, raised := t.Dict().GetItem(f, name.ToObject())`
`317`	`317`	`if v != nil \|\| raised != nil {`
`318`	`318`	`return v, raised`
`319`	`319`	`}`