refactor: Optimizing list view with multi aggregate

Signed-off-by: Vincent Boutour <[email protected]>
ViBiOh · Sep 3, 2022 · 36a5565 · 36a5565
1 parent 71c26e0
commit 36a5565
Show file tree

Hide file tree

Showing 8 changed files with 106 additions and 37 deletions.
diff --git a/go.mod b/go.mod
@@ -3,12 +3,12 @@ module github.com/ViBiOh/fibr
 go 1.19
 
 require (
-	github.com/ViBiOh/ChatPotte v0.2.8
+	github.com/ViBiOh/ChatPotte v0.2.9
 	github.com/ViBiOh/absto v1.0.3
-	github.com/ViBiOh/auth/v2 v2.14.5
+	github.com/ViBiOh/auth/v2 v2.14.6
 	github.com/ViBiOh/exas v0.5.0
 	github.com/ViBiOh/flags v1.2.0
-	github.com/ViBiOh/httputils/v4 v4.47.3
+	github.com/ViBiOh/httputils/v4 v4.47.4
 	github.com/ViBiOh/vith v0.5.0
 	github.com/golang/mock v1.6.0
 	github.com/prometheus/client_golang v1.13.0

diff --git a/go.sum b/go.sum
@@ -33,18 +33,18 @@ cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9
 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
-github.com/ViBiOh/ChatPotte v0.2.8 h1:IJRUj6phgPfB663/+3Dbh12iNMo+O2cUpI854tLFVLQ=
-github.com/ViBiOh/ChatPotte v0.2.8/go.mod h1:RtTdEAQjXtKoiH0AFGCcamLYaBHhzNtOO88H6LnSz2U=
+github.com/ViBiOh/ChatPotte v0.2.9 h1:RbCiNCAYg8udO+GO7Ap0YVfyXKolZ393P4BHE+cRvhc=
+github.com/ViBiOh/ChatPotte v0.2.9/go.mod h1:SOJMFIJf5rsLWzVeaavZzVU2ml3XmrNDbTxNEIr31uE=
 github.com/ViBiOh/absto v1.0.3 h1:guwVdRN7mIJM6XO65ABqDjz/7JiFMGJ7+VfmE/Ni3Zw=
 github.com/ViBiOh/absto v1.0.3/go.mod h1:nu8N6LQnyxYhdnTAOtlpr4zca2tOcj5uUgI30oCF4vo=
-github.com/ViBiOh/auth/v2 v2.14.5 h1:aHTotDNS4Vdq8JrJfbMdQQSC1COEv1Bm47zq5k81rpk=
-github.com/ViBiOh/auth/v2 v2.14.5/go.mod h1:+RI7zvIPIT0dLycvvkqdlNQRhn9aqFMv5GqjafSG31M=
+github.com/ViBiOh/auth/v2 v2.14.6 h1:zDhOzwjurrUAsNElh4VXc5MnYKcENoAu25E5eQiw1j0=
+github.com/ViBiOh/auth/v2 v2.14.6/go.mod h1:GcCYmx8SzH7LhnmeB9x3bGWRa5g4uAY4GszxNKwFJjE=
 github.com/ViBiOh/exas v0.5.0 h1:ZL5v5rUOkkwo6PxhbOVe96L0RUre7SfStaVcqhE/IbU=
 github.com/ViBiOh/exas v0.5.0/go.mod h1:sKRV/vz85Vj0caT+a3XWVFp0gyzXe36eZWL+XKge93E=
 github.com/ViBiOh/flags v1.2.0 h1:DaujjNXzD29KxKyp4eZdn7c9+uBN5DokWgDAe7DcUmc=
 github.com/ViBiOh/flags v1.2.0/go.mod h1:UyMB5zeD/aId7Xw3x7577ZNU298JmukzOcV8p/H2W1s=
-github.com/ViBiOh/httputils/v4 v4.47.3 h1:3J+Mfa+gptmzkYZZ+BW3Ra9C+Mdqol/I6v/Df7vWrpY=
-github.com/ViBiOh/httputils/v4 v4.47.3/go.mod h1:ghw8fNnYr+1450CNeTyaDreaMDC7plFutd54ttT3EMs=
+github.com/ViBiOh/httputils/v4 v4.47.4 h1:W0e+aciohDP+qaJk7JDo1OtGn7Lf+W0DPgu9S5H+vSQ=
+github.com/ViBiOh/httputils/v4 v4.47.4/go.mod h1:ghw8fNnYr+1450CNeTyaDreaMDC7plFutd54ttT3EMs=
 github.com/ViBiOh/vith v0.5.0 h1:BpmjwWxd4NGmUJF12XpFOngK32539Lw3mvbMOaeOmhE=
 github.com/ViBiOh/vith v0.5.0/go.mod h1:Wf40eedOBEcxeLzGzGpuO+2G8qe5Ov1WIIBPXohyKco=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=

diff --git a/pkg/crud/list.go b/pkg/crud/list.go
@@ -24,44 +24,55 @@ func (a App) list(ctx context.Context, request provider.Request, message rendere
 	ctx, end := tracer.StartSpan(ctx, a.tracer, "list", trace.WithAttributes(attribute.String("item", item.Pathname)))
 	defer end()
 
-	directoryAggregate, err := a.exifApp.GetAggregateFor(ctx, item)
-	if err != nil && !absto.IsNotExist(err) {
-		logger.WithField("fn", "crud.List").WithField("item", request.Path).Error("get aggregate: %s", err)
-	}
+	wg := concurrent.NewSimple()
 
-	items := make([]provider.RenderItem, len(files))
-	wg := concurrent.NewLimited(provider.MaxConcurrency)
+	var directoryAggregate provider.Aggregate
+	wg.Go(func() {
+		var err error
 
-	var thumbnails map[string]absto.Item
+		directoryAggregate, err = a.exifApp.GetAggregateFor(ctx, item)
+		if err != nil && !absto.IsNotExist(err) {
+			logger.WithField("fn", "crud.list").WithField("item", item.Pathname).Error("get aggregate: %s", err)
+		}
+	})
+
+	var aggregates map[string]provider.Aggregate
 	wg.Go(func() {
 		var err error
+
+		aggregates, err = a.exifApp.ListAggregateFor(ctx, files...)
+		if err != nil {
+			logger.WithField("fn", "crud.list").WithField("item", item.Pathname).Error("list exifs: %s", err)
+		}
+	})
+
+	var thumbnails map[string]absto.Item
+	thumbnailDone := make(chan struct{})
+	go func() {
+		defer close(thumbnailDone)
+
+		var err error
+
 		thumbnails, err = a.thumbnailApp.ListDir(ctx, item)
 		if err != nil {
-			logger.WithField("item", item.Pathname).Error("list thumbnail: %s", err)
+			logger.WithField("fn", "crud.list").WithField("item", item.Pathname).Error("list thumbnail: %s", err)
 			return
 		}
-	})
+	}()
 
-	for index, item := range files {
-		index := index
-		item := item
+	wg.Wait()
 
-		wg.Go(func() {
-			aggregate, err := a.exifApp.GetAggregateFor(ctx, item)
-			if err != nil {
-				logger.WithField("fn", "crud.List").WithField("item", item.Pathname).Error("read: %s", err)
-			}
+	items := make([]provider.RenderItem, len(files))
 
-			renderItem := provider.StorageToRender(item, request)
-			renderItem.Aggregate = aggregate
-			renderItem.IsCover = item.Name == directoryAggregate.Cover
+	for index, item := range files {
+		renderItem := provider.StorageToRender(item, request)
+		renderItem.Aggregate = aggregates[item.ID]
+		renderItem.IsCover = item.Name == directoryAggregate.Cover
 
-			items[index] = renderItem
-		})
+		items[index] = renderItem
 	}
 
-	wg.Wait()
-
+	<-thumbnailDone
 	hasThumbnail, hasStory, cover := a.enrichThumbnail(ctx, directoryAggregate, items, thumbnails)
 
 	content := map[string]any{

diff --git a/pkg/crud/story.go b/pkg/crud/story.go
@@ -26,8 +26,6 @@ func (a App) story(r *http.Request, request provider.Request, item absto.Item, f
 	wg := concurrent.NewSimple()
 
 	var directoryAggregate provider.Aggregate
-	var exifs map[string]exas.Exif
-
 	wg.Go(func() {
 		var err error
 
@@ -37,6 +35,7 @@ func (a App) story(r *http.Request, request provider.Request, item absto.Item, f
 		}
 	})
 
+	var exifs map[string]exas.Exif
 	wg.Go(func() {
 		var err error
 

diff --git a/pkg/exif/aggregate.go b/pkg/exif/aggregate.go
@@ -66,6 +66,27 @@ func (a App) GetAggregateFor(ctx context.Context, item absto.Item) (provider.Agg
 	return a.aggregateCacheApp.Get(ctx, item)
 }
 
+func (a App) ListAggregateFor(ctx context.Context, items ...absto.Item) (map[string]provider.Aggregate, error) {
+	ctx, end := tracer.StartSpan(ctx, a.tracer, "list_aggregate")
+	defer end()
+
+	exifs, err := a.aggregateCacheApp.List(ctx, items...)
+	if err != nil {
+		return nil, err
+	}
+
+	output := make(map[string]provider.Aggregate, len(items))
+	exifsLen := len(exifs)
+
+	for index, item := range items {
+		if index < exifsLen {
+			output[item.ID] = exifs[index]
+		}
+	}
+
+	return output, nil
+}
+
 func (a App) SaveExifFor(ctx context.Context, item absto.Item, exif exas.Exif) error {
 	return a.exifCacheApp.EvictOnSuccess(ctx, item, a.saveMetadata(ctx, item, exif))
 }
@@ -107,7 +128,8 @@ func (a App) computeAndSaveAggregate(ctx context.Context, dir absto.Item) error
 			if absto.IsNotExist(err) {
 				return nil
 			}
-			return fmt.Errorf("unable load exif data: %w", err)
+
+			return fmt.Errorf("load exif data: %w", err)
 		}
 
 		if !exifData.Date.IsZero() {

diff --git a/pkg/exif/exif.go b/pkg/exif/exif.go
@@ -102,14 +102,30 @@ func New(config Config, storageApp absto.Storage, prometheusRegisterer prometheu
 	}
 
 	app.exifCacheApp = cache.New(redisClient, redisKey, func(ctx context.Context, item absto.Item) (exas.Exif, error) {
+		if item.IsDir {
+			return exas.Exif{}, cache.ErrIgnore
+		}
+
 		value, err := app.loadExif(ctx, item)
 		if absto.IsNotExist(err) {
 			return value, cache.ErrIgnore
 		}
 
 		return value, err
 	}, cacheDuration, provider.MaxConcurrency, tracerApp.GetTracer("exif_cache"))
-	app.aggregateCacheApp = cache.New(redisClient, redisKey, app.loadAggregate, cacheDuration, provider.MaxConcurrency, tracerApp.GetTracer("ggregate_cache"))
+
+	app.aggregateCacheApp = cache.New(redisClient, redisKey, func(ctx context.Context, item absto.Item) (provider.Aggregate, error) {
+		if !item.IsDir {
+			return provider.Aggregate{}, cache.ErrIgnore
+		}
+
+		value, err := app.loadAggregate(ctx, item)
+		if absto.IsNotExist(err) {
+			return value, cache.ErrIgnore
+		}
+
+		return value, err
+	}, cacheDuration, provider.MaxConcurrency, tracerApp.GetTracer("ggregate_cache"))
 
 	return app, nil
 }

diff --git a/pkg/mocks/interfaces.go b/pkg/mocks/interfaces.go
diff --git a/pkg/provider/interfaces.go b/pkg/provider/interfaces.go
@@ -44,6 +44,7 @@ type WebhookManager interface {
 type ExifManager interface {
 	ListDir(ctx context.Context, item absto.Item) ([]absto.Item, error)
 	GetAggregateFor(ctx context.Context, item absto.Item) (Aggregate, error)
+	ListAggregateFor(ctx context.Context, items ...absto.Item) (map[string]Aggregate, error)
 	SaveAggregateFor(ctx context.Context, item absto.Item, aggregate Aggregate) error
 	GetExifFor(ctx context.Context, item absto.Item) (exas.Exif, error)
 	ListExifFor(ctx context.Context, items ...absto.Item) (map[string]exas.Exif, error)