Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propose using filepath.WalkDir instead of filepath.Walk #1014

Closed
neclepsio opened this issue May 28, 2021 · 1 comment
Closed

Propose using filepath.WalkDir instead of filepath.Walk #1014

neclepsio opened this issue May 28, 2021 · 1 comment

Comments

@neclepsio
Copy link

neclepsio commented May 28, 2021

Go 1.16 added filepath.WalkDir alongside filepath.Walk for performance reasons (see golang/go#42027). In my experience, traversing a big network share to retrieve few files based on their name, I got a huge speedup.

From Go 1.16 docs:

Walk is less efficient than WalkDir, introduced in Go 1.16, which avoids calling os.Lstat on every visited file or directory.

The differences between WalkDirFunc compared to filepath.WalkFunc are:

  • The second argument has type fs.DirEntry instead of fs.FileInfo.
  • The function is called before reading a directory, to allow SkipDir to bypass the directory read entirely.
  • If a directory read fails, the function is called a second time for that directory to report the error.

filepath.Walk was not deprecated, but I think filepath.WalkDir should always be preferred: even when using the information not present in fs.DirEntry, you could save time calling os.Stat only when needed.

So I propose to add a check to suggest the use of filepath.WalkDir instead of filepath.Walk.

@neclepsio neclepsio added the needs-triage Newly filed issue that needs triage label May 28, 2021
@dominikh
Copy link
Owner

I don't think we can make this suggestion, as it doesn't fit into any of the categories of checks. Using Walk is not a bug, nor always inferior to using WalkDir either in performance or simplicity, namely when all stats are required. That excludes all of SA and S. And I don't think that "never use filepath.Walk" would make for a good general style rule, thus excluding ST.

Go didn't deprecate filepath.Walk, and I feel like we shouldn't overrule this decision by flagging uses of filepath.Walk anyway.

This would be the kind of rule that an organisation would have to decide on and implement themselves.

@dominikh dominikh added new-check and removed needs-triage Newly filed issue that needs triage labels May 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants