Skip to content
This repository was archived by the owner on Jun 3, 2025. It is now read-only.

Commit 406e502

Browse files
author
Aaron Lehmann
committed
Avoid redundant calls to filepath.Clean
filepath.Clean shows up in profiles as a hot spot, and there seem to be many redundant calls, particularly in ignorelist handling. We can avoid these redundant calls by pre-cleaning entries in the ignore list, and providing fast paths when we know we're already dealing with a cleaned candidate path. Before: 580ms 3.03% 72.35% 590ms 3.08% path/filepath.(*lazybuf).append (inline) 390ms 2.03% 74.39% 990ms 5.16% path/filepath.Clean After: 0.13s 0.69% 84.01% 0.17s 0.91% path/filepath.(*lazybuf).append (inline) 0.13s 0.69% 84.70% 0.31s 1.65% path/filepath.Clean
1 parent 81a7294 commit 406e502

5 files changed

Lines changed: 53 additions & 38 deletions

File tree

‎pkg/commands/copy_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
153153
extractedFiles: []string{"/foo.txt"},
154154
contextFiles: []string{"foo.txt"},
155155
}
156-
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
156+
c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
157157
*tc.count++
158158
return nil
159159
}
@@ -166,7 +166,7 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
166166
description: "with no image",
167167
expectErr: true,
168168
}
169-
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
169+
c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
170170
return nil
171171
}
172172
tc.command = c
@@ -176,7 +176,7 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
176176
c := &CachingCopyCommand{
177177
img: fakeImage{},
178178
}
179-
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
179+
c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
180180
return nil
181181
}
182182
return testCase{
@@ -193,7 +193,7 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
193193
},
194194
},
195195
}
196-
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
196+
c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
197197
return nil
198198
}
199199
tc := testCase{

‎pkg/commands/run_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
204204
extractedFiles: []string{"/foo.txt"},
205205
contextFiles: []string{"foo.txt"},
206206
}
207-
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
207+
c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
208208
*tc.count++
209209
return nil
210210
}
@@ -217,7 +217,7 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
217217
desctiption: "with no image",
218218
expectErr: true,
219219
}
220-
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
220+
c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
221221
return nil
222222
}
223223
tc.command = c
@@ -228,7 +228,7 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
228228
img: fakeImage{},
229229
}
230230

231-
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
231+
c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
232232
return nil
233233
}
234234

@@ -246,7 +246,7 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
246246
},
247247
},
248248
}
249-
c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
249+
c.extractFn = func(_ string, _ *tar.Header, _ string, _ io.Reader) error {
250250
return nil
251251
}
252252
tc := testCase{

‎pkg/filesystem/resolve.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func ResolvePaths(paths []string, wl []util.IgnoreListEntry) (pathsToAdd []strin
8080

8181
// If the given path is a symlink and the target is part of the ignorelist
8282
// ignore the target
83-
if util.CheckProvidedIgnoreList(evaled, wl) {
83+
if util.CheckCleanedPathAgainstProvidedIgnoreList(evaled, wl) {
8484
logrus.Debugf("Path %s is ignored, ignoring it", evaled)
8585
continue
8686
}

‎pkg/util/fs_util.go‎

Lines changed: 42 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ type IgnoreListEntry struct {
6060

6161
var defaultIgnoreList = []IgnoreListEntry{
6262
{
63-
Path: config.KanikoDir,
63+
Path: filepath.Clean(config.KanikoDir),
6464
PrefixMatchOnly: false,
6565
},
6666
{
@@ -86,7 +86,7 @@ type FileContext struct {
8686
ExcludedFiles []string
8787
}
8888

89-
type ExtractFunction func(string, *tar.Header, io.Reader) error
89+
type ExtractFunction func(string, *tar.Header, string, io.Reader) error
9090

9191
type FSConfig struct {
9292
includeWhiteout bool
@@ -100,11 +100,17 @@ func IgnoreList() []IgnoreListEntry {
100100
}
101101

102102
func AddToIgnoreList(entry IgnoreListEntry) {
103-
ignorelist = append(ignorelist, entry)
103+
ignorelist = append(ignorelist, IgnoreListEntry{
104+
Path: filepath.Clean(entry.Path),
105+
PrefixMatchOnly: entry.PrefixMatchOnly,
106+
})
104107
}
105108

106109
func AddToDefaultIgnoreList(entry IgnoreListEntry) {
107-
defaultIgnoreList = append(defaultIgnoreList, entry)
110+
defaultIgnoreList = append(defaultIgnoreList, IgnoreListEntry{
111+
Path: filepath.Clean(entry.Path),
112+
PrefixMatchOnly: entry.PrefixMatchOnly,
113+
})
108114
}
109115

110116
func IncludeWhiteout() FSOpt {
@@ -175,7 +181,8 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e
175181
return nil, errors.Wrap(err, fmt.Sprintf("error reading tar %d", i))
176182
}
177183

178-
path := filepath.Join(root, filepath.Clean(hdr.Name))
184+
cleanedName := filepath.Clean(hdr.Name)
185+
path := filepath.Join(root, cleanedName)
179186
base := filepath.Base(path)
180187
dir := filepath.Dir(path)
181188

@@ -185,7 +192,7 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e
185192
name := strings.TrimPrefix(base, archive.WhiteoutPrefix)
186193
path := filepath.Join(dir, name)
187194

188-
if CheckIgnoreList(path) {
195+
if CheckCleanedPathAgainstIgnoreList(path) {
189196
logrus.Tracef("Not deleting %s, as it's ignored", path)
190197
continue
191198
}
@@ -205,11 +212,11 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e
205212

206213
}
207214

208-
if err := cfg.extractFunc(root, hdr, tr); err != nil {
215+
if err := cfg.extractFunc(root, hdr, cleanedName, tr); err != nil {
209216
return nil, err
210217
}
211218

212-
extractedFiles = append(extractedFiles, filepath.Join(root, filepath.Clean(hdr.Name)))
219+
extractedFiles = append(extractedFiles, filepath.Join(root, cleanedName))
213220
}
214221
}
215222
return extractedFiles, nil
@@ -224,7 +231,7 @@ func DeleteFilesystem() error {
224231
return nil //nolint:nilerr
225232
}
226233

227-
if CheckIgnoreList(path) {
234+
if CheckCleanedPathAgainstIgnoreList(path) {
228235
if !isExist(path) {
229236
logrus.Debugf("Path %s ignored, but not exists", path)
230237
return nil
@@ -276,16 +283,17 @@ func UnTar(r io.Reader, dest string) ([]string, error) {
276283
if err != nil {
277284
return nil, err
278285
}
279-
if err := ExtractFile(dest, hdr, tr); err != nil {
286+
cleanedName := filepath.Clean(hdr.Name)
287+
if err := ExtractFile(dest, hdr, cleanedName, tr); err != nil {
280288
return nil, err
281289
}
282-
extractedFiles = append(extractedFiles, filepath.Join(dest, filepath.Clean(hdr.Name)))
290+
extractedFiles = append(extractedFiles, filepath.Join(dest, cleanedName))
283291
}
284292
return extractedFiles, nil
285293
}
286294

287-
func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error {
288-
path := filepath.Join(dest, filepath.Clean(hdr.Name))
295+
func ExtractFile(dest string, hdr *tar.Header, cleanedName string, tr io.Reader) error {
296+
path := filepath.Join(dest, cleanedName)
289297
base := filepath.Base(path)
290298
dir := filepath.Dir(path)
291299
mode := hdr.FileInfo().Mode()
@@ -297,7 +305,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error {
297305
return err
298306
}
299307

300-
if CheckIgnoreList(abs) && !checkIgnoreListRoot(dest) {
308+
if CheckCleanedPathAgainstIgnoreList(abs) && !checkIgnoreListRoot(dest) {
301309
logrus.Debugf("Not adding %s because it is ignored", path)
302310
return nil
303311
}
@@ -358,7 +366,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error {
358366
if err != nil {
359367
return err
360368
}
361-
if CheckIgnoreList(abs) {
369+
if CheckCleanedPathAgainstIgnoreList(abs) {
362370
logrus.Tracef("Skipping link from %s to %s because %s is ignored", hdr.Linkname, path, hdr.Linkname)
363371
return nil
364372
}
@@ -399,6 +407,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error {
399407
}
400408

401409
func IsInProvidedIgnoreList(path string, wl []IgnoreListEntry) bool {
410+
path = filepath.Clean(path)
402411
for _, entry := range wl {
403412
if !entry.PrefixMatchOnly && path == entry.Path {
404413
return true
@@ -412,9 +421,9 @@ func IsInIgnoreList(path string) bool {
412421
return IsInProvidedIgnoreList(path, ignorelist)
413422
}
414423

415-
func CheckProvidedIgnoreList(path string, wl []IgnoreListEntry) bool {
424+
func CheckCleanedPathAgainstProvidedIgnoreList(path string, wl []IgnoreListEntry) bool {
416425
for _, wl := range ignorelist {
417-
if HasFilepathPrefix(path, wl.Path, wl.PrefixMatchOnly) {
426+
if hasCleanedFilepathPrefix(path, wl.Path, wl.PrefixMatchOnly) {
418427
return true
419428
}
420429
}
@@ -423,7 +432,11 @@ func CheckProvidedIgnoreList(path string, wl []IgnoreListEntry) bool {
423432
}
424433

425434
func CheckIgnoreList(path string) bool {
426-
return CheckProvidedIgnoreList(path, ignorelist)
435+
return CheckCleanedPathAgainstIgnoreList(filepath.Clean(path))
436+
}
437+
438+
func CheckCleanedPathAgainstIgnoreList(path string) bool {
439+
return CheckCleanedPathAgainstProvidedIgnoreList(path, ignorelist)
427440
}
428441

429442
func checkIgnoreListRoot(root string) bool {
@@ -463,7 +476,7 @@ func DetectFilesystemIgnoreList(path string) error {
463476
}
464477
if lineArr[4] != config.RootDir {
465478
logrus.Tracef("Adding ignore list entry %s from line: %s", lineArr[4], line)
466-
ignorelist = append(ignorelist, IgnoreListEntry{
479+
AddToIgnoreList(IgnoreListEntry{
467480
Path: lineArr[4],
468481
PrefixMatchOnly: false,
469482
})
@@ -480,12 +493,13 @@ func DetectFilesystemIgnoreList(path string) error {
480493
func RelativeFiles(fp string, root string) ([]string, error) {
481494
var files []string
482495
fullPath := filepath.Join(root, fp)
496+
cleanedRoot := filepath.Clean(root)
483497
logrus.Debugf("Getting files and contents at root %s for %s", root, fullPath)
484498
err := filepath.Walk(fullPath, func(path string, info os.FileInfo, err error) error {
485499
if err != nil {
486500
return err
487501
}
488-
if CheckIgnoreList(path) && !HasFilepathPrefix(path, root, false) {
502+
if CheckCleanedPathAgainstIgnoreList(path) && !hasCleanedFilepathPrefix(filepath.Clean(path), cleanedRoot, false) {
489503
return nil
490504
}
491505
relPath, err := filepath.Rel(root, path)
@@ -591,7 +605,7 @@ func CreateFile(path string, reader io.Reader, perm os.FileMode, uid uint32, gid
591605
// AddVolumePath adds the given path to the volume ignorelist.
592606
func AddVolumePathToIgnoreList(path string) {
593607
logrus.Infof("Adding volume %s to ignorelist", path)
594-
ignorelist = append(ignorelist, IgnoreListEntry{
608+
AddToIgnoreList(IgnoreListEntry{
595609
Path: path,
596610
PrefixMatchOnly: true,
597611
})
@@ -778,11 +792,12 @@ func (c FileContext) ExcludesFile(path string) bool {
778792

779793
// HasFilepathPrefix checks if the given file path begins with prefix
780794
func HasFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool {
781-
prefix = filepath.Clean(prefix)
795+
return hasCleanedFilepathPrefix(filepath.Clean(path), filepath.Clean(prefix), prefixMatchOnly)
796+
}
797+
798+
func hasCleanedFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool {
782799
prefixArray := strings.Split(prefix, "/")
783-
path = filepath.Clean(path)
784800
pathArray := strings.SplitN(path, "/", len(prefixArray)+1)
785-
786801
if len(pathArray) < len(prefixArray) {
787802
return false
788803
}
@@ -968,7 +983,7 @@ func CopyOwnership(src string, destDir string, root string) error {
968983
}
969984
destPath := filepath.Join(destDir, relPath)
970985

971-
if CheckIgnoreList(src) && CheckIgnoreList(destPath) {
986+
if CheckCleanedPathAgainstIgnoreList(src) && CheckCleanedPathAgainstIgnoreList(destPath) {
972987
if !isExist(destPath) {
973988
logrus.Debugf("Path %s ignored, but not exists", destPath)
974989
return nil
@@ -979,7 +994,7 @@ func CopyOwnership(src string, destDir string, root string) error {
979994
logrus.Debugf("Not copying ownership for %s, as it's ignored", destPath)
980995
return nil
981996
}
982-
if CheckIgnoreList(destDir) && CheckIgnoreList(path) {
997+
if CheckIgnoreList(destDir) && CheckCleanedPathAgainstIgnoreList(path) {
983998
if !isExist(path) {
984999
logrus.Debugf("Path %s ignored, but not exists", path)
9851000
return nil
@@ -1118,7 +1133,7 @@ func GetFSInfoMap(dir string, existing map[string]os.FileInfo) (map[string]os.Fi
11181133
timer := timing.Start("Walking filesystem with Stat")
11191134
godirwalk.Walk(dir, &godirwalk.Options{
11201135
Callback: func(path string, ent *godirwalk.Dirent) error {
1121-
if CheckIgnoreList(path) {
1136+
if CheckCleanedPathAgainstIgnoreList(path) {
11221137
if IsDestDir(path) {
11231138
logrus.Tracef("Skipping paths under %s, as it is a ignored directory", path)
11241139
return filepath.SkipDir

‎pkg/util/fs_util_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,7 @@ func TestExtractFile(t *testing.T) {
810810
defer os.RemoveAll(r)
811811

812812
for _, hdr := range tc.hdrs {
813-
if err := ExtractFile(r, hdr, bytes.NewReader(tc.contents)); err != nil {
813+
if err := ExtractFile(r, hdr, filepath.Clean(hdr.Name), bytes.NewReader(tc.contents)); err != nil {
814814
t.Fatal(err)
815815
}
816816
}
@@ -1008,7 +1008,7 @@ func Test_CopyFile_skips_self(t *testing.T) {
10081008
}
10091009
}
10101010

1011-
func fakeExtract(dest string, hdr *tar.Header, tr io.Reader) error {
1011+
func fakeExtract(_ string, _ *tar.Header, _ string, _ io.Reader) error {
10121012
return nil
10131013
}
10141014

0 commit comments

Comments
 (0)