Skip to content

Commit

Permalink
refactor: replace filepath.Walk with filepath.WalkDir
Browse files Browse the repository at this point in the history
filepath.WalkDir, introduced in Go 1.16, is more performant as it avoids creating
unnecessary intermediate os.FileInfo objects. While filepath.Walk calls os.Lstat for every file or directory
to retrieve os.FileInfo, filepath.WalkDir provides a fs.DirEntry, which includes file
type information without requiring a stat call.

This change reduces unnecessary system calls and aligns with modern Go practices
for directory traversal.

Epic: None

Release note (performance improvement): Improved directory traversal performance by switching from filepath.Walk to filepath.WalkDir.

fix: lint error fixed with crlfmt
  • Loading branch information
Gofastasf committed Jan 16, 2025
1 parent 31e84cb commit 39739fd
Show file tree
Hide file tree
Showing 36 changed files with 1,779 additions and 5,204 deletions.
4 changes: 3 additions & 1 deletion build/bazelutil/whereis/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import (

// whereis is a helper executable that is basically just `realpath`. It's meant
// to be used like:
// bazel run ... --run_under //build/bazelutil/whereis
//
// bazel run ... --run_under //build/bazelutil/whereis
//
// ... which will print the location of the binary you're running. Useful
// because Bazel can be a little unclear about where exactly to find any given
// executable.
Expand Down
3 changes: 2 additions & 1 deletion pkg/acceptance/util_docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"context"
"crypto/rand"
"fmt"
"io/fs"
"math/big"
"os"
"path/filepath"
Expand Down Expand Up @@ -170,7 +171,7 @@ func testDocker(
// so the files can be used inside a docker container. The caller function is responsible for cleaning up.
// This function doesn't copy the original file permissions and uses 755 for directories and files.
func copyRunfiles(source, destination string) error {
return filepath.WalkDir(source, func(path string, dirEntry os.DirEntry, walkErr error) error {
return filepath.WalkDir(source, func(path string, dirEntry fs.DirEntry, walkErr error) error {
if walkErr != nil {
return walkErr
}
Expand Down
9 changes: 5 additions & 4 deletions pkg/backup/full_cluster_backup_restore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"context"
gosql "database/sql"
"fmt"
"io/fs"
"os"
"path/filepath"
"strconv"
Expand Down Expand Up @@ -667,14 +668,14 @@ func TestClusterRestoreFailCleanup(t *testing.T) {

// Bugger the backup by removing the SST files. (Note this messes up all of
// the backups, but there is only one at this point.)
if err := filepath.Walk(tempDir, func(path string, info os.FileInfo, err error) error {
if err := filepath.WalkDir(tempDir, func(path string, d fs.DirEntry, err error) error {
if err != nil {
t.Fatal(err)
}
if info.Name() == backupbase.BackupManifestName ||
if d.Name() == backupbase.BackupManifestName ||
!strings.HasSuffix(path, ".sst") ||
info.Name() == backupinfo.BackupMetadataDescriptorsListPath ||
info.Name() == backupinfo.BackupMetadataFilesListPath {
d.Name() == backupinfo.BackupMetadataDescriptorsListPath ||
d.Name() == backupinfo.BackupMetadataFilesListPath {
return nil
}
return os.Remove(path)
Expand Down
5 changes: 3 additions & 2 deletions pkg/blobs/local_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package blobs
import (
"context"
"io"
"io/fs"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -186,11 +187,11 @@ func (l *LocalStorage) List(pattern string) ([]string, error) {
}
}

if err := filepath.Walk(walkRoot, func(p string, f os.FileInfo, err error) error {
if err := filepath.WalkDir(walkRoot, func(p string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if f.IsDir() {
if d.IsDir() {
return nil
}
if listingParent && !strings.HasPrefix(p, fullPath) {
Expand Down
13 changes: 7 additions & 6 deletions pkg/ccl/changefeedccl/sink_cloudstorage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"context"
"fmt"
"io"
"io/fs"
"math"
"net/url"
"os"
Expand Down Expand Up @@ -111,33 +112,33 @@ func TestCloudStorageSink(t *testing.T) {
return false
}

walkFn := func(path string, info os.FileInfo, err error) error {
walkDirFn := func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if path == absRoot {
return nil
}
if info.IsDir() && !hasChildDirs(path) {
if d.IsDir() && !hasChildDirs(path) {
relPath, _ := filepath.Rel(absRoot, path)
folders = append(folders, relPath)
}
return nil
}

require.NoError(t, filepath.Walk(absRoot, walkFn))
require.NoError(t, filepath.WalkDir(absRoot, walkDirFn))
return folders
}

// slurpDir returns the contents of every file under root (relative to the
// temp dir created above), sorted by the name of the file.
slurpDir := func(t *testing.T) []string {
var files []string
walkFn := func(path string, info os.FileInfo, err error) error {
walkDirFn := func(path string, d fs.DirEntry, err error) error {
if err != nil {
return err
}
if info.IsDir() {
if d.IsDir() {
return nil
}
file, err := os.ReadFile(path)
Expand All @@ -152,7 +153,7 @@ func TestCloudStorageSink(t *testing.T) {
}
absRoot := filepath.Join(externalIODir, testDir(t))
require.NoError(t, os.MkdirAll(absRoot, 0755))
require.NoError(t, filepath.Walk(absRoot, walkFn))
require.NoError(t, filepath.WalkDir(absRoot, walkDirFn))
return files
}

Expand Down
7 changes: 4 additions & 3 deletions pkg/ccl/changefeedccl/testfeed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"encoding/base64"
gojson "encoding/json"
"fmt"
"io/fs"
"math/rand"
"net/url"
"os"
Expand Down Expand Up @@ -1493,13 +1494,13 @@ func (c *cloudFeed) Next() (*cdctest.TestFeedMessage, error) {
return nil, err
}

if err := filepath.Walk(c.dir, c.walkDir); err != nil {
if err := filepath.WalkDir(c.dir, c.walkDir); err != nil {
return nil, err
}
}
}

func (c *cloudFeed) walkDir(path string, info os.FileInfo, err error) error {
func (c *cloudFeed) walkDir(path string, d fs.DirEntry, err error) error {
if strings.HasSuffix(path, `.tmp`) {
// File in the process of being written by ExternalStorage. Ignore.
return nil
Expand All @@ -1520,7 +1521,7 @@ func (c *cloudFeed) walkDir(path string, info os.FileInfo, err error) error {
return err
}

if info.IsDir() {
if d.IsDir() {
// Nothing to do for directories.
return nil
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 39739fd

Please sign in to comment.