From 5f1ea6a4aeb3ce8131b6d01946b32be5448f190d Mon Sep 17 00:00:00 2001 From: Dylan Trotter Date: Mon, 19 Jun 2017 07:52:47 -0700 Subject: [PATCH] Make __dict__ modifiable 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. --- runtime/builtin_types.go | 12 ++----- runtime/builtin_types_test.go | 2 +- runtime/core_test.go | 14 ++++++++ runtime/frame.go | 2 +- runtime/native.go | 2 +- runtime/native_test.go | 4 +-- runtime/object.go | 55 +++++++++++++++++++++++++------ runtime/object_test.go | 62 ++++++++++++++++++++++++++++++----- runtime/super.go | 2 +- runtime/type.go | 6 ++-- runtime/type_test.go | 8 ++--- 11 files changed, 129 insertions(+), 40 deletions(-) diff --git a/runtime/builtin_types.go b/runtime/builtin_types.go index 6a6f21a3..0cb60bdb 100644 --- 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 index 09924c32..76c4eed8 100644 --- 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 index d41b682b..72c55695 100644 --- 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 index 661aaf7b..d00cf4b0 100644 --- 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 index 2498654a..be59e908 100644 --- 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 index 4d3502dc..3f681b9b 100644 --- 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 index f4ad2073..0d343212 100644 --- 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 index 22b6e3cd..2065f6d6 100644 --- 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 index f8b27509..9509d00f 100644 --- 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 index 5dc2dcd0..b5ac5a15 100644 --- 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 index 1308eb9c..8eeedf9c 100644 --- 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{