Try to LRU cache offsets and symlinks

3 files changed, 55 insertions, 23 deletions

diff --git a/internal/vfs/zip/archive.go b/internal/vfs/zip/archive.go
index 5fc55b0d..158fb87b 100644
--- a/internal/vfs/zip/archive.go
+++ b/internal/vfs/zip/archive.go
@@ -8,8 +8,10 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	log "github.com/sirupsen/logrus"
@@ -24,7 +26,9 @@ const (
 	maxSymlinkSize = 256
 
 	// DefaultOpenTimeout to request an archive and read its contents the first time
-	DefaultOpenTimeout = 30 * time.Second
+	DefaultOpenTimeout      = 30 * time.Second
+	DataOffsetCacheInterval = 60 * time.Second
+	ReadLinkCacheInterval   = 60 * time.Second
 )
 
 var (
@@ -36,11 +40,15 @@ var (
 // It represents a zip archive saving all its files in memory.
 // It holds an httprange.Resource that can be read with httprange.RangedReader in chunks.
 type zipArchive struct {
+	fs *zipVFS
+
 	path        string
 	once        sync.Once
 	done        chan struct{}
 	openTimeout time.Duration
 
+	cacheKey string
+
 	resource *httprange.Resource
 	reader   *httprange.RangedReader
 	archive  *zip.Reader
@@ -50,12 +58,14 @@ type zipArchive struct {
 	files map[string]*zip.File
 }
 
-func newArchive(path string, openTimeout time.Duration) *zipArchive {
+func newArchive(fs *zipVFS, path string, openTimeout time.Duration) *zipArchive {
 	return &zipArchive{
+		fs:          fs,
 		path:        path,
 		done:        make(chan struct{}),
 		files:       make(map[string]*zip.File),
 		openTimeout: openTimeout,
+		cacheKey:    strconv.FormatInt(atomic.AddInt64(&fs.archiveCount, 1), 10) + ":",
 	}
 }
 
@@ -158,12 +168,15 @@ func (a *zipArchive) Open(ctx context.Context, name string) (vfs.File, error) {
 		return nil, os.ErrNotExist
 	}
 
-	// TODO: cache dataOffsets of files https://gitlab.com/gitlab-org/gitlab-pages/-/issues/461
-	dataOffset, err := file.DataOffset()
+	item, err := a.fs.dataOffsetCache.Fetch(a.cacheKey+":"+name, DataOffsetCacheInterval, func() (interface{}, error) {
+		return file.DataOffset()
+	})
 	if err != nil {
 		return nil, err
 	}
 
+	dataOffset := item.Value().(int64)
+
 	// only read from dataOffset up to the size of the compressed file
 	reader := a.reader.SectionReader(ctx, dataOffset, int64(file.CompressedSize64))
 
@@ -198,28 +211,37 @@ func (a *zipArchive) Readlink(ctx context.Context, name string) (string, error)
 		return "", errNotSymlink
 	}
 
-	rc, err := file.Open()
-	if err != nil {
-		return "", err
-	}
-	defer rc.Close()
+	item, err := a.fs.readlinkCache.Fetch(a.cacheKey+":"+name, ReadLinkCacheInterval, func() (interface{}, error) {
+		rc, err := file.Open()
+		if err != nil {
+			return nil, err
+		}
+		defer rc.Close()
+
+		var symlink [maxSymlinkSize + 1]byte
 
-	symlink := make([]byte, maxSymlinkSize+1)
+		// read up to len(symlink) bytes from the link file
+		n, err := io.ReadFull(rc, symlink[:])
+		if err != nil && err != io.ErrUnexpectedEOF {
+			// if err == io.ErrUnexpectedEOF the link is smaller than len(symlink) so it's OK to not return it
+			return nil, err
+		}
 
-	// read up to len(symlink) bytes from the link file
-	n, err := io.ReadFull(rc, symlink)
-	if err != nil && err != io.ErrUnexpectedEOF {
-		// if err == io.ErrUnexpectedEOF the link is smaller than len(symlink) so it's OK to not return it
+		// cache symlink up to desired size
+		return string(symlink[:n]), nil
+	})
+	if err != nil {
 		return "", err
 	}
 
+	symlink := item.Value().(string)
+
 	// return errSymlinkSize if the number of bytes read from the link is too big
-	if n > maxSymlinkSize {
+	if len(symlink) > maxSymlinkSize {
 		return "", errSymlinkSize
 	}
 
-	// only return the n bytes read from the link
-	return string(symlink[:n]), nil
+	return symlink, nil
 }
 
 // onEvicted called by the zipVFS.cache when an archive is removed from the cache
diff --git a/internal/vfs/zip/archive_test.go b/internal/vfs/zip/archive_test.go
index bb094038..d778eefb 100644
--- a/internal/vfs/zip/archive_test.go
+++ b/internal/vfs/zip/archive_test.go
@@ -181,7 +181,8 @@ func TestArchiveCanBeReadAfterOpenCtxCanceled(t *testing.T) {
 	testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip")
 	defer cleanup()
 
-	zip := newArchive(testServerURL+"/public.zip", time.Second)
+	fs := New().(*zipVFS)
+	zip := newArchive(fs, testServerURL+"/public.zip", time.Second)
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
 
@@ -203,7 +204,8 @@ func TestReadArchiveFails(t *testing.T) {
 	testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip")
 	defer cleanup()
 
-	zip := newArchive(testServerURL+"/unkown.html", time.Second)
+	fs := New().(*zipVFS)
+	zip := newArchive(fs, testServerURL+"/unkown.html", time.Second)
 
 	err := zip.openArchive(context.Background())
 	require.Error(t, err)
@@ -218,7 +220,8 @@ func openZipArchive(t *testing.T) (*zipArchive, func()) {
 
 	testServerURL, cleanup := newZipFileServerURL(t, "group/zip.gitlab.io/public.zip")
 
-	zip := newArchive(testServerURL+"/public.zip", time.Second)
+	fs := New().(*zipVFS)
+	zip := newArchive(fs, testServerURL+"/public.zip", time.Second)
 
 	err := zip.openArchive(context.Background())
 	require.NoError(t, err)
diff --git a/internal/vfs/zip/vfs.go b/internal/vfs/zip/vfs.go
index fd0855f7..a99b8771 100644
--- a/internal/vfs/zip/vfs.go
+++ b/internal/vfs/zip/vfs.go
@@ -6,6 +6,7 @@ import (
 	"net/url"
 	"time"
 
+	"github.com/karlseguin/ccache"
 	"github.com/patrickmn/go-cache"
 
 	"gitlab.com/gitlab-org/gitlab-pages/internal/vfs"
@@ -25,14 +26,20 @@ var (
 
 // zipVFS is a simple cached implementation of the vfs.VFS interface
 type zipVFS struct {
-	cache *cache.Cache
+	cache           *cache.Cache
+	dataOffsetCache *ccache.Cache
+	readlinkCache   *ccache.Cache
+
+	archiveCount int64
 }
 
 // New creates a zipVFS instance that can be used by a serving request
 func New() vfs.VFS {
 	zipVFS := &zipVFS{
 		// TODO: add cache operation callbacks https://gitlab.com/gitlab-org/gitlab-pages/-/issues/465
-		cache: cache.New(defaultCacheExpirationInterval, defaultCacheCleanupInterval),
+		cache:           cache.New(defaultCacheExpirationInterval, defaultCacheCleanupInterval),
+		dataOffsetCache: ccache.New(ccache.Configure().MaxSize(10000).ItemsToPrune(2000)),
+		readlinkCache:   ccache.New(ccache.Configure().MaxSize(1000).ItemsToPrune(2000)),
 	}
 
 	zipVFS.cache.OnEvicted(func(s string, i interface{}) {
@@ -86,7 +93,7 @@ func (fs *zipVFS) findOrOpenArchive(ctx context.Context, path string) (*zipArchi
 			fs.cache.SetDefault(path, archive)
 		}
 	} else {
-		archive = newArchive(path, DefaultOpenTimeout)
+		archive = newArchive(fs, path, DefaultOpenTimeout)
 
 		// if adding the archive to the cache fails it means it's already been added before
 		// this is done to find concurrent additions.