Skip to content

Commit

Permalink
Don't need Max
Browse files Browse the repository at this point in the history
It would use a separate query/widget to get the maximum value for the
given time range; this is needed so we know how to scale the charts: the
second page may have a lot of visits for one day, even though the first
page has more visits overall.

But we can just get that by a for loop since we already have all that
data, we don't really need a query. Turns out that looping over 10 items
is a lot faster than scanning over 100k rows in SQL, who would have
guessed 'ey? This is a fairly significant performance win for larger
sites.

The reason it used to do a query is cases where a page has more visits
after pagination than the previous maximum value, but we can just
re-render the charts with the new max value; that used to be slow, but
now it's fast.
  • Loading branch information
arp242 committed Oct 26, 2022
1 parent 99ad5f9 commit f1fae72
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 199 deletions.
18 changes: 0 additions & 18 deletions db/query/hit_list.GetMax-daily.sql

This file was deleted.

4 changes: 0 additions & 4 deletions db/query/hit_list.GetMax-hourly.sql

This file was deleted.

30 changes: 10 additions & 20 deletions handlers/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,18 +192,6 @@ func (h backend) dashboard(w http.ResponseWriter, r *http.Request) error {
shared.Total, shared.TotalUnique, shared.TotalUniqueUTC, shared.TotalEvents, shared.TotalEventsUnique =
tc.Total, tc.TotalUnique, tc.TotalUniqueUTC, tc.TotalEvents, tc.TotalEventsUnique

// Copy max and refs to pages; they're in separate "widgets" so they can run
// in parallel.
//
if pages := wid.Get("pages"); len(pages) > 0 {
if m := wid.GetOne("max"); m != nil {
max := m.(*widgets.Max)
for _, p := range pages {
p.(*widgets.Pages).Max = max.Max
}
}
}

// Render widget templates.
func() {
var wg sync.WaitGroup
Expand Down Expand Up @@ -367,7 +355,9 @@ func (h backend) loadWidget(w http.ResponseWriter, r *http.Request) error {
}
switch wid.Name() {
case "pages":
ret["total_unique_display"] = wid.(*widgets.Pages).UniqueDisplay
p := wid.(*widgets.Pages)
ret["total_unique_display"] = p.UniqueDisplay
ret["max"] = p.Max
}

return zhttp.JSON(w, ret)
Expand All @@ -381,15 +371,15 @@ func (h backend) loadWidget(w http.ResponseWriter, r *http.Request) error {
//
// Values for rng:
//
// week, month, quarter, half-year, year
// The start date is set to exactly this period ago. The end date is set to
// the end of the current day.
// week, month, quarter, half-year, year
// The start date is set to exactly this period ago. The end date is set to
// the end of the current day.
//
// week-cur, month-cur
// The current week or month; both the start and return are modified.
// week-cur, month-cur
// The current week or month; both the start and return are modified.
//
// Any digit
// Last n days.
// Any digit
// Last n days.
func timeRange(r string, tz *time.Location, sundayStartsWeek bool) ztime.Range {
rng := ztime.NewRange(ztime.Now().In(tz)).Current(ztime.Day)
switch r {
Expand Down
68 changes: 13 additions & 55 deletions hit_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,23 +319,23 @@ func (h *HitList) Totals(ctx context.Context, rng ztime.Range, pathFilter []int6
// Let's say we have two days with an offset of UTC+2, this means we
// need to transform this:
//
// 2019-12-05 → [0,0,0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0,1,0]
// 2019-12-06 → [0,0,0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0,1,0]
// 2019-12-07 → [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
// 2019-12-05 → [0,0,0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0,1,0]
// 2019-12-06 → [0,0,0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0,1,0]
// 2019-12-07 → [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
//
// To:
//
// 2019-12-05 → [0,0,0,0,0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0]
// 2019-12-06 → [1,0,0,0,0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0]
// 2019-12-07 → [1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
// 2019-12-05 → [0,0,0,0,0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0]
// 2019-12-06 → [1,0,0,0,0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0]
// 2019-12-07 → [1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
//
// And skip the first 2 hours of the first day.
//
// Or, for UTC-2:
//
// 2019-12-04 → [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
// 2019-12-05 → [0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0,1,0,0,0]
// 2019-12-06 → [0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0,1,0,0,0]
// 2019-12-04 → [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0]
// 2019-12-05 → [0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0,1,0,0,0]
// 2019-12-06 → [0,0,0,0,0,0,0,0,0,4,7,0,0,0,0,0,0,0,0,0,1,0,0,0]
//
// And skip the last 2 hours of the last day.
//
Expand Down Expand Up @@ -434,15 +434,15 @@ func addTotals(hh HitLists, daily bool, totalDisplay, totalUniqueDisplay *int) {
for k := range hh[i].Stats[j].Hourly {
hh[i].Stats[j].Daily += hh[i].Stats[j].Hourly[k]
hh[i].Stats[j].DailyUnique += hh[i].Stats[j].HourlyUnique[k]
if !daily && hh[i].Stats[j].Hourly[k] > hh[i].Max {
hh[i].Max = hh[i].Stats[j].Hourly[k]
if !daily && hh[i].Stats[j].HourlyUnique[k] > hh[i].Max {
hh[i].Max = hh[i].Stats[j].HourlyUnique[k]
}
}

hh[i].Count += hh[i].Stats[j].Daily
hh[i].CountUnique += hh[i].Stats[j].DailyUnique
if daily && hh[i].Stats[j].Daily > hh[i].Max {
hh[i].Max = hh[i].Stats[j].Daily
if daily && hh[i].Stats[j].DailyUnique > hh[i].Max {
hh[i].Max = hh[i].Stats[j].DailyUnique
}
}

Expand Down Expand Up @@ -498,48 +498,6 @@ func GetTotalCount(ctx context.Context, rng ztime.Range, pathFilter []int64, noE
return t, errors.Wrap(err, "GetTotalCount")
}

// GetMax gets the path with the higest number of pageviews per hour or day for
// this date range.
func GetMax(ctx context.Context, rng ztime.Range, pathFilter []int64, daily bool) (int, error) {
site := MustGetSite(ctx)
user := MustGetUser(ctx)
var (
query string
params zdb.P
)
if daily {
query = "load:hit_list.GetMax-daily"
params = zdb.P{
"site": site.ID,
"start": rng.Start,
"end": rng.End,
"tz": user.Settings.Timezone.OffsetRFC3339(),
"filter": pathFilter,
"pgsql": zdb.SQLDialect(ctx) == zdb.DialectPostgreSQL,
"sqlite": zdb.SQLDialect(ctx) == zdb.DialectSQLite,
}
} else {
query = "load:hit_list.GetMax-hourly"
params = zdb.P{
"site": site.ID,
"start": rng.Start,
"end": rng.End,
"filter": pathFilter,
}
}

var max int
err := zdb.Get(ctx, &max, query, params)
if err != nil && !zdb.ErrNoRows(err) {
return 0, errors.Wrap(err, "getMax")
}

if max < 10 {
max = 10
}
return max, nil
}

// Diff gets the difference in percentage of all paths in this HitList.
//
// e.g. if called with start=2020-01-20; end=2020-01-2020-01-27, then it will
Expand Down
50 changes: 0 additions & 50 deletions hit_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,56 +186,6 @@ func TestHitListsList(t *testing.T) {
}
}

func TestGetMax(t *testing.T) {
ztime.SetNow(t, "2020-06-18 12:00:00")
ctx := gctest.DB(t)

rng := ztime.NewRange(ztime.Now()).To(ztime.Now())

gctest.StoreHits(ctx, t, false,
Hit{Path: "/a"},
Hit{Path: "/b"},
Hit{Path: "/a"},
Hit{Path: "/a"},
Hit{Path: "/a"},
Hit{Path: "/a"},
Hit{Path: "/a"},
Hit{Path: "/a"},
Hit{Path: "/a"},
Hit{Path: "/a"},
Hit{Path: "/a"},
Hit{Path: "/a"},
)

t.Run("hourly", func(t *testing.T) {
want := []int{10, 10, 10, 10}
for i, filter := range [][]int64{nil, []int64{1}, []int64{2}, []int64{1, 2}} {
have, err := GetMax(ctx, rng, filter, false)
if err != nil {
t.Fatal(err)
}
w := want[i]
if have != w {
t.Errorf("have %d; want %d (filter=%v)", have, w, filter)
}
}
})

t.Run("daily", func(t *testing.T) {
want := []int{10, 10, 10, 10}
for i, filter := range [][]int64{nil, []int64{1}, []int64{2}, []int64{1, 2}} {
have, err := GetMax(ctx, rng, filter, true)
if err != nil {
t.Fatal(err)
}
w := want[i]
if have != w {
t.Errorf("have %d; want %d (filter=%v)", have, w, filter)
}
}
})
}

func TestGetTotalCount(t *testing.T) {
ztime.SetNow(t, "2020-06-18 12:00:00")
ctx := gctest.DB(t)
Expand Down
14 changes: 11 additions & 3 deletions public/dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@
}

// Get the Y-axis scale.
var get_original_scale = function(current) { return $('.count-list-pages').attr('data-max') }
var get_current_scale = function(current) { return $('.count-list-pages').attr('data-scale') }
var get_original_scale = function() { return parseInt($('.count-list-pages').attr('data-max'), 0) }
var get_current_scale = function() { return parseInt($('.count-list-pages').attr('data-scale'), 0) }

// Reload a single widget.
var reload_widget = function(wid, data, done) {
Expand Down Expand Up @@ -478,7 +478,7 @@

let ctx = canvas.getContext('2d', {alpha: false}),
max = Math.max(10, parseInt(c.dataset.max, 10)),
scale = parseInt($(c).closest('.count-list-pages').attr('data-scale'), 0),
scale = get_current_scale(),
daily = c.dataset.daily === 'true',
isBar = $(c).is('.chart-bar'),
isEvent = $(c).closest('tr').hasClass('event'),
Expand Down Expand Up @@ -655,6 +655,14 @@
}),
success: function(data) {
pages.find('.count-list-pages >tbody.pages').append(data.html)

// Update scale in case it's higher than the previous maximum value.
if (data.max > get_original_scale()) {
$('.count-list-pages').attr('data-max', data.max)
$('.count-list-pages').attr('data-scale', data.max)
$('.count-list-pages .scale').text(data.max)
}

draw_all_charts()

highlight_filter($('#filter-paths').val())
Expand Down
30 changes: 0 additions & 30 deletions widgets/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,36 +11,6 @@ import (
"zgo.at/goatcounter/v2"
)

// TODO: do we actually need this? Seems to me we can just use the "max" from
// the pages overview? If pagination adds new pages with more stuff we can
// always recalc the lot. I don't know why I did it like this; may be a
// reason...
type Max struct {
loaded bool
err error
html template.HTML
s goatcounter.WidgetSettings

Max int
}

func (w Max) Name() string { return "max" }
func (w Max) Type() string { return "data-only" }
func (w Max) Label(ctx context.Context) string { return "" }
func (w *Max) SetHTML(h template.HTML) {}
func (w Max) HTML() template.HTML { return w.html }
func (w *Max) SetErr(h error) { w.err = h }
func (w Max) Err() error { return w.err }
func (w Max) ID() int { return 0 }
func (w Max) Settings() goatcounter.WidgetSettings { return w.s }
func (w *Max) SetSettings(s goatcounter.WidgetSettings) { w.s = s }
func (w Max) RenderHTML(context.Context, SharedData) (string, interface{}) { return "", nil }
func (w *Max) GetData(ctx context.Context, a Args) (more bool, err error) {
w.Max, err = goatcounter.GetMax(ctx, a.Rng, a.PathFilter, a.Daily)
w.loaded = true
return false, err
}

type TotalCount struct {
goatcounter.TotalCount

Expand Down
10 changes: 7 additions & 3 deletions widgets/pages.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ func (w *Pages) GetData(ctx context.Context, a Args) (bool, error) {
errs.Append(err)

wg.Wait()

for _, p := range w.Pages {
if p.Max > w.Max {
w.Max = p.Max
}
}

w.loaded = true
return w.More, errs.ErrorOrNil()
}
Expand Down Expand Up @@ -143,9 +150,6 @@ func (w Pages) RenderHTML(ctx context.Context, shared SharedData) (string, inter
}
}
}
if w.Max == 0 {
w.Max = 10
}

return t, struct {
Context context.Context
Expand Down
17 changes: 1 addition & 16 deletions widgets/widgets.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,6 @@ func FromSiteWidgets(ctx context.Context, www goatcounter.Widgets, params zint.B
for i, w := range www {
ww := NewWidget(w.Name(), i)
ww.SetSettings(w.GetSettings(ctx))

switch w.Name() {
case "pages":
if !params.Has(FilterInternal) {
widgetList = append(widgetList, NewWidget("max", 0))
}
// XXX
// if params.Has(ShowRefs) {
// r := NewWidget("refs", 0).(*Refs)
// r.Limit = int(w.GetSetting("limit_refs").(float64))
// widgetList = append(widgetList, r)
// }
}
widgetList = append(widgetList, ww)
}

Expand Down Expand Up @@ -129,7 +116,7 @@ func (l List) InitialAndLazy() (initial List, lazy List) {
lazy = make(List, 0, len(l)-3)
for _, w := range l {
switch w.Name() {
case "max", "totalcount":
case "totalcount":
initial = append(initial, w)
default:
if first {
Expand Down Expand Up @@ -162,8 +149,6 @@ func NewWidget(name string, id int) Widget {
switch name {
case "totalcount":
return &TotalCount{}
case "max":
return &Max{}

case "pages":
return &Pages{id: id}
Expand Down

0 comments on commit f1fae72

Please sign in to comment.