From 1fa6b3ff261981a78e64227efc1b5892a3355cf2 Mon Sep 17 00:00:00 2001 From: Azareal Date: Mon, 22 Mar 2021 19:58:13 +1000 Subject: [PATCH] Optimise MemoryForumPermsStore by splitting recalcCanSeeAll out of Reload and only calling it once. Reduce the amount of lock contention in recalcCanSeeAll by only loading each forum once. Stop logging the group ID twice in recalcCanSeeAll. Add the CacheAdd method to GroupCache and use it. Reduce the amount of boilerplate. --- common/forum_perms_store.go | 86 ++++++++++++++++++++++++------------- common/group_store.go | 27 +++++++----- 2 files changed, 73 insertions(+), 40 deletions(-) diff --git a/common/forum_perms_store.go b/common/forum_perms_store.go index fa8616b6..549cb7c2 100644 --- a/common/forum_perms_store.go +++ b/common/forum_perms_store.go @@ -49,6 +49,7 @@ func (s *MemoryForumPermsStore) Init() error { return s.ReloadAll() } +// TODO: Optimise this? func (s *MemoryForumPermsStore) ReloadAll() error { DebugLog("Reloading the forum perms") fids, err := Forums.GetAllIDs() @@ -56,11 +57,14 @@ func (s *MemoryForumPermsStore) ReloadAll() error { return err } for _, fid := range fids { - err := s.Reload(fid) - if err != nil { - return err + if e := s.reload(fid); e != nil { + return e } } + if e := s.recalcCanSeeAll(); e != nil { + return e + } + TopicListThaw.Thaw() return nil } @@ -73,8 +77,20 @@ func (s *MemoryForumPermsStore) parseForumPerm(perms []byte) (pperms *ForumPerms return pperms, err } -// TODO: Need a more thread-safe way of doing this. Possibly with sync.Map? func (s *MemoryForumPermsStore) Reload(fid int) error { + e := s.reload(fid) + if e != nil { + return e + } + if e = s.recalcCanSeeAll(); e != nil { + return e + } + TopicListThaw.Thaw() + return nil +} + +// TODO: Need a more thread-safe way of doing this. Possibly with sync.Map? +func (s *MemoryForumPermsStore) reload(fid int) error { DebugLogf("Reloading the forum permissions for forum #%d", fid) rows, err := s.getByForum.Query(fid) if err != nil { @@ -111,7 +127,10 @@ func (s *MemoryForumPermsStore) Reload(fid int) error { s.oddForums[fid] = forumPerms s.oddLock.Unlock() } + return nil +} +func (s *MemoryForumPermsStore) recalcCanSeeAll() error { groups, err := Groups.GetAll() if err != nil { return err @@ -121,56 +140,64 @@ func (s *MemoryForumPermsStore) Reload(fid int) error { return err } - gcache, ok := Groups.(GroupCache) + gc, ok := Groups.(GroupCache) if !ok { TopicListThaw.Thaw() return nil } - for _, group := range groups { - DebugLogf("Updating the forum permissions for Group #%d", group.ID) + // A separate loop to avoid contending on the odd-even locks as much + fForumPerms := make(map[int]map[int]*ForumPerms) + for _, fid := range fids { + var forumPerms map[int]*ForumPerms + var ok bool + if fid%2 == 0 { + s.evenLock.RLock() + forumPerms, ok = s.evenForums[fid] + s.evenLock.RUnlock() + } else { + s.oddLock.RLock() + forumPerms, ok = s.oddForums[fid] + s.oddLock.RUnlock() + } + if ok { + fForumPerms[fid] = forumPerms + } + } + + // TODO: Can we recalculate CanSee without calculating every other forum? + for _, g := range groups { + DebugLogf("Updating the forum permissions for Group #%d", g.ID) canSee := []int{} for _, fid := range fids { DebugDetailf("Forum #%+v\n", fid) - var forumPerms map[int]*ForumPerms - var ok bool - if fid%2 == 0 { - s.evenLock.RLock() - forumPerms, ok = s.evenForums[fid] - s.evenLock.RUnlock() - } else { - s.oddLock.RLock() - forumPerms, ok = s.oddForums[fid] - s.oddLock.RUnlock() - } - - var forumPerm *ForumPerms + forumPerms, ok := fForumPerms[fid] if !ok { continue } - forumPerm, ok = forumPerms[group.ID] + fp, ok := forumPerms[g.ID] if !ok { - if group.Perms.ViewTopic { + if g.Perms.ViewTopic { canSee = append(canSee, fid) } continue } - if forumPerm.Overrides { - if forumPerm.ViewTopic { + if fp.Overrides { + if fp.ViewTopic { canSee = append(canSee, fid) } - } else if group.Perms.ViewTopic { + } else if g.Perms.ViewTopic { canSee = append(canSee, fid) } - DebugDetail("group.ID: ", group.ID) - DebugDetailf("forumPerm: %+v\n", forumPerm) + //DebugDetail("g.ID: ", g.ID) + DebugDetailf("forumPerm: %+v\n", fp) DebugDetail("canSee: ", canSee) } DebugDetailf("canSee (length %d): %+v \n", len(canSee), canSee) - gcache.SetCanSee(group.ID, canSee) + gc.SetCanSee(g.ID, canSee) } - TopicListThaw.Thaw() + return nil } @@ -193,6 +220,7 @@ func (s *MemoryForumPermsStore) GetAllMap() (bigMap map[int]map[int]*ForumPerms) // TODO: Add a hook here and have plugin_guilds use it // TODO: Check if the forum exists? // TODO: Fix the races +// TODO: Return BlankForumPerms() when the forum permission set doesn't exist? func (s *MemoryForumPermsStore) Get(fid, gid int) (fp *ForumPerms, err error) { var fmap map[int]*ForumPerms var ok bool diff --git a/common/group_store.go b/common/group_store.go index d16872a5..9a945186 100644 --- a/common/group_store.go +++ b/common/group_store.go @@ -32,6 +32,7 @@ type GroupStore interface { type GroupCache interface { CacheSet(g *Group) error SetCanSee(gid int, canSee []int) error + CacheAdd(g *Group) error Length() int } @@ -85,8 +86,7 @@ func (s *MemoryGroupStore) LoadGroups() error { } s.groups[g.ID] = g } - err = rows.Err() - if err != nil { + if err = rows.Err(); err != nil { return err } s.groupCount = i @@ -256,14 +256,14 @@ func (s *MemoryGroupStore) Create(name, tag string, isAdmin, isMod, isBanned boo GetHookTable().Vhook("create_group_preappend", &pluginPerms, &pluginPermsBytes) // Generate the forum permissions based on the presets... - fdata, err := Forums.GetAll() + forums, err := Forums.GetAll() if err != nil { return 0, err } presetSet := make(map[int]string) permSet := make(map[int]*ForumPerms) - for _, forum := range fdata { + for _, f := range forums { var thePreset string switch { case isAdmin: @@ -276,12 +276,12 @@ func (s *MemoryGroupStore) Create(name, tag string, isAdmin, isMod, isBanned boo thePreset = "members" } - permmap := PresetToPermmap(forum.Preset) + permmap := PresetToPermmap(f.Preset) permItem := permmap[thePreset] permItem.Overrides = true - permSet[forum.ID] = permItem - presetSet[forum.ID] = forum.Preset + permSet[f.ID] = permItem + presetSet[f.ID] = f.Preset } err = ReplaceForumPermsForGroupTx(tx, gid, presetSet, permSet) @@ -298,16 +298,21 @@ func (s *MemoryGroupStore) Create(name, tag string, isAdmin, isMod, isBanned boo isBanned = false } - s.Lock() - s.groups[gid] = &Group{gid, name, isMod, isAdmin, isBanned, tag, perms, []byte(permstr), pluginPerms, pluginPermsBytes, blankIntList, 0} - s.groupCount++ - s.Unlock() + s.CacheAdd(&Group{gid, name, isMod, isAdmin, isBanned, tag, perms, []byte(permstr), pluginPerms, pluginPermsBytes, blankIntList, 0}) TopicListThaw.Thaw() return gid, FPStore.ReloadAll() //return gid, TopicList.RebuildPermTree() } +func (s *MemoryGroupStore) CacheAdd(g *Group) error { + s.Lock() + s.groups[g.ID] = g + s.groupCount++ + s.Unlock() + return nil +} + func (s *MemoryGroupStore) GetAll() (results []*Group, err error) { var i int s.RLock()