Skip to content

Commit 3590779

Browse files
committed
Addressing comments
1 parent a0206e2 commit 3590779

File tree

6 files changed

+52
-47
lines changed

6 files changed

+52
-47
lines changed

go/pkg/diskcache/BUILD.bazel

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
33
go_library(
44
name = "diskcache",
55
srcs = [
6-
"atim_darwin.go",
7-
"atim_linux.go",
8-
"atim_windows.go",
96
"diskcache.go",
7+
"sys_darwin.go",
8+
"sys_linux.go",
9+
"sys_windows.go",
1010
],
1111
importpath = "github.com/bazelbuild/remote-apis-sdks/go/pkg/diskcache",
1212
visibility = ["//visibility:public"],

go/pkg/diskcache/diskcache.go

Lines changed: 43 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -118,43 +118,50 @@ func New(ctx context.Context, root string, maxCapacityBytes uint64) *DiskCache {
118118
_ = os.MkdirAll(root, os.ModePerm)
119119
// We use Git's directory/file naming structure as inspiration:
120120
// https://git-scm.com/book/en/v2/Git-Internals-Git-Objects#:~:text=The%20subdirectory%20is%20named%20with%20the%20first%202%20characters%20of%20the%20SHA%2D1%2C%20and%20the%20filename%20is%20the%20remaining%2038%20characters.
121+
var wg sync.WaitGroup
122+
wg.Add(256)
121123
for i := 0; i < 256; i++ {
122-
_ = os.MkdirAll(filepath.Join(root, fmt.Sprintf("%02x", i)), os.ModePerm)
123-
}
124-
_ = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
125-
// We log and continue on all errors, because cache read errors are not critical.
126-
if err != nil {
127-
log.Errorf("Error reading cache directory: %v", err)
128-
return nil
129-
}
130-
if d.IsDir() {
131-
return nil
132-
}
133-
subdir := filepath.Base(filepath.Dir(path))
134-
k, err := res.getKeyFromFileName(subdir + d.Name())
135-
if err != nil {
136-
log.Errorf("Error parsing cached file name %s: %v", path, err)
137-
return nil
138-
}
139-
atime, err := GetLastAccessTime(path)
140-
if err != nil {
141-
log.Errorf("Error getting last accessed time of %s: %v", path, err)
142-
return nil
143-
}
144-
it := &qitem{
145-
key: k,
146-
lat: atime,
147-
}
148-
size, err := res.getItemSize(k)
149-
if err != nil {
150-
log.Errorf("Error getting file size of %s: %v", path, err)
151-
return nil
152-
}
153-
res.store.Store(k, it)
154-
atomic.AddInt64(&res.sizeBytes, size)
155-
heap.Push(res.queue, it)
156-
return nil
157-
})
124+
prefixDir := filepath.Join(root, fmt.Sprintf("%02x", i))
125+
go func() {
126+
defer wg.Done()
127+
_ = os.MkdirAll(prefixDir, os.ModePerm)
128+
_ = filepath.WalkDir(prefixDir, func(path string, d fs.DirEntry, err error) error {
129+
// We log and continue on all errors, because cache read errors are not critical.
130+
if err != nil {
131+
log.Errorf("Error reading cache directory: %v", err)
132+
return nil
133+
}
134+
if d.IsDir() {
135+
return nil
136+
}
137+
subdir := filepath.Base(filepath.Dir(path))
138+
k, err := res.getKeyFromFileName(subdir + d.Name())
139+
if err != nil {
140+
log.Errorf("Error parsing cached file name %s: %v", path, err)
141+
return nil
142+
}
143+
atime, err := GetLastAccessTime(path)
144+
if err != nil {
145+
log.Errorf("Error getting last accessed time of %s: %v", path, err)
146+
return nil
147+
}
148+
it := &qitem{
149+
key: k,
150+
lat: atime,
151+
}
152+
size, err := res.getItemSize(k)
153+
if err != nil {
154+
log.Errorf("Error getting file size of %s: %v", path, err)
155+
return nil
156+
}
157+
res.store.Store(k, it)
158+
atomic.AddInt64(&res.sizeBytes, size)
159+
heap.Push(res.queue, it)
160+
return nil
161+
})
162+
}()
163+
}
164+
wg.Wait()
158165
go res.gc()
159166
return res
160167
}

go/pkg/diskcache/diskcache_test.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,9 @@ import (
2121

2222
// Test utility only. Assumes all modifications are done, and at least one GC is expected.
2323
func waitForGc(d *DiskCache) {
24-
for {
25-
select {
26-
case t := <-d.testGcTicks:
27-
if t == d.gcTick {
28-
return
29-
}
24+
for t := range d.testGcTicks {
25+
if t == d.gcTick {
26+
return
3027
}
3128
}
3229
}

go/pkg/diskcache/atim_darwin.go renamed to go/pkg/diskcache/sys_darwin.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// Utility to get the last accessed time on Darwin.
2+
// System utilities that differ between OS implementations.
23
package diskcache
34

45
import (

go/pkg/diskcache/atim_linux.go renamed to go/pkg/diskcache/sys_linux.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Utility to get the last accessed time on Linux.
1+
// System utilities that differ between OS implementations.
22
package diskcache
33

44
import (

go/pkg/diskcache/atim_windows.go renamed to go/pkg/diskcache/sys_windows.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Utility to get the last accessed time on Windows.
1+
// System utilities that differ between OS implementations.
22
package diskcache
33

44
import (

0 commit comments

Comments
 (0)