From ea4da062c33c0f45c90a377d72f6d81d0a3e5568 Mon Sep 17 00:00:00 2001 From: Azareal Date: Wed, 28 Apr 2021 19:29:15 +1000 Subject: [PATCH] Use Rstore.Exists() instead of Rstore.Get() in ReplyEditSubmit. Reduce allocs in WsHubImpl.getUsers() Reduce allocs in wolContextRender() Use b.String() instead of string(b.Bytes()) in preparseWidget() Reduce boilerplate. --- common/widget_wol_context.go | 9 ++-- common/widgets.go | 98 ++++++++++++++++++------------------ common/ws_hub.go | 73 ++++++++++++++------------- routes/poll.go | 5 +- routes/reply.go | 31 +++++------- 5 files changed, 109 insertions(+), 107 deletions(-) diff --git a/common/widget_wol_context.go b/common/widget_wol_context.go index d86e0f31..602ac3cf 100644 --- a/common/widget_wol_context.go +++ b/common/widget_wol_context.go @@ -14,13 +14,16 @@ func wolContextRender(widget *Widget, hvars interface{}) (string, error) { if ok { ucount = len(topic) if ucount < 30 { + users = make([]*User, len(topic)) + i := 0 for wsUser, _ := range topic { - users = append(users, wsUser.User) + users[i] = wsUser.User + i++ } } } topicMutex.RUnlock() wol := &wolUsers{header, phrases.GetTmplPhrase("widget.online_view_topic_name"), users, ucount} - err := header.Theme.RunTmpl("widget_online", wol, header.Writer) - return "", err + e := header.Theme.RunTmpl("widget_online", wol, header.Writer) + return "", e } diff --git a/common/widgets.go b/common/widgets.go index 8e318bcd..81079f40 100644 --- a/common/widgets.go +++ b/common/widgets.go @@ -49,15 +49,15 @@ type NameTextPair struct { Text template.HTML } -func preparseWidget(w *Widget, wdata string) (err error) { +func preparseWidget(w *Widget, wdata string) (e error) { prebuildWidget := func(name string, data interface{}) (string, error) { var b bytes.Buffer - err := DefaultTemplates.ExecuteTemplate(&b, name+".html", data) - content := string(b.Bytes()) + e := DefaultTemplates.ExecuteTemplate(&b, name+".html", data) + content := b.String() if Config.MinifyTemplates { content = min.Minify(content) } - return content, err + return content, e } sbytes := []byte(wdata) @@ -66,11 +66,11 @@ func preparseWidget(w *Widget, wdata string) (err error) { switch w.Type { case "simple", "about": var tmp NameTextPair - err = json.Unmarshal(sbytes, &tmp) - if err != nil { - return err + e = json.Unmarshal(sbytes, &tmp) + if e != nil { + return e } - w.Body, err = prebuildWidget("widget_"+w.Type, tmp) + w.Body, e = prebuildWidget("widget_"+w.Type, tmp) case "search_and_filter": w.Literal = false w.BuildFunc = widgetSearchAndFilter @@ -88,9 +88,12 @@ func preparseWidget(w *Widget, wdata string) (err error) { // TODO: Test this // TODO: Should we toss this through a proper parser rather than crudely replacing it? - w.Location = strings.Replace(w.Location, " ", "", -1) - w.Location = strings.Replace(w.Location, "frontend", "!panel", -1) - w.Location = strings.Replace(w.Location, "!!", "", -1) + rep := func(from, to string) { + w.Location = strings.Replace(w.Location, from, to, -1) + } + rep(" ", "") + rep("frontend", "!panel") + rep("!!", "") // Skip blank zones locs := strings.Split(w.Location, "|") @@ -105,7 +108,7 @@ func preparseWidget(w *Widget, wdata string) (err error) { w.Location = w.Location[:len(w.Location)-1] } - return err + return e } func GetDockList() []string { @@ -193,11 +196,11 @@ func BuildWidget(dock string, h *Header) (sbody string) { widgets = Docks.RightOfNav case "topMenu": // 1 = id for the default menu - mhold, err := Menus.Get(1) - if err == nil { - err := mhold.Build(h.Writer, h.CurrentUser, h.Path) - if err != nil { - LogError(err) + mhold, e := Menus.Get(1) + if e == nil { + e := mhold.Build(h.Writer, h.CurrentUser, h.Path) + if e != nil { + LogError(e) } } return "" @@ -212,9 +215,9 @@ func BuildWidget(dock string, h *Header) (sbody string) { continue } if widget.Allowed(h.Zone, h.ZoneID) { - item, err := widget.Build(h) - if err != nil { - LogError(err) + item, e := widget.Build(h) + if e != nil { + LogError(e) } sbody += item } @@ -248,11 +251,11 @@ func BuildWidget2(dock int, h *Header) (sbody string) { widgets = Docks.RightOfNav case 2: // 1 = id for the default menu - mhold, err := Menus.Get(1) - if err == nil { - err := mhold.Build(h.Writer, h.CurrentUser, h.Path) - if err != nil { - LogError(err) + mhold, e := Menus.Get(1) + if e == nil { + e := mhold.Build(h.Writer, h.CurrentUser, h.Path) + if e != nil { + LogError(e) } } return "" @@ -267,9 +270,9 @@ func BuildWidget2(dock int, h *Header) (sbody string) { continue } if w.Allowed(h.Zone, h.ZoneID) { - item, err := w.Build(h) - if err != nil { - LogError(err) + item, e := w.Build(h) + if e != nil { + LogError(e) } sbody += item } @@ -314,9 +317,9 @@ func BuildWidget3(dock int, h *Header) { continue } if w.Allowed(h.Zone, h.ZoneID) { - item, err := w.Build(h) - if err != nil { - LogError(err) + item, e := w.Build(h) + if e != nil { + LogError(e) } if item != "" { h.Writer.Write(uutils.StringToBytes(item)) @@ -362,23 +365,22 @@ func HasWidgets2(dock int, h *Header) bool { return wcount > 0 } -func getDockWidgets(dock string) (widgets []*Widget, err error) { - rows, err := widgetStmts.getDockList.Query(dock) - if err != nil { - return nil, err +func getDockWidgets(dock string) (widgets []*Widget, e error) { + rows, e := widgetStmts.getDockList.Query(dock) + if e != nil { + return nil, e } defer rows.Close() for rows.Next() { w := &Widget{Position: 0, Side: dock} - err = rows.Scan(&w.ID, &w.Position, &w.Type, &w.Enabled, &w.Location, &w.RawBody) - if err != nil { - return nil, err + e = rows.Scan(&w.ID, &w.Position, &w.Type, &w.Enabled, &w.Location, &w.RawBody) + if e != nil { + return nil, e } - - err = preparseWidget(w, w.RawBody) - if err != nil { - return nil, err + e = preparseWidget(w, w.RawBody) + if e != nil { + return nil, e } Widgets.set(w) widgets = append(widgets, w) @@ -393,9 +395,9 @@ func InitWidgets() (fi error) { if fi != nil { return } - dock, err := getDockWidgets(name) - if err != nil { - fi = err + dock, e := getDockWidgets(name) + if e != nil { + fi = e return } setDock(name, dock) @@ -484,9 +486,9 @@ func (s *WidgetScheduler) Tick() error { if widget.TickFunc == nil { continue } - err := widget.TickFunc(widget) - if err != nil { - return errors.WithStack(err) + e := widget.TickFunc(widget) + if e != nil { + return errors.WithStack(e) } } return nil diff --git a/common/ws_hub.go b/common/ws_hub.go index 6f47b65a..394ba469 100644 --- a/common/ws_hub.go +++ b/common/ws_hub.go @@ -51,8 +51,8 @@ func (h *WsHubImpl) Start() { l.RLock() defer l.RUnlock() // TODO: Copy to temporary slice for less contention? - for _, user := range userMap { - user.Ping() + for _, u := range userMap { + u.Ping() } } select { @@ -135,19 +135,19 @@ func wsTopicListTick(h *WsHubImpl) error { groups := make(map[int]*Group) canSeeMap := make(map[string][]int) - for groupID, _ := range groupIDs { - group, err := Groups.Get(groupID) + for gid, _ := range groupIDs { + g, err := Groups.Get(gid) if err != nil { // TODO: Do we really want to halt all pushes for what is possibly just one user? return err } - groups[group.ID] = group + groups[g.ID] = g - canSee := make([]byte, len(group.CanSee)) - for i, item := range group.CanSee { + canSee := make([]byte, len(g.CanSee)) + for i, item := range g.CanSee { canSee[i] = byte(item) } - canSeeMap[string(canSee)] = group.CanSee + canSeeMap[string(canSee)] = g.CanSee } canSeeRenders := make(map[string][]byte) @@ -213,11 +213,11 @@ func wsTopicListTick(h *WsHubImpl) error { modSet = make(map[int]int, len(l)) for i, t := range l { // TODO: Abstract this? - fp, err := FPStore.Get(t.ParentID, u.Group) - if err == ErrNoRows { + fp, e := FPStore.Get(t.ParentID, u.Group) + if e == ErrNoRows { fp = BlankForumPerms() - } else if err != nil { - return err + } else if e != nil { + return e } var ccanMod, ccanLock, ccanMove bool if fp.Overrides { @@ -329,17 +329,17 @@ func (h *WsHubImpl) broadcastMessage(msg string) error { m.RLock() defer m.RUnlock() for _, wsUser := range users { - err := wsUser.WriteAll(msg) - if err != nil { - return err + e := wsUser.WriteAll(msg) + if e != nil { + return e } } return nil } // TODO: Can we move this RLock inside the closure safely? - err := userLoop(h.evenOnlineUsers, &h.evenUserLock) - if err != nil { - return err + e := userLoop(h.evenOnlineUsers, &h.evenUserLock) + if e != nil { + return e } return userLoop(h.oddOnlineUsers, &h.oddUserLock) } @@ -366,12 +366,15 @@ func (h *WsHubImpl) getUsers(uids []int) (wsUsers []*WSUser, err error) { if len(uids) == 0 { return nil, errWsNouser } + wsUsers = make([]*WSUser, len(uids)) + i := 0 appender := func(l *sync.RWMutex, users map[int]*WSUser) { l.RLock() defer l.RUnlock() // We don't want to keep a lock on this for too long, so we'll accept some nil pointers for _, uid := range uids { - wsUsers = append(wsUsers, users[uid]) + wsUsers[i] = users[uid] + i++ } } appender(&h.evenUserLock, h.evenOnlineUsers) @@ -387,8 +390,8 @@ func (h *WsHubImpl) AllUsers() (users []*User) { appender := func(l *sync.RWMutex, userMap map[int]*WSUser) { l.RLock() defer l.RUnlock() - for _, user := range userMap { - users = append(users, user.User) + for _, u := range userMap { + users = append(users, u.User) } } appender(&h.evenUserLock, h.evenOnlineUsers) @@ -461,26 +464,26 @@ func (h *WsHubImpl) RemoveConn(wsUser *WSUser, conn *websocket.Conn) { } func (h *WsHubImpl) PushMessage(targetUser int, msg string) error { - wsUser, err := h.getUser(targetUser) - if err != nil { - return err + wsUser, e := h.getUser(targetUser) + if e != nil { + return e } return wsUser.WriteAll(msg) } -func (h *WsHubImpl) pushAlert(targetUser int, alert Alert) error { - wsUser, err := h.getUser(targetUser) - if err != nil { - return err +func (h *WsHubImpl) pushAlert(targetUser int, a Alert) error { + wsUser, e := h.getUser(targetUser) + if e != nil { + return e } - astr, err := BuildAlert(alert, *wsUser.User) - if err != nil { - return err + astr, e := BuildAlert(a, *wsUser.User) + if e != nil { + return e } return wsUser.WriteAll(astr) } -func (h *WsHubImpl) pushAlerts(users []int, alert Alert) error { +func (h *WsHubImpl) pushAlerts(users []int, a Alert) error { wsUsers, err := h.getUsers(users) if err != nil { return err @@ -491,7 +494,7 @@ func (h *WsHubImpl) pushAlerts(users []int, alert Alert) error { if wsUser == nil { continue } - alert, err := BuildAlert(alert, *wsUser.User) + alert, err := BuildAlert(a, *wsUser.User) if err != nil { errs = append(errs, err) } @@ -503,8 +506,8 @@ func (h *WsHubImpl) pushAlerts(users []int, alert Alert) error { // Return the first error if len(errs) != 0 { - for _, err := range errs { - return err + for _, e := range errs { + return e } } return nil diff --git a/routes/poll.go b/routes/poll.go index b9ce3a90..c38a3cd0 100644 --- a/routes/poll.go +++ b/routes/poll.go @@ -88,9 +88,8 @@ func PollResults(w http.ResponseWriter, r *http.Request, u *c.User, sPollID stri optList := "" var votes int for rows.Next() { - err := rows.Scan(&votes) - if err != nil { - return c.InternalError(err, w, r) + if e := rows.Scan(&votes); e != nil { + return c.InternalError(e, w, r) } optList += strconv.Itoa(votes) + "," } diff --git a/routes/reply.go b/routes/reply.go index dbc11b42..d4570875 100644 --- a/routes/reply.go +++ b/routes/reply.go @@ -225,13 +225,8 @@ func ReplyEditSubmit(w http.ResponseWriter, r *http.Request, u *c.User, srid str } else if err != nil { return c.InternalErrorJSQ(err, w, r, js) } - - // TODO: Avoid the load to get this faster? - reply, err = c.Rstore.Get(reply.ID) - if err == sql.ErrNoRows { + if !c.Rstore.Exists(reply.ID) { return c.PreErrorJSQ("The updated reply doesn't exist.", w, r, js) - } else if err != nil { - return c.InternalErrorJSQ(err, w, r, js) } skip, rerr := lite.Hooks.VhookSkippable("action_end_edit_reply", reply.ID, u) @@ -265,8 +260,8 @@ func ReplyDeleteSubmit(w http.ResponseWriter, r *http.Request, u *c.User, srid s return c.NoPermissionsJSQ(w, r, u, js) } } - if err := reply.Delete(); err != nil { - return c.InternalErrorJSQ(err, w, r, js) + if e := reply.Delete(); e != nil { + return c.InternalErrorJSQ(e, w, r, js) } skip, rerr := lite.Hooks.VhookSkippable("action_end_delete_reply", reply.ID, u) @@ -282,19 +277,19 @@ func ReplyDeleteSubmit(w http.ResponseWriter, r *http.Request, u *c.User, srid s } // ? - What happens if an error fires after a redirect...? - /*creator, err := c.Users.Get(reply.CreatedBy) - if err == nil { - err = creator.DecreasePostStats(c.WordCount(reply.Content), false) - if err != nil { - return c.InternalErrorJSQ(err, w, r, js) + /*creator, e := c.Users.Get(reply.CreatedBy) + if e == nil { + e = creator.DecreasePostStats(c.WordCount(reply.Content), false) + if e != nil { + return c.InternalErrorJSQ(e, w, r, js) } - } else if err != sql.ErrNoRows { - return c.InternalErrorJSQ(err, w, r, js) + } else if e != sql.ErrNoRows { + return c.InternalErrorJSQ(e, w, r, js) }*/ - err := c.ModLogs.Create("delete", reply.ParentID, "reply", u.GetIP(), u.ID) - if err != nil { - return c.InternalErrorJSQ(err, w, r, js) + e := c.ModLogs.Create("delete", reply.ParentID, "reply", u.GetIP(), u.ID) + if e != nil { + return c.InternalErrorJSQ(e, w, r, js) } return nil }