Skip to content

Commit

Permalink
encoding/gob: shave off some init time cost
Browse files Browse the repository at this point in the history
Avoid unnecessary allocations when calling reflect.TypeOf;
we can use nil pointers, which fit into an interface without allocating.
This saves about 1% of CPU time.

The builtin types are limited to typeIds between 0 and firstUserId,
and since firstUserId is 64, builtinIdToType does not need to be a map.
We can simply use an array of length firstUserId, which is simpler.
This saves about 1% of CPU time.

idToType is similar to firstUserId in that it is a map keyed by typeIds.
The difference is that it can grow with the user's types.
However, each added type gets the next available typeId,
meaning that we can use a growing slice, similar to the case above.
nextId then becomes the current length of the slice.
This saves about 1% of CPU time.

typeInfoMap is stored globally as an atomic.Value,
where each modification loads the map, makes a whole copy,
adds the new element, and stores the modified copy.
This is perfectly fine when the user registers types,
as that can happen concurrently and at any point in the future.

However, during init time, we sequentially register many types,
and the overhead of copying maps adds up noticeably.
During init time, use a regular global map instead,
which gets replaced by the atomic.Value when our init work is done.
This saves about 2% of CPU time.

Finally, avoid calling checkId in bootstrapType;
we have just called setTypeId, whose logic for getting nextId is simple,
so the extra check doesn't gain us much.
This saves about 1% of CPU time.

Using benchinit, which transforms GODEBUG=inittrace=1 data into Go
benchmark compatible output, results in a nice improvement:

	name         old time/op    new time/op    delta
	EncodingGob     175µs ± 0%     162µs ± 0%  -7.45%  (p=0.016 n=5+4)

	name         old alloc/op   new alloc/op   delta
	EncodingGob    39.0kB ± 0%    36.1kB ± 0%  -7.35%  (p=0.016 n=5+4)

	name         old allocs/op  new allocs/op  delta
	EncodingGob       588 ± 0%       558 ± 0%  -5.10%  (p=0.000 n=5+4)

Updates golang#26775.

Change-Id: I28618e8b96ef440480e666ef2cd5c4a9a332ef21
Reviewed-on: https://go-review.googlesource.com/c/go/+/460543
Reviewed-by: Carlos Amedee <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Reviewed-by: Rob Pike <[email protected]>
Run-TryBot: Rob Pike <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
  • Loading branch information
mvdan committed Jan 20, 2023
1 parent f53137f commit 8259ac4
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 29 deletions.
6 changes: 2 additions & 4 deletions src/encoding/gob/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,7 @@ func (deb *debugger) singletonValue(indent tab, id typeId) {
deb.dump("Singleton value")
// is it a builtin type?
wire := deb.wireType[id]
_, ok := builtinIdToType[id]
if !ok && wire == nil {
if builtinIdToType(id) == nil && wire == nil {
errorf("type id %d not defined", id)
}
m := deb.uint64()
Expand Down Expand Up @@ -573,8 +572,7 @@ func (deb *debugger) printWireType(indent tab, wire *wireType) {
//
// builtinValue | ArrayValue | MapValue | SliceValue | StructValue | InterfaceValue
func (deb *debugger) fieldValue(indent tab, id typeId) {
_, ok := builtinIdToType[id]
if ok {
if builtinIdToType(id) != nil {
if id == tInterface {
deb.interfaceValue(indent)
} else {
Expand Down
8 changes: 4 additions & 4 deletions src/encoding/gob/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,7 @@ func (dec *Decoder) decOpFor(wireId typeId, rt reflect.Type, name string, inProg
break
}
var elemId typeId
if tt, ok := builtinIdToType[wireId]; ok {
if tt := builtinIdToType(wireId); tt != nil {
elemId = tt.(*sliceType).Elem
} else {
elemId = dec.wireType[wireId].SliceT.Elem
Expand Down Expand Up @@ -1068,7 +1068,7 @@ func (dec *Decoder) compatibleType(fr reflect.Type, fw typeId, inProgress map[re
}
// Extract and compare element types.
var sw *sliceType
if tt, ok := builtinIdToType[fw]; ok {
if tt := builtinIdToType(fw); tt != nil {
sw, _ = tt.(*sliceType)
} else if wire != nil {
sw = wire.SliceT
Expand Down Expand Up @@ -1136,7 +1136,7 @@ func (dec *Decoder) compileDec(remoteId typeId, ut *userTypeInfo) (engine *decEn
var wireStruct *structType
// Builtin types can come from global pool; the rest must be defined by the decoder.
// Also we know we're decoding a struct now, so the client must have sent one.
if t, ok := builtinIdToType[remoteId]; ok {
if t := builtinIdToType(remoteId); t != nil {
wireStruct, _ = t.(*structType)
} else {
wire := dec.wireType[remoteId]
Expand Down Expand Up @@ -1199,7 +1199,7 @@ func (dec *Decoder) getDecEnginePtr(remoteId typeId, ut *userTypeInfo) (enginePt
// emptyStruct is the type we compile into when ignoring a struct value.
type emptyStruct struct{}

var emptyStructType = reflect.TypeOf(emptyStruct{})
var emptyStructType = reflect.TypeOf((*emptyStruct)(nil)).Elem()

// getIgnoreEnginePtr returns the engine for the specified type when the value is to be discarded.
func (dec *Decoder) getIgnoreEnginePtr(wireId typeId) (enginePtr **decEngine, err error) {
Expand Down
65 changes: 44 additions & 21 deletions src/encoding/gob/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,6 @@ func userType(rt reflect.Type) *userTypeInfo {
// Internally, typeIds are used as keys to a map to recover the underlying type info.
type typeId int32

var nextId typeId // incremented for each new type we build
var typeLock sync.Mutex // set while building a type
const firstUserId = 64 // lowest id number granted to user

Expand All @@ -172,18 +171,25 @@ type gobType interface {
safeString(seen map[typeId]bool) string
}

var types = make(map[reflect.Type]gobType)
var idToType = make(map[typeId]gobType)
var builtinIdToType map[typeId]gobType // set in init() after builtins are established
var types = make(map[reflect.Type]gobType, 32)
var idToType = make([]gobType, 1, firstUserId)
var builtinIdToTypeSlice [firstUserId]gobType // set in init() after builtins are established

func builtinIdToType(id typeId) gobType {
if id < 0 || int(id) >= len(builtinIdToTypeSlice) {
return nil
}
return builtinIdToTypeSlice[id]
}

func setTypeId(typ gobType) {
// When building recursive types, someone may get there before us.
if typ.id() != 0 {
return
}
nextId++
nextId := typeId(len(idToType))
typ.setId(nextId)
idToType[nextId] = typ
idToType = append(idToType, typ)
}

func (t typeId) gobType() gobType {
Expand Down Expand Up @@ -256,30 +262,29 @@ var (
)

// Predefined because it's needed by the Decoder
var tWireType = mustGetTypeInfo(reflect.TypeOf(wireType{})).id
var tWireType = mustGetTypeInfo(reflect.TypeOf((*wireType)(nil)).Elem()).id
var wireTypeUserInfo *userTypeInfo // userTypeInfo of (*wireType)

func init() {
// Some magic numbers to make sure there are no surprises.
checkId(16, tWireType)
checkId(17, mustGetTypeInfo(reflect.TypeOf(arrayType{})).id)
checkId(18, mustGetTypeInfo(reflect.TypeOf(CommonType{})).id)
checkId(19, mustGetTypeInfo(reflect.TypeOf(sliceType{})).id)
checkId(20, mustGetTypeInfo(reflect.TypeOf(structType{})).id)
checkId(21, mustGetTypeInfo(reflect.TypeOf(fieldType{})).id)
checkId(23, mustGetTypeInfo(reflect.TypeOf(mapType{})).id)

builtinIdToType = make(map[typeId]gobType)
checkId(17, mustGetTypeInfo(reflect.TypeOf((*arrayType)(nil)).Elem()).id)
checkId(18, mustGetTypeInfo(reflect.TypeOf((*CommonType)(nil)).Elem()).id)
checkId(19, mustGetTypeInfo(reflect.TypeOf((*sliceType)(nil)).Elem()).id)
checkId(20, mustGetTypeInfo(reflect.TypeOf((*structType)(nil)).Elem()).id)
checkId(21, mustGetTypeInfo(reflect.TypeOf((*fieldType)(nil)).Elem()).id)
checkId(23, mustGetTypeInfo(reflect.TypeOf((*mapType)(nil)).Elem()).id)

for k, v := range idToType {
builtinIdToType[k] = v
builtinIdToTypeSlice[k] = v
}

// Move the id space upwards to allow for growth in the predefined world
// without breaking existing files.
if nextId > firstUserId {
if nextId := len(idToType); nextId > firstUserId {
panic(fmt.Sprintln("nextId too large:", nextId))
}
nextId = firstUserId
idToType = idToType[:firstUserId]
registerBasics()
wireTypeUserInfo = userType(reflect.TypeOf((*wireType)(nil)))
}
Expand Down Expand Up @@ -620,9 +625,8 @@ func bootstrapType(name string, e any, expect typeId) typeId {
typ := &CommonType{Name: name}
types[rt] = typ
setTypeId(typ)
checkId(expect, nextId)
userType(rt) // might as well cache it now
return nextId
return typ.id()
}

// Representation of the information we send and receive about this type.
Expand Down Expand Up @@ -685,7 +689,16 @@ type typeInfo struct {
// protected by a mutex.
var typeInfoMap atomic.Value

// typeInfoMapInit is used instead of typeInfoMap during init time,
// as types are registered sequentially during init and we can save
// the overhead of making map copies.
// It is saved to typeInfoMap and set to nil before init finishes.
var typeInfoMapInit = make(map[reflect.Type]*typeInfo, 16)

func lookupTypeInfo(rt reflect.Type) *typeInfo {
if m := typeInfoMapInit; m != nil {
return m[rt]
}
m, _ := typeInfoMap.Load().(map[reflect.Type]*typeInfo)
return m[rt]
}
Expand Down Expand Up @@ -750,9 +763,14 @@ func buildTypeInfo(ut *userTypeInfo, rt reflect.Type) (*typeInfo, error) {
}
}

if m := typeInfoMapInit; m != nil {
m[rt] = info
return info, nil
}

// Create new map with old contents plus new entry.
newm := make(map[reflect.Type]*typeInfo)
m, _ := typeInfoMap.Load().(map[reflect.Type]*typeInfo)
newm := make(map[reflect.Type]*typeInfo, len(m))
for k, v := range m {
newm[k] = v
}
Expand Down Expand Up @@ -911,3 +929,8 @@ func registerBasics() {
Register([]bool(nil))
Register([]string(nil))
}

func init() {
typeInfoMap.Store(typeInfoMapInit)
typeInfoMapInit = nil
}

0 comments on commit 8259ac4

Please sign in to comment.