From 66731e274048fa0a1e5d9cff60cf4e1ce76017e7 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 4 Nov 2020 10:08:10 +0530 Subject: [PATCH] MM-29980: Optimize profilesInChannels cache to fast path (#16116) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * MM-29980: Optimize profilesInChannels cache to fast path We add one more message type to the fast path- profiles in channels. There are 2 primary reasons for this: - This is not really a new model type, but just a map of users. And users already use the fast path. So we can get some more gains without really investing much more code. - A more important reason is that with the upcoming striped mutex changes, we will get a higher throughput at the cost of a bit more CPU utilization. The reason being that since less amount of time will be spent in lock-contention, the CPU is free to do more stuff. So this change is to counter that increase. As usual, this gives much better performance than the original decoder. Micro-benchmark results ``` name old time/op new time/op delta LRU/UserMap=new-8 16.6µs ± 3% 3.9µs ± 4% -76.15% (p=0.000 n=10+10) name old alloc/op new alloc/op delta LRU/UserMap=new-8 4.78kB ± 0% 2.74kB ± 0% -42.65% (p=0.000 n=10+10) name old allocs/op new allocs/op delta LRU/UserMap=new-8 38.0 ± 0% 30.0 ± 0% -21.05% (p=0.000 n=10+10) ``` https://mattermost.atlassian.net/browse/MM-29980 Here are some results from a load test. The comparison is done with a 2 node cluster; one running master and one running with this patch so that it's easier to compare. The total users are 2000. ```release-note NONE ``` * Fix gofmt * Trigger CI --- model/serialized_gen.go | 157 ++++++++++++++++++++++++++++ model/user.go | 6 ++ services/cache/lru.go | 5 + services/cache/lru_test.go | 44 ++++++++ store/localcachelayer/user_layer.go | 2 +- 5 files changed, 213 insertions(+), 1 deletion(-) diff --git a/model/serialized_gen.go b/model/serialized_gen.go index c64d88de2e..1f16f1cb74 100644 --- a/model/serialized_gen.go +++ b/model/serialized_gen.go @@ -1620,3 +1620,160 @@ func (z *User) Msgsize() (s int) { s += msgp.BoolSize + msgp.StringPrefixSize + len(z.MfaSecret) + msgp.Int64Size + msgp.BoolSize + msgp.StringPrefixSize + len(z.BotDescription) + msgp.Int64Size + msgp.StringPrefixSize + len(z.TermsOfServiceId) + msgp.Int64Size return } + +// DecodeMsg implements msgp.Decodable +func (z *UserMap) DecodeMsg(dc *msgp.Reader) (err error) { + var zb0003 uint32 + zb0003, err = dc.ReadMapHeader() + if err != nil { + err = msgp.WrapError(err) + return + } + if (*z) == nil { + (*z) = make(UserMap, zb0003) + } else if len((*z)) > 0 { + for key := range *z { + delete((*z), key) + } + } + for zb0003 > 0 { + zb0003-- + var zb0001 string + var zb0002 *User + zb0001, err = dc.ReadString() + if err != nil { + err = msgp.WrapError(err) + return + } + if dc.IsNil() { + err = dc.ReadNil() + if err != nil { + err = msgp.WrapError(err, zb0001) + return + } + zb0002 = nil + } else { + if zb0002 == nil { + zb0002 = new(User) + } + err = zb0002.DecodeMsg(dc) + if err != nil { + err = msgp.WrapError(err, zb0001) + return + } + } + (*z)[zb0001] = zb0002 + } + return +} + +// EncodeMsg implements msgp.Encodable +func (z UserMap) EncodeMsg(en *msgp.Writer) (err error) { + err = en.WriteMapHeader(uint32(len(z))) + if err != nil { + err = msgp.WrapError(err) + return + } + for zb0004, zb0005 := range z { + err = en.WriteString(zb0004) + if err != nil { + err = msgp.WrapError(err) + return + } + if zb0005 == nil { + err = en.WriteNil() + if err != nil { + return + } + } else { + err = zb0005.EncodeMsg(en) + if err != nil { + err = msgp.WrapError(err, zb0004) + return + } + } + } + return +} + +// MarshalMsg implements msgp.Marshaler +func (z UserMap) MarshalMsg(b []byte) (o []byte, err error) { + o = msgp.Require(b, z.Msgsize()) + o = msgp.AppendMapHeader(o, uint32(len(z))) + for zb0004, zb0005 := range z { + o = msgp.AppendString(o, zb0004) + if zb0005 == nil { + o = msgp.AppendNil(o) + } else { + o, err = zb0005.MarshalMsg(o) + if err != nil { + err = msgp.WrapError(err, zb0004) + return + } + } + } + return +} + +// UnmarshalMsg implements msgp.Unmarshaler +func (z *UserMap) UnmarshalMsg(bts []byte) (o []byte, err error) { + var zb0003 uint32 + zb0003, bts, err = msgp.ReadMapHeaderBytes(bts) + if err != nil { + err = msgp.WrapError(err) + return + } + if (*z) == nil { + (*z) = make(UserMap, zb0003) + } else if len((*z)) > 0 { + for key := range *z { + delete((*z), key) + } + } + for zb0003 > 0 { + var zb0001 string + var zb0002 *User + zb0003-- + zb0001, bts, err = msgp.ReadStringBytes(bts) + if err != nil { + err = msgp.WrapError(err) + return + } + if msgp.IsNil(bts) { + bts, err = msgp.ReadNilBytes(bts) + if err != nil { + return + } + zb0002 = nil + } else { + if zb0002 == nil { + zb0002 = new(User) + } + bts, err = zb0002.UnmarshalMsg(bts) + if err != nil { + err = msgp.WrapError(err, zb0001) + return + } + } + (*z)[zb0001] = zb0002 + } + o = bts + return +} + +// Msgsize returns an upper bound estimate of the number of bytes occupied by the serialized message +func (z UserMap) Msgsize() (s int) { + s = msgp.MapHeaderSize + if z != nil { + for zb0004, zb0005 := range z { + _ = zb0005 + s += msgp.StringPrefixSize + len(zb0004) + if zb0005 == nil { + s += msgp.NilSize + } else { + s += zb0005.Msgsize() + } + } + } + return +} diff --git a/model/user.go b/model/user.go index a053e53438..dd4e2ba8ca 100644 --- a/model/user.go +++ b/model/user.go @@ -98,6 +98,12 @@ type User struct { TermsOfServiceCreateAt int64 `db:"-" json:"terms_of_service_create_at,omitempty"` } +//msgp UserMap + +// UserMap is a map from a userId to a user object. +// It is used to generate methods which can be used for fast serialization/de-serialization. +type UserMap map[string]*User + type UserUpdate struct { Old *User New *User diff --git a/services/cache/lru.go b/services/cache/lru.go index 0ab71b3d72..5aa1c98d90 100644 --- a/services/cache/lru.go +++ b/services/cache/lru.go @@ -221,6 +221,11 @@ func (l *LRU) get(key string, value interface{}) error { _, err := s.UnmarshalMsg(e.value) *v = &s return err + case *map[string]*model.User: + var u model.UserMap + _, err := u.UnmarshalMsg(e.value) + *v = u + return err } // Slow path for other structs. diff --git a/services/cache/lru_test.go b/services/cache/lru_test.go index 041b7cce6f..97e1ca1ea1 100644 --- a/services/cache/lru_test.go +++ b/services/cache/lru_test.go @@ -279,6 +279,17 @@ func TestLRUMarshalUnMarshal(t *testing.T) { // This does not make an actual difference in terms of functionality. u.Timezone = nil require.Equal(t, user, u) + + tt := make(map[string]*model.User) + tt["1"] = u + err = l.Set("mm", model.UserMap(tt)) + require.Nil(t, err) + + var out map[string]*model.User + err = l.Get("mm", &out) + require.Nil(t, err) + out["1"].Timezone = nil + require.Equal(t, tt, out) } func BenchmarkLRU(b *testing.B) { @@ -434,6 +445,39 @@ func BenchmarkLRU(b *testing.B) { } }) + uMap := map[string]*model.User{ + "id1": { + Id: "id1", + CreateAt: 1111, + UpdateAt: 1112, + Username: "user1", + Password: "pass", + }, + "id2": { + Id: "id2", + CreateAt: 1113, + UpdateAt: 1114, + Username: "user2", + Password: "pass2", + }, + } + + b.Run("UserMap=new", func(b *testing.B) { + for i := 0; i < b.N; i++ { + l2 := NewLRU(&LRUOptions{ + Size: 1, + DefaultExpiry: 0, + InvalidateClusterEvent: "", + }) + err := l2.Set("test", model.UserMap(uMap)) + require.Nil(b, err) + + var val map[string]*model.User + err = l2.Get("test", &val) + require.Nil(b, err) + } + }) + post := &model.Post{ Id: "id", CreateAt: 11111, diff --git a/store/localcachelayer/user_layer.go b/store/localcachelayer/user_layer.go index cdf9997784..2f497c8ac1 100644 --- a/store/localcachelayer/user_layer.go +++ b/store/localcachelayer/user_layer.go @@ -85,7 +85,7 @@ func (s LocalCacheUserStore) GetAllProfilesInChannel(channelId string, allowFrom } if allowFromCache { - s.rootStore.doStandardAddToCache(s.rootStore.profilesInChannelCache, channelId, userMap) + s.rootStore.doStandardAddToCache(s.rootStore.profilesInChannelCache, channelId, model.UserMap(userMap)) } return userMap, nil