proto: make safe for App Engine
Factor unsafe code into its own file that can be replaced with
reflect code when building for App Engine.
benchmark old MB/s new MB/s speedup
BenchmarkMarshal 126.34 120.38 0.95x
BenchmarkUnmarshal 111.84 110.78 0.99x
BenchmarkMarshalBytes 893.70 853.55 0.96x
BenchmarkUnmarshalBytes 615.23 619.73 1.01x
Comparing the two new variants, old = unsafe, new = reflect:
benchmark old MB/s new MB/s speedup
BenchmarkMarshal 120.38 50.52 0.42x
BenchmarkUnmarshal 110.78 20.12 0.18x
BenchmarkMarshalBytes 853.55 182.21 0.21x
BenchmarkUnmarshalBytes 619.73 227.66 0.37x
But really these numbers are unfair since they ignore the
qualitative difference: proto was completely unavailable
on App Engine, and now it can be run there.
R=dsymonds, r
CC=golang-dev
http://codereview.appspot.com/6494085
diff --git a/proto/extensions.go b/proto/extensions.go
index 61cee23..56e04ce 100644
--- a/proto/extensions.go
+++ b/proto/extensions.go
@@ -57,6 +57,8 @@
ExtensionMap() map[int32]Extension
}
+var extendableProtoType = reflect.TypeOf((*extendableProto)(nil)).Elem()
+
// ExtensionDesc represents an extension specification.
// Used in generated code from the protocol compiler.
type ExtensionDesc struct {
@@ -129,14 +131,14 @@
et := reflect.TypeOf(e.desc.ExtensionType)
props := new(Properties)
- props.Init(et, "unknown_name", e.desc.Tag, 0)
+ props.Init(et, "unknown_name", e.desc.Tag, nil)
p := NewBuffer(nil)
- // The encoder must be passed a pointer to e.value.
- // Allocate a copy of value so that we can use its address.
+ // If e.value has type T, the encoder expects a *struct{ X T }.
+ // Pass a *T with a zero field and hope it all works out.
x := reflect.New(et)
x.Elem().Set(reflect.ValueOf(e.value))
- if err := props.enc(p, props, x.Pointer()); err != nil {
+ if err := props.enc(p, props, toStructPointer(x)); err != nil {
return err
}
e.enc = p.buf
@@ -203,12 +205,14 @@
rep := extension.repeated()
props := &Properties{}
- props.Init(t, "irrelevant_name", extension.Tag, 0)
+ props.Init(t, "irrelevant_name", extension.Tag, nil)
// t is a pointer to a struct, pointer to basic type or a slice.
// Allocate a "field" to store the pointer/slice itself; the
// pointer/slice will be stored here. We pass
// the address of this field to props.dec.
+ // This passes a zero field and a *t and lets props.dec
+ // interpret it as a *struct{ x t }.
value := reflect.New(t).Elem()
for {
@@ -217,7 +221,7 @@
return nil, err
}
- if err := props.dec(o, props, value.UnsafeAddr()); err != nil {
+ if err := props.dec(o, props, toStructPointer(value.Addr())); err != nil {
return nil, err
}