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

@@ -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 e := s.reload(fid); e != nil {
-		if err != nil {
+			return e
-			return err
 		}
 	}
+	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 {
+	// A separate loop to avoid contending on the odd-even locks as much
-		DebugLogf("Updating the forum permissions for Group #%d", group.ID)
+	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
+			forumPerms, ok := fForumPerms[fid]
-			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
 			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 fp.Overrides {
-				if forumPerm.ViewTopic {
+				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)
+			//DebugDetail("g.ID: ", g.ID)
-			DebugDetailf("forumPerm: %+v\n", forumPerm)
+			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

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 = rows.Err(); err != nil {
-	if 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
+		permSet[f.ID] = permItem
-		presetSet[forum.ID] = forum.Preset
+		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.CacheAdd(&Group{gid, name, isMod, isAdmin, isBanned, tag, perms, []byte(permstr), pluginPerms, pluginPermsBytes, blankIntList, 0})
-	s.groups[gid] = &Group{gid, name, isMod, isAdmin, isBanned, tag, perms, []byte(permstr), pluginPerms, pluginPermsBytes, blankIntList, 0}
-	s.groupCount++
-	s.Unlock()
 
 	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()