Skip to content

perf: add fast path for simple recursive globs#2884

Open
Napolitain wants to merge 3 commits into
go-task:mainfrom
Napolitain:issue-2853-simple-glob-fast-path
Open

perf: add fast path for simple recursive globs#2884
Napolitain wants to merge 3 commits into
go-task:mainfrom
Napolitain:issue-2853-simple-glob-fast-path

Conversation

@Napolitain

@Napolitain Napolitain commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

This is a stacked optimization PR on top of the filesystem benchmark work in #2881. Please review or merge #2881 first; this branch intentionally depends on those benchmarks so the performance impact can be evaluated with the same many-small and few-large fixtures.

This is independent from #2883.

This PR optimizes expansion for simple recursive source globs like path/to/folder/**/*.yaml.

The fast path only handles narrow literal-root recursive globs. More complex shell-style patterns continue to use the existing expander, and symlinked directories fall back to the existing path to preserve current behavior.

BenchmarkManySmallFiles/checksum-28                    3         389359224 ns/op           0.26 MB/s             0.09537 source_MiB/op     20000 source_files/op      2004835176 B/op   491402 allocs/op
BenchmarkManySmallFiles/timestamp-28                   3          83254561 ns/op           1.20 MB/s             0.09537 source_MiB/op     20000 source_files/op      42646077 B/op     311394 allocs/op
BenchmarkManySmallFiles/native-mtime-28                3          18549550 ns/op           5.39 MB/s             0.09537 source_MiB/op     20000 source_files/op      11558392 B/op     123036 allocs/op
BenchmarkManySmallFiles/none-28                        3            744356 ns/op         2598650 B/op       3193 allocs/op

BenchmarkFewLargeFiles/checksum-28                     3          60481191 ns/op        8876.66 MB/s           512.0 source_MiB/op            4.000 source_files/op    1064938 B/op       1551 allocs/op
BenchmarkFewLargeFiles/timestamp-28                    3            113883 ns/op        4714246.05 MB/s        512.0 source_MiB/op            4.000 source_files/op     262730 B/op       1525 allocs/op
BenchmarkFewLargeFiles/native-mtime-28                 3             11552 ns/op        46475623.60 MB/s               512.0 source_MiB/op             4.000 source_files/op      4701 B/op         44 allocs/op
BenchmarkFewLargeFiles/none-28                         3           1048155 ns/op         2573429 B/op       3185 allocs/op

Comment thread internal/fingerprint/glob.go Outdated
return nil, false, nil
}

results := make(map[string]bool)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly use a slice rather than map.

var results []string
....
matched, _ := filepath.Match(namePattern, d.Name()) // d.Name() is faster than filepath.Base(path)
if !matched {
return nil
}
results = append(results, path)
return nil
...
return results, true, nil

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done !

@trulede

trulede commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

About twice as fast, right? Or do I read the results incorrectly.

@trulede

trulede commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

I wondered why Make is so fast ... and after some lazy prompting ... got some kind of idea.

Basically, cache the globing on the first run, and then do os.Stat calls against that cache for subsequent calls. Could be an idea?

package main

import (
	"encoding/json"
	"errors"
	"os"
)

// FileIndex stores the absolute path and the last known modification time
type FileIndex map[string]int64

// LoadIndex reads the cached file metadata from disk
func LoadIndex(cachePath string) (FileIndex, error) {
	data, err := os.ReadFile(cachePath)
	if errors.Is(err, os.ErrNotExist) {
		return make(FileIndex), nil
	}
	if err != nil {
		return nil, err
	}

	var index FileIndex
	if err := json.Unmarshal(data, &index); err != nil {
		return nil, err
	}
	return index, nil
}

// SaveIndex persists the updated file metadata to disk
func SaveIndex(cachePath string, index FileIndex) error {
	data, err := json.MarshalIndent(index, "", "  ")
	if err != nil {
		return err
	}
	return os.WriteFile(cachePath, data, 0644)
}

// CheckChanges fast-scans the explicit list, leveraging the OS VFS cache
func CheckChanges(index FileIndex) (changed []string, missing []string) {
	for path, cachedMtime := range index {
		info, err := os.Stat(path)
		if err != nil {
			if errors.Is(err, os.ErrNotExist) {
				missing = append(missing, path)
			}
			continue
		}

		// Direct timestamp verification
		if info.ModTime().UnixNano() != cachedMtime {
			changed = append(changed, path)
		}
	}
	return changed, missing
}

@andreynering andreynering left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Napolitain

I added a comment below. Can you take a look and rebase with main?

Comment thread internal/fingerprint/glob.go Outdated
return collectKeys(results), nil
}

func fastRecursiveGlob(pattern string) ([]string, bool, error) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a package dedicated to filesystem functions: internal/fsext.

I propose that we move this function there as a new internal/fsext/glob.go file.

If easy enough, a test for this function would be nice as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved the fast recursive glob implementation to internal/fsext/glob.go and added tests.

@andreynering andreynering mentioned this pull request Jul 11, 2026
@Napolitain Napolitain force-pushed the issue-2853-simple-glob-fast-path branch from a61a3b9 to d2b399a Compare July 12, 2026 01:20
Recognize literal-root recursive patterns such as path/to/folder/**/*.yaml and expand them with filepath.WalkDir plus filepath.Match. Keep complex shell patterns and symlinked directories on the existing mvdan shell expander while preserving post-expansion gitignore filtering.

Verification: go test ./...; golangci-lint run
Move simple recursive glob expansion into the shared internal filesystem package and keep fingerprinting as its caller. Add focused tests for direct and nested matches, unsupported patterns, non-directory roots, and symlinked-directory fallback.
Accumulate WalkDir matches directly in a slice and compare the filename pattern against DirEntry.Name. WalkDir yields each path once, so the fast path can remove its deduplication map and sort the normalized result slice before returning.
@Napolitain Napolitain force-pushed the issue-2853-simple-glob-fast-path branch from d2b399a to a0d07ac Compare July 12, 2026 02:19
@Napolitain Napolitain marked this pull request as ready for review July 12, 2026 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants