diff --git a/modules/caddyhttp/fileserver/browse.go b/modules/caddyhttp/fileserver/browse.go index 3b97f2ff3..97f1fc128 100644 --- a/modules/caddyhttp/fileserver/browse.go +++ b/modules/caddyhttp/fileserver/browse.go @@ -333,20 +333,6 @@ func (fsrv *FileServer) makeBrowseTemplate(tplCtx *templateContext) (*template.T return tpl, nil } -// isSymlinkTargetDir returns true if f's symbolic link target -// is a directory. -func (fsrv *FileServer) isSymlinkTargetDir(fileSystem fs.FS, f fs.FileInfo, root, urlPath string) bool { - if !isSymlink(f) { - return false - } - target := caddyhttp.SanitizedPathJoin(root, path.Join(urlPath, f.Name())) - targetInfo, err := fs.Stat(fileSystem, target) - if err != nil { - return false - } - return targetInfo.IsDir() -} - // isSymlink return true if f is a symbolic link. func isSymlink(f fs.FileInfo) bool { return f.Mode()&os.ModeSymlink != 0 diff --git a/modules/caddyhttp/fileserver/browsetplcontext.go b/modules/caddyhttp/fileserver/browsetplcontext.go index fee5edd4f..112d8caec 100644 --- a/modules/caddyhttp/fileserver/browsetplcontext.go +++ b/modules/caddyhttp/fileserver/browsetplcontext.go @@ -44,6 +44,9 @@ func (fsrv *FileServer) directoryListing(ctx context.Context, fileSystem fs.FS, Path: urlPath, CanGoUp: canGoUp, lastModified: parentModTime, + // preallocate, since we know the upper bound on the number of + // items; avoids repeated slice growth/copy for large directories + Items: make([]fileInfo, 0, len(entries)), } for _, entry := range entries { @@ -71,7 +74,23 @@ func (fsrv *FileServer) directoryListing(ctx context.Context, fileSystem fs.FS, tplCtx.lastModified = modTime } - isDir := entry.IsDir() || fsrv.isSymlinkTargetDir(fileSystem, info, root, urlPath) + fileIsSymlink := isSymlink(info) + size := info.Size() + + // for a symlink, a single stat of its target tells us both whether + // the target is a directory and what its size is + var targetInfo fs.FileInfo + var targetPath string + if fileIsSymlink { + targetPath = caddyhttp.SanitizedPathJoin(root, path.Join(urlPath, info.Name())) + // An error most likely means the symlink target doesn't exist, + // which isn't entirely unusual and shouldn't fail the listing. + // In this case, targetInfo stays nil and we fall back to + // treating it as a non-directory, using the symlink's own size. + targetInfo, _ = fs.Stat(fileSystem, targetPath) + } + + isDir := entry.IsDir() || (targetInfo != nil && targetInfo.IsDir()) // add the slash after the escape of path to avoid escaping the slash as well if isDir { @@ -81,34 +100,24 @@ func (fsrv *FileServer) directoryListing(ctx context.Context, fileSystem fs.FS, tplCtx.NumFiles++ } - size := info.Size() - if !isDir { // increase the total by the symlink's size, not the target's size, // by incrementing before we follow the symlink tplCtx.TotalFileSize += size } - fileIsSymlink := isSymlink(info) symlinkPath := "" if fileIsSymlink { - path := caddyhttp.SanitizedPathJoin(root, path.Join(urlPath, info.Name())) - fileInfo, err := fs.Stat(fileSystem, path) - if err == nil { - size = fileInfo.Size() + if targetInfo != nil { + size = targetInfo.Size() } if fsrv.Browse.RevealSymlinks { - symLinkTarget, err := os.Readlink(path) + symLinkTarget, err := os.Readlink(targetPath) if err == nil { symlinkPath = symLinkTarget } } - - // An error most likely means the symlink target doesn't exist, - // which isn't entirely unusual and shouldn't fail the listing. - // In this case, just use the size of the symlink itself, which - // was already set above. } if !isDir { @@ -220,6 +229,14 @@ func (l *browseTemplateContext) applySortAndLimit(sortParam, orderParam, limitPa l.Sort = sortParam l.Order = orderParam + // Only compute lowercase names when the selected sort comparator needs it; + // avoid O(n) work when sorting by time or when no sorting is requested. + if l.Sort == sortByName || l.Sort == sortByNameDirFirst || l.Sort == sortBySize { + for i := range l.Items { + l.Items[i].nameLower = strings.ToLower(l.Items[i].Name) + } + } + if l.Order == "desc" { switch l.Sort { case sortByName: @@ -282,6 +299,11 @@ type fileInfo struct { // a pointer to the template context is useful inside nested templates Tpl *browseTemplateContext `json:"-"` + + // cached lowercase Name, precomputed once by applySortAndLimit so that + // byName/byNameDirFirst/bySize don't each recompute it on every one of + // the O(n log n) comparisons sort.Sort makes; not serialized. + nameLower string } // HasExt returns true if the filename has any of the given suffixes, case-insensitive. @@ -328,7 +350,7 @@ func (l byName) Len() int { return len(l.Items) } func (l byName) Swap(i, j int) { l.Items[i], l.Items[j] = l.Items[j], l.Items[i] } func (l byName) Less(i, j int) bool { - return strings.ToLower(l.Items[i].Name) < strings.ToLower(l.Items[j].Name) + return l.Items[i].nameLower < l.Items[j].nameLower } func (l byNameDirFirst) Len() int { return len(l.Items) } @@ -337,7 +359,7 @@ func (l byNameDirFirst) Swap(i, j int) { l.Items[i], l.Items[j] = l.Items[j], l. func (l byNameDirFirst) Less(i, j int) bool { // sort by name if both are dir or file if l.Items[i].IsDir == l.Items[j].IsDir { - return strings.ToLower(l.Items[i].Name) < strings.ToLower(l.Items[j].Name) + return l.Items[i].nameLower < l.Items[j].nameLower } // sort dir ahead of file return l.Items[i].IsDir @@ -347,7 +369,7 @@ func (l bySize) Len() int { return len(l.Items) } func (l bySize) Swap(i, j int) { l.Items[i], l.Items[j] = l.Items[j], l.Items[i] } func (l bySize) Less(i, j int) bool { - const directoryOffset = -1 << 31 // = -math.MinInt32 + const directoryOffset = -1 << 31 // = -2147483648 (min int32) iSize, jSize := l.Items[i].Size, l.Items[j].Size @@ -361,7 +383,7 @@ func (l bySize) Less(i, j int) bool { jSize = directoryOffset } if l.Items[i].IsDir && l.Items[j].IsDir { - return strings.ToLower(l.Items[i].Name) < strings.ToLower(l.Items[j].Name) + return l.Items[i].nameLower < l.Items[j].nameLower } return iSize < jSize diff --git a/modules/caddyhttp/fileserver/browsetplcontext_bench_test.go b/modules/caddyhttp/fileserver/browsetplcontext_bench_test.go new file mode 100644 index 000000000..a895264bc --- /dev/null +++ b/modules/caddyhttp/fileserver/browsetplcontext_bench_test.go @@ -0,0 +1,224 @@ +// Copyright 2015 Matthew Holt and The Caddy Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package fileserver + +import ( + "context" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + "testing" + "time" + + "go.uber.org/zap" + + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/caddy/v2/internal/filesystems" +) + +// makeBenchDir populates dir with n regular files and a handful of +// symlinks (both to files and to a subdirectory), mirroring what a +// real, large, browsable directory tends to contain. +func makeBenchDir(tb testing.TB, dir string, n int) { + tb.Helper() + + subdir := filepath.Join(dir, "subdir") + if err := os.Mkdir(subdir, 0o755); err != nil { + tb.Fatal(err) + } + + for i := range n { + name := filepath.Join(dir, fmt.Sprintf("file-%d.txt", i)) + if err := os.WriteFile(name, []byte("x"), 0o600); err != nil { + tb.Fatal(err) + } + } + + target := filepath.Join(dir, "file-0.txt") + if err := os.Symlink(target, filepath.Join(dir, "symlink-to-file")); err != nil { + tb.Skipf("symlink not supported on this platform: %v", err) + } + if err := os.Symlink(subdir, filepath.Join(dir, "symlink-to-dir")); err != nil { + tb.Skipf("symlink not supported on this platform: %v", err) + } +} + +// readBenchDirEntries reads all entries of dir once, up front, so that +// benchmark iterations measure only the cost of directoryListing itself +// (allocations, stat calls per entry) rather than the readdir syscall. +// +// It deliberately opens the directory and calls the ReadDir method on the +// resulting file (like loadDirectoryContents does in production), not +// fs.ReadDir/os.ReadDir - the latter sort entries by filename before +// returning them, which would silently hand the sort benchmarks +// already-sorted input and let sort.Sort's adaptive algorithm breeze +// through with far fewer comparisons than a real, filesystem-ordered +// directory listing requires. +func readBenchDirEntries(tb testing.TB, dir string) (fs.FS, []fs.DirEntry) { + tb.Helper() + + fileSystem := filesystems.OsFS{} + f, err := fileSystem.Open(dir) + if err != nil { + tb.Fatal(err) + } + defer f.Close() + + entries, err := f.(fs.ReadDirFile).ReadDir(-1) + if err != nil { + tb.Fatal(err) + } + return fileSystem, entries +} + +func benchFileServer() *FileServer { + return &FileServer{ + Browse: &Browse{}, + logger: zap.NewNop(), + } +} + +func benchmarkDirectoryListing(b *testing.B, n int, listFn func(fsrv *FileServer, ctx context.Context, fileSystem fs.FS, entries []fs.DirEntry, root string) *browseTemplateContext) { + dir := b.TempDir() + makeBenchDir(b, dir, n) + fileSystem, entries := readBenchDirEntries(b, dir) + fsrv := benchFileServer() + ctx := context.Background() + + b.ReportAllocs() + + for b.Loop() { + listFn(fsrv, ctx, fileSystem, entries, dir) + } +} + +func BenchmarkDirectoryListing(b *testing.B) { + for _, n := range []int{100, 1_000, 10_000, 50_000} { + b.Run(fmt.Sprintf("entries=%d", n), func(b *testing.B) { + benchmarkDirectoryListing(b, n, func(fsrv *FileServer, ctx context.Context, fileSystem fs.FS, entries []fs.DirEntry, root string) *browseTemplateContext { + return fsrv.directoryListing(ctx, fileSystem, time.Time{}, entries, true, root, "/", caddy.NewReplacer()) + }) + }) + } +} + +// makeBenchItems builds n synthetic fileInfo items (no disk I/O), mixing +// upper/lower case names and marking roughly 1 in 10 as directories, to +// exercise the sort comparators - including bySize's directory name +// tie-break - the way a real, large, mixed listing would. +func makeBenchItems(n int) []fileInfo { + items := make([]fileInfo, n) + for i := range n { + name := fmt.Sprintf("file_%d.txt", i) + if i%2 == 0 { + name = fmt.Sprintf("FILE_%d.TXT", i) + } + items[i] = fileInfo{ + Name: name, + Size: int64(i), + IsDir: i%10 == 0, + } + } + return items +} + +func fillNameLower(items []fileInfo) { + for i := range items { + items[i].nameLower = strings.ToLower(items[i].Name) + } +} + +// benchmarkSort times sortFn against a fresh, unsorted copy of a synthetic +// item set on every iteration. The copy, and the optional prepare step +// (e.g. filling the nameLower cache), happen outside the timed portion so +// the benchmark isolates the cost of the sort itself. +func benchmarkSort(b *testing.B, n int, prepare func(items []fileInfo), sortFn func(items []fileInfo)) { + pristine := makeBenchItems(n) + items := make([]fileInfo, n) + + for b.Loop() { + b.StopTimer() + copy(items, pristine) + if prepare != nil { + prepare(items) + } + b.StartTimer() + sortFn(items) + } +} + +func BenchmarkSortByName(b *testing.B) { + for _, n := range []int{100, 1_000, 10_000, 50_000} { + b.Run(fmt.Sprintf("entries=%d", n), func(b *testing.B) { + benchmarkSort(b, n, fillNameLower, func(items []fileInfo) { + sort.Sort(byName(browseTemplateContext{Items: items})) + }) + }) + } +} + +func BenchmarkSortByNameDirFirst(b *testing.B) { + for _, n := range []int{100, 1_000, 10_000, 50_000} { + b.Run(fmt.Sprintf("entries=%d", n), func(b *testing.B) { + benchmarkSort(b, n, fillNameLower, func(items []fileInfo) { + sort.Sort(byNameDirFirst(browseTemplateContext{Items: items})) + }) + }) + } +} + +func BenchmarkSortBySize(b *testing.B) { + for _, n := range []int{100, 1_000, 10_000, 50_000} { + b.Run(fmt.Sprintf("entries=%d", n), func(b *testing.B) { + benchmarkSort(b, n, fillNameLower, func(items []fileInfo) { + sort.Sort(bySize(browseTemplateContext{Items: items})) + }) + }) + } +} + +// --------------------------------------------------------------------- +// Overall benchmark: the whole real-world browse-request path - reading +// directory entries into a listing and then sorting it. +// --------------------------------------------------------------------- + +func benchmarkDirectoryListingAndSort(b *testing.B, n int, run func(fsrv *FileServer, ctx context.Context, fileSystem fs.FS, entries []fs.DirEntry, root string) *browseTemplateContext) { + dir := b.TempDir() + makeBenchDir(b, dir, n) + fileSystem, entries := readBenchDirEntries(b, dir) + fsrv := benchFileServer() + ctx := context.Background() + + b.ReportAllocs() + + for b.Loop() { + run(fsrv, ctx, fileSystem, entries, dir) + } +} + +func BenchmarkDirectoryListingAndSort(b *testing.B) { + for _, n := range []int{100, 1_000, 10_000, 50_000} { + b.Run(fmt.Sprintf("entries=%d", n), func(b *testing.B) { + benchmarkDirectoryListingAndSort(b, n, func(fsrv *FileServer, ctx context.Context, fileSystem fs.FS, entries []fs.DirEntry, root string) *browseTemplateContext { + listing := fsrv.directoryListing(ctx, fileSystem, time.Time{}, entries, true, root, "/", caddy.NewReplacer()) + listing.applySortAndLimit(sortByNameDirFirst, sortOrderAsc, "", "") + return listing + }) + }) + } +} diff --git a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go index 84fd4493c..15e02db5a 100644 --- a/modules/caddyhttp/reverseproxy/selectionpolicies_test.go +++ b/modules/caddyhttp/reverseproxy/selectionpolicies_test.go @@ -133,36 +133,36 @@ func TestWeightedRoundRobinPolicy(t *testing.T) { func TestWeightedRoundRobinSelection_Validate(t *testing.T) { tests := []struct { - name string - weights []int - wantErr bool + name string + weights []int + wantErr bool }{ { - name: "Valid 0 2 1 case", - weights: []int{0, 2, 1}, - wantErr: false, + name: "Valid 0 2 1 case", + weights: []int{0, 2, 1}, + wantErr: false, }, { - name: "Invalid 0 case (single)", - weights: []int{0}, - wantErr: true, + name: "Invalid 0 case (single)", + weights: []int{0}, + wantErr: true, }, { - name: "Invalid 0 0 case (multiple)", - weights: []int{0, 0}, - wantErr: true, + name: "Invalid 0 0 case (multiple)", + weights: []int{0, 0}, + wantErr: true, }, { - name: "Valid weights", - weights: []int{1, 1, 1}, - wantErr: false, + name: "Valid weights", + weights: []int{1, 1, 1}, + wantErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := &WeightedRoundRobinSelection{ - Weights: tt.weights, + Weights: tt.weights, } _ = s.Provision(caddy.Context{}) err := s.Validate() diff --git a/modules/caddyhttp/templates/tplcontext_test.go b/modules/caddyhttp/templates/tplcontext_test.go index 1ff6caef0..a29225521 100644 --- a/modules/caddyhttp/templates/tplcontext_test.go +++ b/modules/caddyhttp/templates/tplcontext_test.go @@ -424,9 +424,9 @@ func TestStripHTML(t *testing.T) { expect: ``, }, { - // false start — second '<' increments depth, single '>' only closes one level - input: `