[gold] refactor diffstore to split out subpackages
These small helpers can/should be written to not use boltdb,
since that might not scale well.
Bug: skia:9080
Change-Id: Id0e0dfa9a9c38298623c04b28d77c95ecb9a2809
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/219776
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
diff --git a/golden/cmd/sampler/main.go b/golden/cmd/sampler/main.go
index 11e9293..d8d35f2 100644
--- a/golden/cmd/sampler/main.go
+++ b/golden/cmd/sampler/main.go
@@ -179,6 +179,9 @@
evt := eventbus.New()
ts, err := auth.NewDefaultTokenSource(true)
+ if err != nil {
+ sklog.Fatalf("Cannot get token source: %s", err)
+ }
fsClient, err := firestore.NewClient(ctx, *fsProjectID, "gold", *fsNamespace, ts)
if err != nil {
diff --git a/golden/cmd/skia_diff_server/main.go b/golden/cmd/skia_diff_server/main.go
index 4c16892..ec9f087 100644
--- a/golden/cmd/skia_diff_server/main.go
+++ b/golden/cmd/skia_diff_server/main.go
@@ -15,6 +15,7 @@
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/golden/go/diff"
"go.skia.org/infra/golden/go/diffstore"
+ "go.skia.org/infra/golden/go/diffstore/mapper/disk_mapper"
gstorage "google.golang.org/api/storage/v1"
"google.golang.org/grpc"
)
@@ -22,7 +23,6 @@
// Command line flags.
var (
cacheSize = flag.Int("cache_size", 1, "Approximate cachesize used to cache images and diff metrics in GiB. This is just a way to limit caching. 0 means no caching at all. Use default for testing.")
- convertLegacy = flag.Bool("convert_legacy", false, "Converts the legacy cache to the new format.")
gsBucketNames = flag.String("gs_buckets", "", "[required] Comma-separated list of google storage bucket that hold uploaded images.")
gsBaseDir = flag.String("gs_basedir", diffstore.DEFAULT_GCS_IMG_DIR_NAME, "String that represents the google storage directory/directories following the GS bucket")
imageDir = flag.String("image_dir", "/tmp/imagedir", "What directory to store test and diff images in.")
@@ -69,16 +69,12 @@
client := httputils.DefaultClientConfig().WithTokenSource(ts).With2xxOnly().Client()
// Get the DiffStore that does the work loading and diffing images.
- mapper := diffstore.NewGoldDiffStoreMapper(&diff.DiffMetrics{})
+ mapper := disk_mapper.New(&diff.DiffMetrics{})
memDiffStore, err := diffstore.NewMemDiffStore(client, *imageDir, strings.Split(*gsBucketNames, ","), *gsBaseDir, *cacheSize, mapper)
if err != nil {
sklog.Fatalf("Allocating DiffStore failed: %s", err)
}
- if *convertLegacy {
- memDiffStore.(*diffstore.MemDiffStore).ConvertLegacy()
- }
-
// Create the server side instance of the DiffService.
codec := diffstore.MetricMapCodec{}
serverImpl := diffstore.NewDiffServiceServer(memDiffStore, codec)
diff --git a/golden/cmd/skiacorrectness/main.go b/golden/cmd/skiacorrectness/main.go
index 75332e3..d8d8c81 100644
--- a/golden/cmd/skiacorrectness/main.go
+++ b/golden/cmd/skiacorrectness/main.go
@@ -44,6 +44,7 @@
"go.skia.org/infra/golden/go/baseline/gcs_baseliner"
"go.skia.org/infra/golden/go/diff"
"go.skia.org/infra/golden/go/diffstore"
+ "go.skia.org/infra/golden/go/diffstore/mapper/disk_mapper"
"go.skia.org/infra/golden/go/expstorage/fs_expstore"
"go.skia.org/infra/golden/go/ignore"
"go.skia.org/infra/golden/go/indexer"
@@ -240,7 +241,7 @@
if *gsBucketNames == "" {
sklog.Fatalf("Must specify --gs_buckets or (--diff_server_http and --diff_server_grpc)")
}
- mapper := diffstore.NewGoldDiffStoreMapper(&diff.DiffMetrics{})
+ mapper := disk_mapper.New(&diff.DiffMetrics{})
diffStore, err = diffstore.NewMemDiffStore(client, *imageDir, strings.Split(*gsBucketNames, ","), diffstore.DEFAULT_GCS_IMG_DIR_NAME, *cacheSize, mapper)
if err != nil {
sklog.Fatalf("Allocating local DiffStore failed: %s", err)
diff --git a/golden/go/baseline/gcs_baseliner/baseliner.go b/golden/go/baseline/gcs_baseliner/baseliner.go
index 624c7aa..f181c9b 100644
--- a/golden/go/baseline/gcs_baseliner/baseliner.go
+++ b/golden/go/baseline/gcs_baseliner/baseliner.go
@@ -263,7 +263,7 @@
} else {
// Clone the retrieved baseline before we inject the issue information.
masterBaseline = masterBaseline.Copy()
- masterBaseline.Expectations.Update(issueBaseline.Expectations)
+ masterBaseline.Expectations.MergeExpectations(issueBaseline.Expectations)
masterBaseline.Issue = issueID
}
}
diff --git a/golden/go/diff/diff.go b/golden/go/diff/diff.go
index 643fbe1..3289451 100644
--- a/golden/go/diff/diff.go
+++ b/golden/go/diff/diff.go
@@ -11,6 +11,7 @@
"os"
"unsafe"
+ "go.skia.org/infra/go/metrics2"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/util"
"go.skia.org/infra/golden/go/types"
@@ -277,7 +278,7 @@
// PixelDiff is a utility function that calculates the DiffMetrics and the image of the
// difference for the provided images.
func PixelDiff(img1, img2 image.Image) (*DiffMetrics, *image.NRGBA) {
-
+ defer metrics2.FuncTimer().Stop()
img1Bounds := img1.Bounds()
img2Bounds := img2.Bounds()
@@ -302,7 +303,7 @@
p1 := GetNRGBA(img1).Pix
p2 := GetNRGBA(img2).Pix
// Compare the bounds, if they are the same then use this fast path.
- // We pun to uint64 to compare 2 pixels at a time, so we also require
+ // We cast to uint64 to compare 2 pixels at a time, so we also require
// an even number of pixels here. If that's a big deal, we can easily
// fix that up, handling the straggler pixel separately at the end.
if img1Bounds.Eq(img2Bounds) && len(p1)%8 == 0 {
diff --git a/golden/go/diff/diff_test.go b/golden/go/diff/diff_test.go
index eaab1c1..be2d09f 100644
--- a/golden/go/diff/diff_test.go
+++ b/golden/go/diff/diff_test.go
@@ -3,6 +3,7 @@
import (
"bytes"
"image"
+ "math"
"path/filepath"
"reflect"
"strings"
@@ -342,3 +343,22 @@
PixelDiff(img1, img2)
}
}
+
+func TestCombinedDiffMetric(t *testing.T) {
+ unittest.SmallTest(t)
+ dm := &DiffMetrics{
+ MaxRGBADiffs: []int{},
+ PixelDiffPercent: 0.0,
+ }
+ assert.InDelta(t, 1.0, CombinedDiffMetric(dm, nil, nil), 0.000001)
+ dm = &DiffMetrics{
+ MaxRGBADiffs: []int{255, 255, 255, 255},
+ PixelDiffPercent: 1.0,
+ }
+ assert.InDelta(t, 1.0, CombinedDiffMetric(dm, nil, nil), 0.000001)
+ dm = &DiffMetrics{
+ MaxRGBADiffs: []int{255, 255, 255, 255},
+ PixelDiffPercent: 0.5,
+ }
+ assert.InDelta(t, math.Sqrt(0.5), CombinedDiffMetric(dm, nil, nil), 0.000001)
+}
diff --git a/golden/go/diff/metrics.go b/golden/go/diff/metrics.go
index 4de92e6..9d083ba 100644
--- a/golden/go/diff/metrics.go
+++ b/golden/go/diff/metrics.go
@@ -3,6 +3,8 @@
import (
"image"
"math"
+
+ "go.skia.org/infra/go/metrics2"
)
const (
@@ -16,7 +18,7 @@
// metrics contains the custom diff metrics.
var metrics = map[string]MetricFn{
- METRIC_COMBINED: combinedDiffMetric,
+ METRIC_COMBINED: CombinedDiffMetric,
METRIC_PERCENT: percentDiffMetric,
METRIC_PIXEL: pixelDiffMetric,
}
@@ -40,6 +42,7 @@
// DefaultDiffFn implements the DiffFn function type. Calculates the basic
// image difference along with custom diff metrics.
func DefaultDiffFn(leftImg *image.NRGBA, rightImg *image.NRGBA) (interface{}, *image.NRGBA) {
+ defer metrics2.FuncTimer().Stop()
ret, diffImg := PixelDiff(leftImg, rightImg)
// Calculate the metrics.
@@ -54,23 +57,22 @@
// combinedDiffMetric returns a value in [0, 1] that represents how large
// the diff is between two images. Implements the MetricFn signature.
-func combinedDiffMetric(basic *DiffMetrics, one *image.NRGBA, two *image.NRGBA) float32 {
- //
+func CombinedDiffMetric(dm *DiffMetrics, _ *image.NRGBA, _ *image.NRGBA) float32 {
// pixelDiffPercent float32, maxRGBA []int) float32 {
- if len(basic.MaxRGBADiffs) == 0 {
+ if len(dm.MaxRGBADiffs) == 0 {
return 1.0
}
// Turn maxRGBA into a percent by taking the root mean square difference from
// [0, 0, 0, 0].
sum := 0.0
- for _, c := range basic.MaxRGBADiffs {
+ for _, c := range dm.MaxRGBADiffs {
sum += float64(c) * float64(c)
}
- normalizedRGBA := math.Sqrt(sum/float64(len(basic.MaxRGBADiffs))) / 255.0
+ normalizedRGBA := math.Sqrt(sum/float64(len(dm.MaxRGBADiffs))) / 255.0
// We take the sqrt of (pixelDiffPercent * normalizedRGBA) to straigten out
// the curve, i.e. think about what a plot of x^2 would look like in the
// range [0, 1].
- return float32(math.Sqrt(float64(basic.PixelDiffPercent) * normalizedRGBA))
+ return float32(math.Sqrt(float64(dm.PixelDiffPercent) * normalizedRGBA))
}
// percentDiffMetric returns pixel percent as the metric. Implements the MetricFn signature.
diff --git a/golden/go/diffstore/bench_test.go b/golden/go/diffstore/bench_test.go
index d694371..13fd6cd 100644
--- a/golden/go/diffstore/bench_test.go
+++ b/golden/go/diffstore/bench_test.go
@@ -8,6 +8,8 @@
assert "github.com/stretchr/testify/require"
"go.skia.org/infra/go/testutils"
"go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/diffstore/mapper/disk_mapper"
+ d_utils "go.skia.org/infra/golden/go/diffstore/testutils"
"go.skia.org/infra/golden/go/ignore"
"go.skia.org/infra/golden/go/mocks"
"go.skia.org/infra/golden/go/serialize"
@@ -22,7 +24,7 @@
func BenchmarkMemDiffStore(b *testing.B) {
sample := loadSample(b)
- baseDir := TEST_DATA_BASE_DIR + "-bench-diffstore"
+ baseDir := d_utils.TEST_DATA_BASE_DIR + "-bench-diffstore"
client := mocks.GetHTTPClient(b)
defer testutils.RemoveAll(b, baseDir)
@@ -47,8 +49,8 @@
}
}
- mapper := NewGoldDiffStoreMapper(&diff.DiffMetrics{})
- diffStore, err := NewMemDiffStore(client, baseDir, []string{TEST_GCS_BUCKET_NAME}, TEST_GCS_IMAGE_DIR, 10, mapper)
+ mapper := disk_mapper.New(&diff.DiffMetrics{})
+ diffStore, err := NewMemDiffStore(client, baseDir, []string{d_utils.TEST_GCS_BUCKET_NAME}, d_utils.TEST_GCS_IMAGE_DIR, 10, mapper)
assert.NoError(b, err)
allDigests := make([]types.DigestSlice, 0, PROCESS_N_TESTS)
processed := 0
diff --git a/golden/go/diffstore/common.go b/golden/go/diffstore/common.go
deleted file mode 100644
index 13861cb..0000000
--- a/golden/go/diffstore/common.go
+++ /dev/null
@@ -1,116 +0,0 @@
-package diffstore
-
-import (
- "encoding/json"
- "fmt"
- "image"
- "image/png"
- "io"
- "os"
- "path"
-
- "github.com/boltdb/bolt"
- "go.skia.org/infra/go/fileutil"
- "go.skia.org/infra/go/util"
- "go.skia.org/infra/golden/go/diff"
- "go.skia.org/infra/golden/go/types"
-)
-
-const (
- // IMG_EXTENSION is the default extension of images.
- IMG_EXTENSION = "png"
-)
-
-// saveFile writes the given file to disk.
-func saveFile(targetPath string, r io.Reader) error {
- f, err := os.Create(targetPath)
- if err != nil {
- return err
- }
- defer util.Close(f)
-
- _, err = io.Copy(f, r)
- return err
-}
-
-// loadImg loads an image from disk.
-func loadImg(sourcePath string) (*image.NRGBA, error) {
- f, err := os.Open(sourcePath)
- if err != nil {
- return nil, err
- }
- defer util.Close(f)
- return decodeImg(f)
-}
-
-// encodeImg encodes the given image as a PNG and writes the result to the
-// given writer.
-func encodeImg(w io.Writer, img *image.NRGBA) error {
- encoder := png.Encoder{CompressionLevel: png.BestSpeed}
- if err := encoder.Encode(w, img); err != nil {
- return err
- }
- return nil
-}
-
-// decodeImg decodes an image from the given reader and returns it as a NRGBA image.
-func decodeImg(reader io.Reader) (*image.NRGBA, error) {
- im, err := png.Decode(reader)
- if err != nil {
- return nil, err
- }
- return diff.GetNRGBA(im), nil
-}
-
-// getDigestImageFileName returns the image name based on the digest.
-func getDigestImageFileName(digest types.Digest) string {
- return fmt.Sprintf("%s.%s", digest, IMG_EXTENSION)
-}
-
-// openBoltDB opens a boltDB in the given given directory with the given name.
-func openBoltDB(baseDir, name string) (*bolt.DB, error) {
- baseDir, err := fileutil.EnsureDirExists(baseDir)
- if err != nil {
- return nil, err
- }
-
- return bolt.Open(path.Join(baseDir, name), 0600, nil)
-}
-
-// saveFilePath saves the given buffer in path.
-func saveFilePath(path string, r io.Reader) error {
- err := fileutil.EnsureDirPathExists(path)
- if err != nil {
- return fmt.Errorf("Unable to create path for %s: %s", path, err)
- }
-
- if err := saveFile(path, r); err != nil {
- return fmt.Errorf("Unable to save file %s. Got error: %s", path, err)
- }
- return nil
-}
-
-// MetricMapCodec implements the util.LRUCodec interface by serializing and
-// deserializing generic diff result structs, instances of map[string]interface{}
-type MetricMapCodec struct{}
-
-// See util.LRUCodec interface
-func (m MetricMapCodec) Encode(data interface{}) ([]byte, error) {
- return json.Marshal(data)
-}
-
-// See util.LRUCodec interface
-func (m MetricMapCodec) Decode(byteData []byte) (interface{}, error) {
- dm := map[types.Digest]*diff.DiffMetrics{}
- err := json.Unmarshal(byteData, &dm)
- if err != nil {
- return nil, err
- }
-
- // Must make result of deserialization generic in order to propagate
- ret := make(map[types.Digest]interface{}, len(dm))
- for k, metric := range dm {
- ret[k] = metric
- }
- return ret, nil
-}
diff --git a/golden/go/diffstore/common/common.go b/golden/go/diffstore/common/common.go
new file mode 100644
index 0000000..3bce304
--- /dev/null
+++ b/golden/go/diffstore/common/common.go
@@ -0,0 +1,98 @@
+package common
+
+import (
+ "fmt"
+ "image"
+ "image/png"
+ "io"
+ "os"
+ "path"
+
+ "github.com/boltdb/bolt"
+ "go.skia.org/infra/go/fileutil"
+ "go.skia.org/infra/go/util"
+ "go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/types"
+)
+
+const (
+ // IMG_EXTENSION is the default extension of images.
+ IMG_EXTENSION = "png"
+)
+
+// SaveFile writes the given file to disk.
+func SaveFile(targetPath string, r io.Reader) error {
+ f, err := os.Create(targetPath)
+ if err != nil {
+ return err
+ }
+ defer util.Close(f)
+
+ _, err = io.Copy(f, r)
+ return err
+}
+
+// LoadImg loads an image from disk.
+func LoadImg(sourcePath string) (*image.NRGBA, error) {
+ f, err := os.Open(sourcePath)
+ if err != nil {
+ return nil, err
+ }
+ defer util.Close(f)
+ return DecodeImg(f)
+}
+
+// EncodeImg encodes the given image as a PNG and writes the result to the
+// given writer.
+func EncodeImg(w io.Writer, img *image.NRGBA) error {
+ encoder := png.Encoder{CompressionLevel: png.BestSpeed}
+ if err := encoder.Encode(w, img); err != nil {
+ return err
+ }
+ return nil
+}
+
+// DecodeImg decodes an image from the given reader and returns it as a NRGBA image.
+func DecodeImg(reader io.Reader) (*image.NRGBA, error) {
+ im, err := png.Decode(reader)
+ if err != nil {
+ return nil, err
+ }
+ return diff.GetNRGBA(im), nil
+}
+
+// GetDigestImageFileName returns the image name based on the digest.
+func GetDigestImageFileName(digest types.Digest) string {
+ return fmt.Sprintf("%s.%s", digest, IMG_EXTENSION)
+}
+
+// OpenBoltDB opens a boltDB in the given given directory with the given name.
+func OpenBoltDB(baseDir, name string) (*bolt.DB, error) {
+ baseDir, err := fileutil.EnsureDirExists(baseDir)
+ if err != nil {
+ return nil, err
+ }
+
+ return bolt.Open(path.Join(baseDir, name), 0600, nil)
+}
+
+// SaveFilePath saves the given buffer in path.
+func SaveFilePath(path string, r io.Reader) error {
+ err := fileutil.EnsureDirPathExists(path)
+ if err != nil {
+ return fmt.Errorf("Unable to create path for %s: %s", path, err)
+ }
+
+ if err := SaveFile(path, r); err != nil {
+ return fmt.Errorf("Unable to save file %s. Got error: %s", path, err)
+ }
+ return nil
+}
+
+func AsStrings(xd types.DigestSlice) []string {
+ s := make([]string, 0, len(xd))
+ for _, d := range xd {
+ s = append(s, string(d))
+ }
+ return s
+}
diff --git a/golden/go/diffstore/diffservice_impl.go b/golden/go/diffstore/diffservice_impl.go
index ac5c919..a3b4c35 100644
--- a/golden/go/diffstore/diffservice_impl.go
+++ b/golden/go/diffstore/diffservice_impl.go
@@ -40,14 +40,6 @@
return d
}
-func asStrings(xd types.DigestSlice) []string {
- s := make([]string, 0, len(xd))
- for _, d := range xd {
- s = append(s, string(d))
- }
- return s
-}
-
// GetDiffs wraps around the Get method of the underlying DiffStore.
func (d *DiffServiceImpl) GetDiffs(ctx context.Context, req *GetDiffsRequest) (*GetDiffsResponse, error) {
diffs, err := d.diffStore.Get(req.Priority, types.Digest(req.MainDigest), asDigests(req.RightDigests))
diff --git a/golden/go/diffstore/failure_store_test.go b/golden/go/diffstore/failure_store_test.go
deleted file mode 100644
index 815e411..0000000
--- a/golden/go/diffstore/failure_store_test.go
+++ /dev/null
@@ -1,53 +0,0 @@
-package diffstore
-
-import (
- "path"
- "testing"
-
- assert "github.com/stretchr/testify/require"
- "go.skia.org/infra/go/testutils"
- "go.skia.org/infra/go/testutils/unittest"
- "go.skia.org/infra/golden/go/diff"
- "go.skia.org/infra/golden/go/types"
-)
-
-func TestFailureHandling(t *testing.T) {
- unittest.MediumTest(t)
-
- // Get a small tile and get them cached.
- w, cleanup := testutils.TempDir(t)
- defer cleanup()
- baseDir := path.Join(w, TEST_DATA_BASE_DIR+"-diffstore-failure")
- client, tile := getSetupAndTile(t, baseDir)
-
- mapper := NewGoldDiffStoreMapper(&diff.DiffMetrics{})
- diffStore, err := NewMemDiffStore(client, baseDir, []string{TEST_GCS_BUCKET_NAME}, TEST_GCS_IMAGE_DIR, 10, mapper)
- assert.NoError(t, err)
-
- validDigestSet := types.DigestSet{}
- for _, trace := range tile.Traces {
- gTrace := trace.(*types.GoldenTrace)
- validDigestSet.AddLists(gTrace.Digests)
- }
- delete(validDigestSet, types.MISSING_DIGEST)
-
- invalidDigest_1 := types.Digest("invaliddigest1")
- invalidDigest_2 := types.Digest("invaliddigest2")
-
- validDigests := validDigestSet.Keys()
- mainDigest := validDigests[0]
- diffDigests := append(validDigests[1:6], invalidDigest_1, invalidDigest_2)
-
- diffs, err := diffStore.Get(diff.PRIORITY_NOW, mainDigest, diffDigests)
- assert.NoError(t, err)
- assert.Equal(t, len(diffDigests)-2, len(diffs))
-
- unavailableDigests := diffStore.UnavailableDigests()
- assert.Equal(t, 2, len(unavailableDigests))
- assert.NotNil(t, unavailableDigests[invalidDigest_1])
- assert.NotNil(t, unavailableDigests[invalidDigest_2])
-
- assert.NoError(t, diffStore.PurgeDigests(types.DigestSlice{invalidDigest_1, invalidDigest_2}, true))
- unavailableDigests = diffStore.UnavailableDigests()
- assert.Equal(t, 0, len(unavailableDigests))
-}
diff --git a/golden/go/diffstore/failure_store.go b/golden/go/diffstore/failurestore/bolt_failurestore/bolt_failurestore.go
similarity index 69%
rename from golden/go/diffstore/failure_store.go
rename to golden/go/diffstore/failurestore/bolt_failurestore/bolt_failurestore.go
index bbe5e2e..aaa731d 100644
--- a/golden/go/diffstore/failure_store.go
+++ b/golden/go/diffstore/failurestore/bolt_failurestore/bolt_failurestore.go
@@ -1,12 +1,15 @@
-package diffstore
+package bolt_failurestore
import (
+ "path/filepath"
"sync"
"github.com/boltdb/bolt"
"go.skia.org/infra/go/boltutil"
"go.skia.org/infra/go/util"
"go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/diffstore/common"
+ "go.skia.org/infra/golden/go/diffstore/failurestore"
"go.skia.org/infra/golden/go/types"
)
@@ -30,10 +33,10 @@
return nil
}
-// failureStore persists DigestFailures in boltDB database. It assumes that the
+// BoltImpl persists DigestFailures in boltDB database. It assumes that the
// number of unavailable digests is small and it keeps a copy of the entire list
// in memory at all time.
-type failureStore struct {
+type BoltImpl struct {
// store stores the digests that have failed to load.
store *boltutil.IndexedBucket
@@ -47,9 +50,10 @@
cacheMutex sync.RWMutex
}
-// newFailureStore returns a new instance of failureStore that opens a database in the given directory.
-func newFailureStore(baseDir string) (*failureStore, error) {
- db, err := openBoltDB(baseDir, FAILUREDB_NAME+".db")
+// New returns a new instance of BoltImpl that opens a database in the given directory.
+func New(baseDir string) (*BoltImpl, error) {
+ baseDir = filepath.Join(baseDir, FAILUREDB_NAME)
+ db, err := common.OpenBoltDB(baseDir, FAILUREDB_NAME+".db")
if err != nil {
return nil, err
}
@@ -65,7 +69,7 @@
return nil, err
}
- ret := &failureStore{
+ ret := &BoltImpl{
store: store,
}
@@ -76,26 +80,26 @@
return ret, nil
}
-// unavailableDigests returns the current list of unavailable digests for fast lookup.
-func (f *failureStore) unavailableDigests() map[types.Digest]*diff.DigestFailure {
+// UnavailableDigests returns the current list of unavailable digests for fast lookup.
+func (f *BoltImpl) UnavailableDigests() map[types.Digest]*diff.DigestFailure {
f.cacheMutex.RLock()
defer f.cacheMutex.RUnlock()
return f.cachedFailures
}
-// addDigestFailureIfNew adds a digest failure to the database only if the
+// AddDigestFailureIfNew adds a digest failure to the database only if the
// there is no failure recorded for the given digest.
-func (f *failureStore) addDigestFailureIfNew(failure *diff.DigestFailure) error {
- unavailable := f.unavailableDigests()
+func (f *BoltImpl) AddDigestFailureIfNew(failure *diff.DigestFailure) error {
+ unavailable := f.UnavailableDigests()
if _, ok := unavailable[failure.Digest]; !ok {
- return f.addDigestFailure(failure)
+ return f.AddDigestFailure(failure)
}
return nil
}
-// addDigestFailure adds a digest failure to the database or updates an
+// AddDigestFailure adds a digest failure to the database or updates an
// existing failure.
-func (f *failureStore) addDigestFailure(failure *diff.DigestFailure) error {
+func (f *BoltImpl) AddDigestFailure(failure *diff.DigestFailure) error {
f.dbMutex.Lock()
defer f.dbMutex.Unlock()
@@ -119,13 +123,13 @@
return f.loadDigestFailures()
}
-// purgeDigestFailures removes the failures identified by digests from the database.
-func (f *failureStore) purgeDigestFailures(digests types.DigestSlice) error {
+// PurgeDigestFailures removes the failures identified by digests from the database.
+func (f *BoltImpl) PurgeDigestFailures(digests types.DigestSlice) error {
f.dbMutex.Lock()
defer f.dbMutex.Unlock()
targets := make([]string, 0, len(digests))
- unavailable := f.unavailableDigests()
+ unavailable := f.UnavailableDigests()
for _, d := range digests {
if _, ok := unavailable[d]; ok {
targets = append(targets, string(d))
@@ -144,7 +148,7 @@
}
// loadDigestFailures loads all digest failures into memory and updates the current cache.
-func (f *failureStore) loadDigestFailures() error {
+func (f *BoltImpl) loadDigestFailures() error {
allFailures, _, err := f.store.List(0, -1)
if err != nil {
return err
@@ -161,3 +165,6 @@
f.cachedFailures = ret
return nil
}
+
+// Make sure BoltImpl fulfills the FailureStore interface
+var _ failurestore.FailureStore = (*BoltImpl)(nil)
diff --git a/golden/go/diffstore/failurestore/bolt_failurestore/bolt_failurestore_test.go b/golden/go/diffstore/failurestore/bolt_failurestore/bolt_failurestore_test.go
new file mode 100644
index 0000000..2a9494e
--- /dev/null
+++ b/golden/go/diffstore/failurestore/bolt_failurestore/bolt_failurestore_test.go
@@ -0,0 +1,104 @@
+package bolt_failurestore
+
+import (
+ "testing"
+ "time"
+
+ assert "github.com/stretchr/testify/require"
+ "go.skia.org/infra/go/testutils"
+ "go.skia.org/infra/go/testutils/unittest"
+ "go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/types"
+)
+
+func TestAddGet(t *testing.T) {
+ unittest.MediumTest(t)
+
+ w, cleanup := testutils.TempDir(t)
+ defer cleanup()
+
+ fs, err := New(w)
+ assert.NoError(t, err)
+
+ assert.Empty(t, fs.UnavailableDigests())
+
+ err = fs.AddDigestFailure(&failureOne)
+ assert.NoError(t, err)
+ err = fs.AddDigestFailure(&failureTwo)
+ assert.NoError(t, err)
+ err = fs.AddDigestFailure(&failureThree)
+ assert.NoError(t, err)
+
+ assert.Equal(t, map[types.Digest]*diff.DigestFailure{
+ digestOne: &failureThree,
+ digestTwo: &failureTwo,
+ }, fs.UnavailableDigests())
+}
+
+func TestAddIfNew(t *testing.T) {
+ unittest.MediumTest(t)
+
+ w, cleanup := testutils.TempDir(t)
+ defer cleanup()
+
+ fs, err := New(w)
+ assert.NoError(t, err)
+
+ err = fs.AddDigestFailureIfNew(&failureOne)
+ assert.NoError(t, err)
+ err = fs.AddDigestFailureIfNew(&failureTwo)
+ assert.NoError(t, err)
+ err = fs.AddDigestFailureIfNew(&failureThree)
+ assert.NoError(t, err)
+
+ assert.Equal(t, map[types.Digest]*diff.DigestFailure{
+ digestOne: &failureOne,
+ digestTwo: &failureTwo,
+ }, fs.UnavailableDigests())
+}
+
+func TestPurge(t *testing.T) {
+ unittest.MediumTest(t)
+
+ w, cleanup := testutils.TempDir(t)
+ defer cleanup()
+
+ fs, err := New(w)
+ assert.NoError(t, err)
+
+ err = fs.AddDigestFailure(&failureOne)
+ assert.NoError(t, err)
+ err = fs.AddDigestFailure(&failureTwo)
+ assert.NoError(t, err)
+ err = fs.PurgeDigestFailures(types.DigestSlice{digestOne})
+ assert.NoError(t, err)
+
+ assert.Equal(t, map[types.Digest]*diff.DigestFailure{
+ digestTwo: &failureTwo,
+ }, fs.UnavailableDigests())
+}
+
+const (
+ digestOne = types.Digest("ab592bfb76536d833e16028bf9525508")
+ digestTwo = types.Digest("9a58d5801670ef194eba7451b08621ac")
+)
+
+var (
+ now = time.Date(2019, time.June, 3, 4, 5, 16, 0, time.UTC)
+
+ failureOne = diff.DigestFailure{
+ Digest: digestOne,
+ Reason: "404",
+ TS: now.Unix() * 1000,
+ }
+ failureTwo = diff.DigestFailure{
+ Digest: digestTwo,
+ Reason: "417",
+ TS: now.Unix()*1000 + 2345,
+ }
+ failureThree = diff.DigestFailure{
+ Digest: digestOne,
+ Reason: "500",
+ TS: now.Unix()*1000 + 6789,
+ }
+)
diff --git a/golden/go/diffstore/failurestore/failurestore.go b/golden/go/diffstore/failurestore/failurestore.go
new file mode 100644
index 0000000..135afc8
--- /dev/null
+++ b/golden/go/diffstore/failurestore/failurestore.go
@@ -0,0 +1,23 @@
+package failurestore
+
+import (
+ "go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/types"
+)
+
+// FailureStore keeps track of any Digests that were unable to be fetched.
+type FailureStore interface {
+ // UnavailableDigests returns the current list of unavailable digests for fast lookup.
+ UnavailableDigests() map[types.Digest]*diff.DigestFailure
+
+ // AddDigestFailureIfNew adds a digest failure to the database only if the
+ // there is no failure recorded for the given digest.
+ AddDigestFailureIfNew(failure *diff.DigestFailure) error
+
+ // AddDigestFailure adds a digest failure to the database or updates an
+ // existing failure.
+ AddDigestFailure(failure *diff.DigestFailure) error
+
+ // PurgeDigestFailures removes the failures identified by digests from the database.
+ PurgeDigestFailures(digests types.DigestSlice) error
+}
diff --git a/golden/go/diffstore/imgloader.go b/golden/go/diffstore/imgloader.go
index 6aa6804..dca03f1 100644
--- a/golden/go/diffstore/imgloader.go
+++ b/golden/go/diffstore/imgloader.go
@@ -21,6 +21,10 @@
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/util"
"go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/diffstore/common"
+ "go.skia.org/infra/golden/go/diffstore/failurestore"
+ "go.skia.org/infra/golden/go/diffstore/failurestore/bolt_failurestore"
+ "go.skia.org/infra/golden/go/diffstore/mapper"
"go.skia.org/infra/golden/go/types"
"google.golang.org/api/option"
)
@@ -51,24 +55,23 @@
imageCache rtcache.ReadThroughCache
// failureStore persists failures in retrieving images.
- failureStore *failureStore
+ failureStore failurestore.FailureStore
// mapper contains various functions for creating image IDs and paths.
- mapper DiffStoreMapper
+ mapper mapper.Mapper
}
// Creates a new instance of ImageLoader.
-func NewImgLoader(client *http.Client, baseDir, imgDir string, gsBucketNames []string, gsImageBaseDir string, maxCacheSize int, mapper DiffStoreMapper) (*ImageLoader, error) {
+func NewImgLoader(client *http.Client, baseDir, imgDir string, gsBucketNames []string, gsImageBaseDir string, maxCacheSize int, m mapper.Mapper) (*ImageLoader, error) {
storageClient, err := storage.NewClient(context.Background(), option.WithHTTPClient(client))
if err != nil {
return nil, err
}
- fsPath := filepath.Join(baseDir, FAILUREDB_NAME)
- fStore, err := newFailureStore(fsPath)
+ fStore, err := bolt_failurestore.New(baseDir)
if err != nil {
return nil, err
}
- sklog.Infof("failure store created at %s", fsPath)
+ sklog.Infof("failure store created at %s", baseDir)
ret := &ImageLoader{
storageClient: storageClient,
@@ -76,7 +79,7 @@
gsBucketNames: gsBucketNames,
gsImageBaseDir: gsImageBaseDir,
failureStore: fStore,
- mapper: mapper,
+ mapper: m,
}
// Set up the work queues that balance the load.
@@ -162,7 +165,7 @@
_, _ = msg.WriteString("\n")
// This captures the edge case when the error is cached in the image loader.
- util.LogErr(il.failureStore.addDigestFailureIfNew(diff.NewDigestFailure(errRet.id, diff.OTHER)))
+ util.LogErr(il.failureStore.AddDigestFailureIfNew(diff.NewDigestFailure(errRet.id, diff.OTHER)))
}
return nil, nil, errors.New(msg.String())
}
@@ -172,14 +175,14 @@
// IsOnDisk returns true if the image that corresponds to the given imageID is in the disk cache.
func (il *ImageLoader) IsOnDisk(imageID types.Digest) bool {
- localRelPath, _, _ := il.mapper.ImagePaths(imageID)
+ localRelPath, _ := il.mapper.ImagePaths(imageID)
return fileutil.FileExists(filepath.Join(il.localImgDir, localRelPath))
}
// PurgeImages removes the images that correspond to the given images.
func (il *ImageLoader) PurgeImages(images types.DigestSlice, purgeGCS bool) error {
for _, id := range images {
- localRelPath, bucket, gsRelPath := il.mapper.ImagePaths(id)
+ localRelPath, gsRelPath := il.mapper.ImagePaths(id)
localPath := filepath.Join(il.localImgDir, localRelPath)
if fileutil.FileExists(localPath) {
if err := os.Remove(localPath); err != nil {
@@ -188,7 +191,7 @@
}
if purgeGCS {
- il.removeImg(bucket, gsRelPath)
+ il.removeImg(gsRelPath)
}
}
return nil
@@ -198,32 +201,31 @@
// It loads an image file either from disk or from Google storage.
func (il *ImageLoader) imageLoadWorker(priority int64, imageID types.Digest) (interface{}, error) {
// Check if the image is in the disk cache.
- localRelPath, bucket, gsRelPath := il.mapper.ImagePaths(imageID)
+ localRelPath, gsRelPath := il.mapper.ImagePaths(imageID)
localPath := filepath.Join(il.localImgDir, localRelPath)
sklog.Debugf("Looking for image with id %s", imageID)
if fileutil.FileExists(localPath) {
- img, err := loadImg(localPath)
+ img, err := common.LoadImg(localPath)
if err != nil {
- util.LogErr(il.failureStore.addDigestFailure(diff.NewDigestFailure(imageID, diff.CORRUPTED)))
+ util.LogErr(il.failureStore.AddDigestFailure(diff.NewDigestFailure(imageID, diff.CORRUPTED)))
return nil, skerr.Fmt("Could not load %s from disk: %s", localPath, err)
}
sklog.Debugf("Found it on disk at %s", localPath)
- util.LogErr(il.failureStore.purgeDigestFailures(types.DigestSlice{imageID}))
+ util.LogErr(il.failureStore.PurgeDigestFailures(types.DigestSlice{imageID}))
return &imgRet{img: img}, nil
}
- sklog.Debugf("%s is not on disk yet, going to download from gs://%s", imageID, bucket)
// Download the image from GCS
- imgBytes, err := il.downloadImg(bucket, gsRelPath)
+ imgBytes, err := il.downloadImg(gsRelPath)
if err != nil {
- util.LogErr(il.failureStore.addDigestFailure(diff.NewDigestFailure(imageID, diff.HTTP)))
+ util.LogErr(il.failureStore.AddDigestFailure(diff.NewDigestFailure(imageID, diff.HTTP)))
return nil, err
}
// Decode it and return it.
- img, err := decodeImg(bytes.NewBuffer(imgBytes))
+ img, err := common.DecodeImg(bytes.NewBuffer(imgBytes))
if err != nil {
- util.LogErr(il.failureStore.addDigestFailure(diff.NewDigestFailure(imageID, diff.CORRUPTED)))
+ util.LogErr(il.failureStore.AddDigestFailure(diff.NewDigestFailure(imageID, diff.CORRUPTED)))
return nil, err
}
@@ -236,9 +238,9 @@
writeDoneCh := make(chan bool)
go func() {
defer close(writeDoneCh)
- localRelPath, _, _ := il.mapper.ImagePaths(imageID)
+ localRelPath, _ := il.mapper.ImagePaths(imageID)
p := filepath.Join(il.localImgDir, localRelPath)
- if err := saveFilePath(p, bytes.NewBuffer(imgBytes)); err != nil {
+ if err := common.SaveFilePath(p, bytes.NewBuffer(imgBytes)); err != nil {
sklog.Errorf("Could not write file to disk at %s: %s", p, err)
}
}()
@@ -248,16 +250,9 @@
// downloadImg retrieves the given image from Google storage. If bucket is not empty
// the gsPath is considered absolute within the bucket, otherwise it is relative to gsImageBaseDir.
// We will look in every bucket we are configured for.
-func (il *ImageLoader) downloadImg(bucket, gsPath string) ([]byte, error) {
- var bucketNames []string
- var objLocation string
- if bucket != "" {
- objLocation = gsPath
- bucketNames = []string{bucket}
- } else {
- objLocation = filepath.Join(il.gsImageBaseDir, gsPath)
- bucketNames = il.gsBucketNames
- }
+func (il *ImageLoader) downloadImg(gsPath string) ([]byte, error) {
+ bucketNames := il.gsBucketNames
+ objLocation := filepath.Join(il.gsImageBaseDir, gsPath)
var err error
var imgData []byte
@@ -329,13 +324,10 @@
}
// removeImg removes the image that corresponds to the given relative path from GCS.
-func (il *ImageLoader) removeImg(bucket, gsRelPath string) {
+func (il *ImageLoader) removeImg(gsRelPath string) {
// If the bucket is not empty then look there otherwise use the default buckets.
objLocation := filepath.Join(il.gsImageBaseDir, gsRelPath)
bucketNames := il.gsBucketNames
- if bucket != "" {
- bucketNames = []string{bucket}
- }
ctx := context.Background()
for _, bucketName := range bucketNames {
diff --git a/golden/go/diffstore/imgloader_test.go b/golden/go/diffstore/imgloader_test.go
index 8dcee32..d425745 100644
--- a/golden/go/diffstore/imgloader_test.go
+++ b/golden/go/diffstore/imgloader_test.go
@@ -1,7 +1,6 @@
package diffstore
import (
- "fmt"
"os"
"path"
"path/filepath"
@@ -13,6 +12,10 @@
"go.skia.org/infra/go/testutils"
"go.skia.org/infra/go/testutils/unittest"
"go.skia.org/infra/go/tiling"
+ "go.skia.org/infra/golden/go/diffstore/common"
+ "go.skia.org/infra/golden/go/diffstore/mapper"
+ "go.skia.org/infra/golden/go/diffstore/mapper/disk_mapper"
+ d_utils "go.skia.org/infra/golden/go/diffstore/testutils"
"go.skia.org/infra/golden/go/types"
)
@@ -24,8 +27,8 @@
func TestImageLoader(t *testing.T) {
unittest.LargeTest(t)
- mapper := GoldDiffStoreMapper{}
- workingDir, tile, imageLoader, cleanup := getImageLoaderAndTile(t, mapper)
+ m := &disk_mapper.DiskMapper{}
+ workingDir, tile, imageLoader, cleanup := getImageLoaderAndTile(t, m)
defer cleanup()
// Iterate over the tile and get all the digests
@@ -45,7 +48,8 @@
// Make sure they are all on disk.
for _, digest := range digests {
- assert.True(t, fileutil.FileExists(fileutil.TwoLevelRadixPath(workingDir, getDigestImageFileName(digest))))
+ fn := common.GetDigestImageFileName(digest)
+ assert.True(t, fileutil.FileExists(fileutil.TwoLevelRadixPath(workingDir, fn)))
}
// Fetch images from the secondary bucket.
@@ -55,23 +59,17 @@
assert.Error(t, err)
}
-// Calls TwoLevelRadixPath to create the local image file path.
-func DefaultImagePath(baseDir, imageID string) string {
- imagePath := fmt.Sprintf("%s.%s", imageID, IMG_EXTENSION)
- return fileutil.TwoLevelRadixPath(baseDir, imagePath)
-}
-
-func getImageLoaderAndTile(t sktest.TestingT, mapper DiffStoreMapper) (string, *tiling.Tile, *ImageLoader, func()) {
+func getImageLoaderAndTile(t sktest.TestingT, m mapper.Mapper) (string, *tiling.Tile, *ImageLoader, func()) {
w, cleanup := testutils.TempDir(t)
- baseDir := path.Join(w, TEST_DATA_BASE_DIR+"-imgloader")
- client, tile := getSetupAndTile(t, baseDir)
+ baseDir := path.Join(w, d_utils.TEST_DATA_BASE_DIR+"-imgloader")
+ client, tile := d_utils.GetSetupAndTile(t, baseDir)
workingDir := filepath.Join(baseDir, "images")
assert.Nil(t, os.Mkdir(workingDir, 0777))
imgCacheCount, _ := getCacheCounts(10)
- gsBuckets := []string{TEST_GCS_BUCKET_NAME, TEST_GCS_SECONDARY_BUCKET}
- imgLoader, err := NewImgLoader(client, baseDir, workingDir, gsBuckets, TEST_GCS_IMAGE_DIR, imgCacheCount, mapper)
+ gsBuckets := []string{d_utils.TEST_GCS_BUCKET_NAME, d_utils.TEST_GCS_SECONDARY_BUCKET}
+ imgLoader, err := NewImgLoader(client, baseDir, workingDir, gsBuckets, d_utils.TEST_GCS_IMAGE_DIR, imgCacheCount, m)
assert.NoError(t, err)
return workingDir, tile, imgLoader, cleanup
}
diff --git a/golden/go/diffstore/mapper.go b/golden/go/diffstore/mapper.go
deleted file mode 100644
index b893230..0000000
--- a/golden/go/diffstore/mapper.go
+++ /dev/null
@@ -1,94 +0,0 @@
-package diffstore
-
-import (
- "fmt"
- "image"
- "strings"
-
- "go.skia.org/infra/go/fileutil"
- "go.skia.org/infra/go/util"
- "go.skia.org/infra/golden/go/diff"
- "go.skia.org/infra/golden/go/types"
- "go.skia.org/infra/golden/go/validation"
-)
-
-const (
- // DIFF_IMG_SEPARATOR is the character that separates two image ids in the
- // resulting diff image.
- DIFF_IMG_SEPARATOR = "-"
-)
-
-// GoldIDPathMapper implements the DiffStoreMapper interface. It translates
-// between digests (image ids) and storage paths. It uses diff.DiffMetrics
-// as the Gold diff metric.
-type GoldDiffStoreMapper struct {
- util.LRUCodec
-}
-
-// NewGoldDiffStoreMapper returns a new instance of GoldDiffStoreMapper that uses
-// a JSON coded to serialize/deserialize instances of diff.DiffMetrics.
-func NewGoldDiffStoreMapper(diffInstance interface{}) DiffStoreMapper {
- return GoldDiffStoreMapper{LRUCodec: util.JSONCodec(diffInstance)}
-}
-
-// DiffFn implements the DiffStoreMapper interface.
-func (g GoldDiffStoreMapper) DiffFn(leftImg *image.NRGBA, rightImg *image.NRGBA) (interface{}, *image.NRGBA) {
- return diff.DefaultDiffFn(leftImg, rightImg)
-}
-
-// DiffID implements the DiffStoreMapper interface.
-func (g GoldDiffStoreMapper) DiffID(leftImgID, rightImgID types.Digest) string {
- _, _, diffID := g.getOrderedDiffID(leftImgID, rightImgID)
- return diffID
-}
-
-// SplitDiffID implements the DiffStoreMapper interface.
-func (g GoldDiffStoreMapper) SplitDiffID(diffID string) (types.Digest, types.Digest) {
- imageIDs := strings.Split(diffID, DIFF_IMG_SEPARATOR)
-
- // TODO(stephana): Remove this legacy handling code as soon as it has converted the
- // database in production.
- if strings.Contains(diffID, ":") {
- imageIDs = strings.Split(diffID, ":")
- }
-
- return types.Digest(imageIDs[0]), types.Digest(imageIDs[1])
-}
-
-// SplitDiffID implements the DiffStoreMapper interface.
-func (g GoldDiffStoreMapper) DiffPath(leftImgDigest, rightImgDigest types.Digest) string {
- // Get the diff ID and the left imageID.
- _, _, diffID := g.getOrderedDiffID(leftImgDigest, rightImgDigest)
- imagePath := fmt.Sprintf("%s.%s", diffID, IMG_EXTENSION)
-
- return fileutil.TwoLevelRadixPath(imagePath)
-}
-
-// ImagePaths implements the DiffStoreMapper interface.
-func (g GoldDiffStoreMapper) ImagePaths(imageID types.Digest) (string, string, string) {
- gsPath := fmt.Sprintf("%s.%s", imageID, IMG_EXTENSION)
- localPath := fileutil.TwoLevelRadixPath(gsPath)
- return localPath, "", gsPath
-}
-
-// IsValidDiffImgIDimplements the DiffStoreMapper interface.
-func (g GoldDiffStoreMapper) IsValidDiffImgID(diffImgID string) bool {
- imageIDs := strings.Split(diffImgID, DIFF_IMG_SEPARATOR)
- if len(imageIDs) != 2 {
- return false
- }
- return g.IsValidImgID(imageIDs[0]) && g.IsValidImgID(imageIDs[1])
-}
-
-// IsValidImgIDimplements the DiffStoreMapper interface.
-func (g GoldDiffStoreMapper) IsValidImgID(imgID string) bool {
- return validation.IsValidDigest(imgID)
-}
-
-func (g GoldDiffStoreMapper) getOrderedDiffID(leftImgID, rightImgID types.Digest) (types.Digest, types.Digest, string) {
- if rightImgID < leftImgID {
- // Make sure the smaller digest is left imageID.
- leftImgID, rightImgID = rightImgID, leftImgID
- }
- return leftImgID, rightImgID, string(leftImgID) + DIFF_IMG_SEPARATOR + string(rightImgID)
-}
diff --git a/golden/go/diffstore/mapper/disk_mapper/disk_mapper.go b/golden/go/diffstore/mapper/disk_mapper/disk_mapper.go
new file mode 100644
index 0000000..d846ede
--- /dev/null
+++ b/golden/go/diffstore/mapper/disk_mapper/disk_mapper.go
@@ -0,0 +1,57 @@
+package disk_mapper
+
+import (
+ "fmt"
+ "image"
+
+ "go.skia.org/infra/go/fileutil"
+ "go.skia.org/infra/go/util"
+ "go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/diffstore/mapper"
+ "go.skia.org/infra/golden/go/types"
+)
+
+const (
+ imgExtension = "png"
+)
+
+// DiskMapper implements the Mapper interface.
+// It uses diff.DiffMetrics as the Gold diff metric.
+// It stores the images and diffs on disk using a two
+// level radix prefix (i.e. for digest "abcdefg.png", the
+// image will be in ab/cd/abcdefg.png). The use of the radix
+// allows us to work around limits in Linux of how many files
+// can be in a folder.
+type DiskMapper struct {
+ util.LRUCodec
+}
+
+// New returns a new instance of DiskMapper that uses
+// a JSON coded to serialize/deserialize instances of diff.DiffMetrics.
+func New(diffInstance interface{}) *DiskMapper {
+ return &DiskMapper{LRUCodec: util.JSONCodec(diffInstance)}
+}
+
+// DiffFn implements the DiffStoreMapper interface.
+func (g *DiskMapper) DiffFn(left *image.NRGBA, right *image.NRGBA) (interface{}, *image.NRGBA) {
+ return diff.DefaultDiffFn(left, right)
+}
+
+// SplitDiffID implements the DiffStoreMapper interface.
+func (g *DiskMapper) DiffPath(left, right types.Digest) string {
+ // Get the diff ID and the left imageID.
+ diffID := mapper.DiffID(left, right)
+ imagePath := fmt.Sprintf("%s.%s", diffID, imgExtension)
+
+ return fileutil.TwoLevelRadixPath(imagePath)
+}
+
+// ImagePaths implements the DiffStoreMapper interface.
+func (g *DiskMapper) ImagePaths(imageID types.Digest) (string, string) {
+ gsPath := fmt.Sprintf("%s.%s", imageID, imgExtension)
+ localPath := fileutil.TwoLevelRadixPath(gsPath)
+ return localPath, gsPath
+}
+
+// Make sure DiskMapper fulfills the Mapper interface
+var _ mapper.Mapper = (*DiskMapper)(nil)
diff --git a/golden/go/diffstore/mapper/disk_mapper/disk_mapper_test.go b/golden/go/diffstore/mapper/disk_mapper/disk_mapper_test.go
new file mode 100644
index 0000000..8ca5caa
--- /dev/null
+++ b/golden/go/diffstore/mapper/disk_mapper/disk_mapper_test.go
@@ -0,0 +1,43 @@
+package disk_mapper
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "go.skia.org/infra/go/testutils/unittest"
+ "go.skia.org/infra/golden/go/diffstore/testutils"
+ "go.skia.org/infra/golden/go/types"
+)
+
+const (
+ // Arbitrary MD5 digests
+ imgOne = types.Digest("098f6bcd4621d373cade4e832627b4f6")
+ imgTwo = types.Digest("1660f0783f4076284bc18c5f4bdc9608")
+
+ exampleDiffId = "098f6bcd4621d373cade4e832627b4f6-1660f0783f4076284bc18c5f4bdc9608"
+
+ // PNG extension.
+ png = ".png"
+)
+
+func TestDiffPath(t *testing.T) {
+ unittest.SmallTest(t)
+
+ dm := New(&testutils.DummyDiffMetrics{})
+
+ actualDiffPath := dm.DiffPath(imgOne, imgTwo)
+ expectedPath := "09/8f/" + exampleDiffId + png
+ assert.Equal(t, expectedPath, actualDiffPath)
+}
+
+func TestImagePaths(t *testing.T) {
+ unittest.SmallTest(t)
+
+ dm := New(&testutils.DummyDiffMetrics{})
+
+ expectedLocalPath := "09/8f/" + string(imgOne) + png
+ expectedGSPath := string(imgOne + png)
+ localPath, gsPath := dm.ImagePaths(imgOne)
+ assert.Equal(t, expectedLocalPath, localPath)
+ assert.Equal(t, expectedGSPath, gsPath)
+}
diff --git a/golden/go/diffstore/mapper/mapper.go b/golden/go/diffstore/mapper/mapper.go
new file mode 100644
index 0000000..757707b
--- /dev/null
+++ b/golden/go/diffstore/mapper/mapper.go
@@ -0,0 +1,79 @@
+package mapper
+
+import (
+ "image"
+ "strings"
+
+ "go.skia.org/infra/go/util"
+ "go.skia.org/infra/golden/go/types"
+ "go.skia.org/infra/golden/go/validation"
+)
+
+// Mapper is the interface to define how the diff metric between two images
+// is calculated and defines how those diffs are stored on disk.
+// TODO(kjlubick): It might be nice to have Mapper just focus on the
+// diff metric and have a different interface for the disk storing.
+// Or make the interface just serve the bytes (if possible).
+type Mapper interface {
+ // LRUCodec defines the Encode and Decode functions to serialize/deserialize
+ // instances of the diff metrics returned by the DiffFn function below.
+ util.LRUCodec
+
+ // DiffFn calculates the different between two given images and returns a
+ // difference image. The type underlying interface{} is the input and output
+ // of the LRUCodec above. It is also what is returned by the Get(...) function
+ // of the DiffStore interface.
+ DiffFn(*image.NRGBA, *image.NRGBA) (interface{}, *image.NRGBA)
+
+ // DiffPath returns the local file path for the diff image of two images.
+ // This path is used to store the diff image on disk and serve it over HTTP.
+ DiffPath(left, right types.Digest) string
+
+ // ImagePaths returns the storage paths for a given image ID. The first return
+ // value is the local file path used to store the image on disk and serve it
+ // over HTTP. The second return value is the GCS path (not including the bucket).
+ ImagePaths(id types.Digest) (string, string)
+}
+
+const (
+ // DiffImageSeparator is the character that separates two image ids in the
+ // resulting diff image.
+ DiffImageSeparator = "-"
+)
+
+// Takes two image IDs and returns a unique diff ID.
+// Note: DiffID(a,b) == DiffID(b, a) holds.
+func DiffID(left, right types.Digest) string {
+ _, _, diffID := getOrderedDiffID(left, right)
+ return diffID
+}
+
+// Inverse function of DiffID.
+// SplitDiffID(DiffID(a,b)) deterministically returns (a,b) or (b,a).
+func SplitDiffID(diffID string) (types.Digest, types.Digest) {
+ imageIDs := strings.Split(diffID, DiffImageSeparator)
+
+ return types.Digest(imageIDs[0]), types.Digest(imageIDs[1])
+}
+
+// IsValidDiffImgID returns true if the given diffImgID is in the correct format.
+func IsValidDiffImgID(diffID string) bool {
+ imageIDs := strings.Split(diffID, DiffImageSeparator)
+ if len(imageIDs) != 2 {
+ return false
+ }
+ return IsValidImgID(imageIDs[0]) && IsValidImgID(imageIDs[1])
+}
+
+// IsValidImgID returns true if the given imgID is in the correct format.
+func IsValidImgID(imgID string) bool {
+ return validation.IsValidDigest(imgID)
+}
+
+func getOrderedDiffID(left, right types.Digest) (types.Digest, types.Digest, string) {
+ if right < left {
+ // Make sure the smaller digest is left imageID.
+ left, right = right, left
+ }
+ return left, right, string(left) + DiffImageSeparator + string(right)
+}
diff --git a/golden/go/diffstore/mapper/mapper_test.go b/golden/go/diffstore/mapper/mapper_test.go
new file mode 100644
index 0000000..b85c621
--- /dev/null
+++ b/golden/go/diffstore/mapper/mapper_test.go
@@ -0,0 +1,48 @@
+package mapper
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "go.skia.org/infra/go/testutils/unittest"
+ "go.skia.org/infra/golden/go/types"
+)
+
+const (
+ // Arbitrary MD5 digests
+ imgOne = types.Digest("098f6bcd4621d373cade4e832627b4f6")
+ imgTwo = types.Digest("1660f0783f4076284bc18c5f4bdc9608")
+
+ exampleDiffID = "098f6bcd4621d373cade4e832627b4f6-1660f0783f4076284bc18c5f4bdc9608"
+)
+
+func TestDiffID(t *testing.T) {
+ unittest.SmallTest(t)
+
+ diOne := DiffID(imgOne, imgTwo)
+ diTwo := DiffID(imgTwo, imgOne)
+ assert.Equal(t, diOne, diTwo)
+ assert.Equal(t, exampleDiffID, diOne)
+
+ assert.True(t, IsValidDiffImgID(diOne))
+ assert.False(t, IsValidDiffImgID("nope"))
+ assert.False(t, IsValidDiffImgID(string(imgOne)))
+}
+
+func TestSplitDiffID(t *testing.T) {
+ unittest.SmallTest(t)
+
+ actualDiffID := DiffID(imgOne, imgTwo)
+ actualLeft, actualRight := SplitDiffID(exampleDiffID)
+ assert.Equal(t, exampleDiffID, actualDiffID)
+ assert.Equal(t, imgOne, actualLeft)
+ assert.Equal(t, imgTwo, actualRight)
+}
+
+func TestIsValidImgID(t *testing.T) {
+ unittest.SmallTest(t)
+
+ assert.True(t, IsValidImgID(string(imgOne)))
+ assert.True(t, IsValidImgID(string(imgTwo)))
+ assert.False(t, IsValidImgID("nope"))
+}
diff --git a/golden/go/diffstore/mapper_test.go b/golden/go/diffstore/mapper_test.go
deleted file mode 100644
index 56e1ea3..0000000
--- a/golden/go/diffstore/mapper_test.go
+++ /dev/null
@@ -1,87 +0,0 @@
-package diffstore
-
-import (
- "path"
- "testing"
-
- assert "github.com/stretchr/testify/require"
- "go.skia.org/infra/go/testutils"
- "go.skia.org/infra/go/testutils/unittest"
- "go.skia.org/infra/golden/go/types"
-)
-
-const (
- // Test digests for GoldDiffStoreMapper.
- TEST_GOLD_LEFT = types.Digest("098f6bcd4621d373cade4e832627b4f6")
- TEST_GOLD_RIGHT = types.Digest("1660f0783f4076284bc18c5f4bdc9608")
-
- // PNG extension.
- DOT_EXT = ".png"
-)
-
-func TestGoldDiffStoreMapper(t *testing.T) {
- unittest.SmallTest(t)
-
- mapper := GoldDiffStoreMapper{}
-
- // Test DiffID and SplitDiffID
- expectedDiffID := string(TEST_GOLD_LEFT + DIFF_IMG_SEPARATOR + TEST_GOLD_RIGHT)
- actualDiffID := mapper.DiffID(TEST_GOLD_LEFT, TEST_GOLD_RIGHT)
- actualLeft, actualRight := mapper.SplitDiffID(expectedDiffID)
- assert.Equal(t, expectedDiffID, actualDiffID)
- assert.Equal(t, TEST_GOLD_LEFT, actualLeft)
- assert.Equal(t, TEST_GOLD_RIGHT, actualRight)
-
- // Test DiffPath
- twoLevelRadix := TEST_GOLD_LEFT[0:2] + "/" + TEST_GOLD_LEFT[2:4] + "/"
- expectedDiffPath := string(twoLevelRadix + TEST_GOLD_LEFT + "-" +
- TEST_GOLD_RIGHT + DOT_EXT)
- actualDiffPath := mapper.DiffPath(TEST_GOLD_LEFT, TEST_GOLD_RIGHT)
- assert.Equal(t, expectedDiffPath, actualDiffPath)
-
- // Test ImagePaths
- expectedLocalPath := string(twoLevelRadix + TEST_GOLD_LEFT + DOT_EXT)
- expectedGSPath := string(TEST_GOLD_LEFT + DOT_EXT)
- localPath, bucket, gsPath := mapper.ImagePaths(TEST_GOLD_LEFT)
- assert.Equal(t, expectedLocalPath, localPath)
- assert.Equal(t, "", bucket)
- assert.Equal(t, expectedGSPath, gsPath)
-
- // Test IsValidDiffImgID
- // Trim the two level radix path and image extension first
- expectedDiffImgID := expectedDiffPath[len(twoLevelRadix) : len(expectedDiffPath)-len(DOT_EXT)]
- assert.True(t, mapper.IsValidDiffImgID(expectedDiffImgID))
-
- // Test IsValidImgID
- assert.True(t, mapper.IsValidImgID(string(TEST_GOLD_LEFT)))
- assert.True(t, mapper.IsValidImgID(string(TEST_GOLD_RIGHT)))
-}
-
-func TestCodec(t *testing.T) {
- unittest.MediumTest(t)
-
- w, cleanup := testutils.TempDir(t)
- defer cleanup()
- baseDir := path.Join(w, TEST_DATA_BASE_DIR+"-codec")
- client, _ := getSetupAndTile(t, baseDir)
-
- // Instantiate a new MemDiffStore with a codec for the test struct defined above.
- mapper := NewGoldDiffStoreMapper(&DummyDiffMetrics{})
- diffStore, err := NewMemDiffStore(client, baseDir, []string{TEST_GCS_BUCKET_NAME}, TEST_GCS_IMAGE_DIR, 10, mapper)
- assert.NoError(t, err)
- memDiffStore := diffStore.(*MemDiffStore)
-
- diffID := mapper.DiffID(TEST_GOLD_LEFT, TEST_GOLD_RIGHT)
- diffMetrics := &DummyDiffMetrics{
- NumDiffPixels: 100,
- PercentDiffPixels: 0.5,
- }
- err = memDiffStore.metricsStore.saveDiffMetrics(diffID, diffMetrics)
- assert.NoError(t, err)
-
- // Verify the returned diff metrics object has the same type and same contents
- // as the object that was saved to the metricsStore.
- metrics, err := memDiffStore.metricsStore.loadDiffMetrics(diffID)
- assert.NoError(t, err)
- assert.Equal(t, diffMetrics, metrics)
-}
diff --git a/golden/go/diffstore/mem_diffstore.go b/golden/go/diffstore/mem_diffstore.go
index 2e51fdb..9452542 100644
--- a/golden/go/diffstore/mem_diffstore.go
+++ b/golden/go/diffstore/mem_diffstore.go
@@ -11,11 +11,16 @@
"sync"
"go.skia.org/infra/go/fileutil"
+ "go.skia.org/infra/go/metrics2"
"go.skia.org/infra/go/rtcache"
"go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/util"
"go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/diffstore/common"
+ "go.skia.org/infra/golden/go/diffstore/mapper"
+ "go.skia.org/infra/golden/go/diffstore/metricsstore"
+ "go.skia.org/infra/golden/go/diffstore/metricsstore/bolt_metricsstore"
"go.skia.org/infra/golden/go/types"
)
@@ -60,13 +65,13 @@
imgLoader *ImageLoader
// metricsStore persists diff metrics.
- metricsStore *metricsStore
+ metricsStore metricsstore.MetricsStore
// wg is used to synchronize background operations like saving files. Used for testing.
wg sync.WaitGroup
// mapper contains various functions for creating image IDs, paths and diff metrics.
- mapper DiffStoreMapper
+ mapper mapper.Mapper
// maxGoRoutinesCh is a buffered channel that is used to limit the number of goroutines for diffing.
maxGoRoutinesCh chan bool
@@ -79,7 +84,7 @@
// If diffFn is not specified, the diff.DefaultDiffFn will be used. If codec is
// not specified, a JSON codec for the diff.DiffMetrics struct will be used.
// If mapper is not specified, GoldIDPathMapper will be used.
-func NewMemDiffStore(client *http.Client, baseDir string, gsBucketNames []string, gsImageBaseDir string, gigs int, mapper DiffStoreMapper) (diff.DiffStore, error) {
+func NewMemDiffStore(client *http.Client, baseDir string, gsBucketNames []string, gsImageBaseDir string, gigs int, m mapper.Mapper) (diff.DiffStore, error) {
imageCacheCount, diffCacheCount := getCacheCounts(gigs)
imgPath := filepath.Join(baseDir, DEFAULT_IMG_DIR_NAME)
@@ -95,12 +100,12 @@
}
// Set up image retrieval, caching and serving.
- imgLoader, err := NewImgLoader(client, baseDir, imgDir, gsBucketNames, gsImageBaseDir, imageCacheCount, mapper)
+ imgLoader, err := NewImgLoader(client, baseDir, imgDir, gsBucketNames, gsImageBaseDir, imageCacheCount, m)
if err != nil {
return nil, skerr.Fmt("Could not create img loader %s", err)
}
- mStore, err := newMetricsStore(baseDir, mapper, mapper)
+ mStore, err := bolt_metricsstore.New(baseDir, m)
if err != nil {
return nil, skerr.Fmt("Could not create metrics store %s", err)
}
@@ -110,7 +115,7 @@
localDiffDir: difDir,
imgLoader: imgLoader,
metricsStore: mStore,
- mapper: mapper,
+ mapper: m,
maxGoRoutinesCh: make(chan bool, maxGoRoutines),
}
@@ -120,13 +125,6 @@
return ret, nil
}
-// TODO(stephana): Remove the conversion below once all caches are fixed.
-
-// ConvertLegacy converts the legacy metrics cache to the new format in the background.
-func (d *MemDiffStore) ConvertLegacy() {
- d.metricsStore.convertDatabaseFromLegacy()
-}
-
// WarmDigests fetches images based on the given list of digests. It does
// not cache the images but makes sure they are downloaded from GCS.
func (d *MemDiffStore) WarmDigests(priority int64, digests types.DigestSlice, sync bool) {
@@ -146,7 +144,7 @@
// with varying priority (ignored vs "regular") we can call this multiple times.
func (d *MemDiffStore) WarmDiffs(priority int64, leftDigests types.DigestSlice, rightDigests types.DigestSlice) {
priority = rtcache.PriorityTimeCombined(priority)
- diffIDs := d.getDiffIds(leftDigests, rightDigests)
+ diffIDs := getDiffIds(leftDigests, rightDigests)
sklog.Infof("Warming %d diffs", len(diffIDs))
d.wg.Add(len(diffIDs))
for _, id := range diffIDs {
@@ -189,7 +187,7 @@
wg.Done()
<-d.maxGoRoutinesCh
}()
- id := d.mapper.DiffID(mainDigest, right)
+ id := mapper.DiffID(mainDigest, right)
ret, err := d.diffMetricsCache.Get(priority, id)
if err != nil {
sklog.Errorf("Unable to calculate diff for %s. Got error: %s", id, err)
@@ -207,7 +205,7 @@
// UnavailableDigests implements the DiffStore interface.
func (m *MemDiffStore) UnavailableDigests() map[types.Digest]*diff.DigestFailure {
- return m.imgLoader.failureStore.unavailableDigests()
+ return m.imgLoader.failureStore.UnavailableDigests()
}
// PurgeDigests implements the DiffStore interface.
@@ -231,18 +229,18 @@
}
removeKeys := make([]string, 0, len(digests))
for _, key := range m.diffMetricsCache.Keys() {
- d1, d2 := m.mapper.SplitDiffID(key)
+ d1, d2 := mapper.SplitDiffID(key)
if digestSet[d1] || digestSet[d2] {
removeKeys = append(removeKeys, key)
}
}
m.diffMetricsCache.Remove(removeKeys)
- if err := m.metricsStore.purgeDiffMetrics(digests); err != nil {
+ if err := m.metricsStore.PurgeDiffMetrics(digests); err != nil {
return err
}
- return m.imgLoader.failureStore.purgeDigestFailures(digests)
+ return m.imgLoader.failureStore.PurgeDigestFailures(digests)
}
// ImageHandler implements the DiffStore interface.
@@ -252,7 +250,7 @@
return nil, fmt.Errorf("Unable to get abs path of %s. Got error: %s", m.baseDir, err)
}
- dotExt := "." + IMG_EXTENSION
+ dotExt := "." + common.IMG_EXTENSION
// Setup the file server and define the handler function.
fileServer := http.FileServer(http.Dir(absPath))
@@ -284,7 +282,7 @@
var localRelPath string
if dir == DEFAULT_IMG_DIR_NAME {
// Validate the requested image ID.
- if !m.mapper.IsValidImgID(imgID) {
+ if !mapper.IsValidImgID(imgID) {
noCacheNotFound(w, r)
return
}
@@ -302,15 +300,15 @@
// Wait until the images are written to disk.
pendingWrites.Wait()
}
- localRelPath, _, _ = m.mapper.ImagePaths(imgDigest)
+ localRelPath, _ = m.mapper.ImagePaths(imgDigest)
} else {
// Validate the requested diff image ID.
- if !m.mapper.IsValidDiffImgID(imgID) {
+ if !mapper.IsValidDiffImgID(imgID) {
noCacheNotFound(w, r)
return
}
- localRelPath = m.mapper.DiffPath(m.mapper.SplitDiffID(imgID))
+ localRelPath = m.mapper.DiffPath(mapper.SplitDiffID(imgID))
}
// Rewrite the path to include the mapper's custom local path construction
@@ -336,10 +334,11 @@
// diffMetricsWorker calculates the diff if it's not in the cache.
func (d *MemDiffStore) diffMetricsWorker(priority int64, id string) (interface{}, error) {
- leftDigest, rightDigest := d.mapper.SplitDiffID(id)
+ defer metrics2.FuncTimer().Stop()
+ leftDigest, rightDigest := mapper.SplitDiffID(id)
// Load it from disk cache if necessary.
- if dm, err := d.metricsStore.loadDiffMetrics(id); err != nil {
+ if dm, err := d.metricsStore.LoadDiffMetrics(id); err != nil {
sklog.Errorf("Error trying to load diff metric: %s", err)
} else if dm != nil {
return dm, nil
@@ -358,7 +357,7 @@
// Encode the result image and save it to disk. If encoding causes an error
// we return an error.
var buf bytes.Buffer
- if err = encodeImg(&buf, diffImg); err != nil {
+ if err = common.EncodeImg(&buf, diffImg); err != nil {
return nil, err
}
@@ -376,7 +375,7 @@
d.wg.Done()
<-d.maxGoRoutinesCh
}()
- if err := d.metricsStore.saveDiffMetrics(diffID, diffMetrics); err != nil {
+ if err := d.metricsStore.SaveDiffMetrics(diffID, diffMetrics); err != nil {
sklog.Errorf("Error saving diff metric: %s", err)
}
}()
@@ -390,7 +389,7 @@
// Get the local file path using the mapper and save the diff image there.
localDiffPath := d.mapper.DiffPath(leftDigest, rightDigest)
- if err := saveFilePath(filepath.Join(d.localDiffDir, localDiffPath), bytes.NewBuffer(imgBytes)); err != nil {
+ if err := common.SaveFilePath(filepath.Join(d.localDiffDir, localDiffPath), bytes.NewBuffer(imgBytes)); err != nil {
sklog.Error(err)
}
}()
@@ -398,13 +397,13 @@
// Returns all combinations of leftDigests and rightDigests using the given
// DiffID function of the DiffStore's mapper.
-func (d *MemDiffStore) getDiffIds(leftDigests, rightDigests types.DigestSlice) []string {
+func getDiffIds(leftDigests, rightDigests types.DigestSlice) []string {
// reminder: diffIds are "digest1:digest2"
diffIDsSet := make(util.StringSet, len(leftDigests)*len(rightDigests))
for _, left := range leftDigests {
for _, right := range rightDigests {
if left != right {
- diffIDsSet[d.mapper.DiffID(left, right)] = true
+ diffIDsSet[mapper.DiffID(left, right)] = true
}
}
}
diff --git a/golden/go/diffstore/diffstore_test.go b/golden/go/diffstore/mem_diffstore_test.go
similarity index 60%
rename from golden/go/diffstore/diffstore_test.go
rename to golden/go/diffstore/mem_diffstore_test.go
index 69711de..bf17dce 100644
--- a/golden/go/diffstore/diffstore_test.go
+++ b/golden/go/diffstore/mem_diffstore_test.go
@@ -2,9 +2,6 @@
import (
"fmt"
- "image"
- "net"
- "net/http/httptest"
"path"
"path/filepath"
"sort"
@@ -17,8 +14,10 @@
"go.skia.org/infra/go/tiling"
"go.skia.org/infra/go/timer"
"go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/diffstore/mapper"
+ "go.skia.org/infra/golden/go/diffstore/mapper/disk_mapper"
+ d_utils "go.skia.org/infra/golden/go/diffstore/testutils"
"go.skia.org/infra/golden/go/types"
- "google.golang.org/grpc"
)
const (
@@ -34,96 +33,17 @@
// Get a small tile and get them cached.
w, cleanup := testutils.TempDir(t)
defer cleanup()
- baseDir := path.Join(w, TEST_DATA_BASE_DIR+"-diffstore")
- client, tile := getSetupAndTile(t, baseDir)
+ baseDir := path.Join(w, d_utils.TEST_DATA_BASE_DIR+"-diffstore")
+ client, tile := d_utils.GetSetupAndTile(t, baseDir)
- mapper := NewGoldDiffStoreMapper(&diff.DiffMetrics{})
- diffStore, err := NewMemDiffStore(client, baseDir, []string{TEST_GCS_BUCKET_NAME}, TEST_GCS_IMAGE_DIR, 10, mapper)
+ m := disk_mapper.New(&diff.DiffMetrics{})
+ diffStore, err := NewMemDiffStore(client, baseDir, []string{d_utils.TEST_GCS_BUCKET_NAME}, d_utils.TEST_GCS_IMAGE_DIR, 10, m)
assert.NoError(t, err)
memDiffStore := diffStore.(*MemDiffStore)
testDiffStore(t, tile, baseDir, diffStore, memDiffStore)
}
-type DummyDiffStoreMapper struct {
- GoldDiffStoreMapper
-}
-
-func (d DummyDiffStoreMapper) DiffFn(leftImg *image.NRGBA, rightImg *image.NRGBA) (interface{}, *image.NRGBA) {
- return 42, nil
-}
-
-func TestDiffFn(t *testing.T) {
- unittest.LargeTest(t)
-
- w, cleanup := testutils.TempDir(t)
- defer cleanup()
- baseDir := path.Join(w, TEST_DATA_BASE_DIR+"-difffn")
- client, _ := getSetupAndTile(t, baseDir)
-
- // Instantiate a new MemDiffStore with the DummyDiffFn.
- mapper := DummyDiffStoreMapper{GoldDiffStoreMapper: NewGoldDiffStoreMapper(&diff.DiffMetrics{}).(GoldDiffStoreMapper)}
- diffStore, err := NewMemDiffStore(client, baseDir, []string{TEST_GCS_BUCKET_NAME}, TEST_GCS_IMAGE_DIR, 10, mapper)
-
- assert.NoError(t, err)
- memDiffStore := diffStore.(*MemDiffStore)
- img1 := image.NewNRGBA(image.Rect(1, 2, 3, 4))
- img2 := image.NewNRGBA(image.Rect(9, 8, 7, 6))
-
- // Check that proper values are returned by the diff function.
- diffMetrics, diffImg := memDiffStore.mapper.DiffFn(img1, img2)
- assert.Equal(t, 42, diffMetrics)
- assert.Nil(t, diffImg)
-}
-
-func TestNetDiffStore(t *testing.T) {
- unittest.LargeTest(t)
-
- w, cleanup := testutils.TempDir(t)
- defer cleanup()
- baseDir := path.Join(w, TEST_DATA_BASE_DIR+"-netdiffstore")
- client, tile := getSetupAndTile(t, baseDir)
-
- mapper := NewGoldDiffStoreMapper(&diff.DiffMetrics{})
- memDiffStore, err := NewMemDiffStore(client, baseDir, []string{TEST_GCS_BUCKET_NAME}, TEST_GCS_IMAGE_DIR, 10, mapper)
- assert.NoError(t, err)
-
- // Start the server that wraps around the MemDiffStore.
- codec := MetricMapCodec{}
- serverImpl := NewDiffServiceServer(memDiffStore, codec)
- lis, err := net.Listen("tcp", "localhost:0")
- assert.NoError(t, err)
-
- // Start the grpc server.
- server := grpc.NewServer()
- RegisterDiffServiceServer(server, serverImpl)
- go func() {
- _ = server.Serve(lis)
- }()
- defer server.Stop()
-
- // Start the http server.
- imgHandler, err := memDiffStore.ImageHandler(IMAGE_URL_PREFIX)
- assert.NoError(t, err)
-
- httpServer := httptest.NewServer(imgHandler)
- defer func() { httpServer.Close() }()
-
- // Create the NetDiffStore.
- addr := lis.Addr().String()
- conn, err := grpc.Dial(addr, grpc.WithInsecure())
- assert.NoError(t, err)
- defer func() {
- assert.NoError(t, conn.Close())
- }()
-
- netDiffStore, err := NewNetDiffStore(conn, httpServer.Listener.Addr().String(), codec)
- assert.NoError(t, err)
-
- // run tests against it.
- testDiffStore(t, tile, baseDir, netDiffStore, memDiffStore.(*MemDiffStore))
-}
-
func testDiffStore(t *testing.T, tile *tiling.Tile, baseDir string, diffStore diff.DiffStore, memDiffStore *MemDiffStore) {
// Pick the test with highest number of digests.
byName := map[types.TestName]types.DigestSet{}
@@ -159,7 +79,7 @@
for _, d1 := range digests {
for _, d2 := range digests {
if d1 != d2 {
- id := memDiffStore.mapper.DiffID(d1, d2)
+ id := mapper.DiffID(d1, d2)
diffIDs = append(diffIDs, id)
assert.True(t, memDiffStore.diffMetricsCache.Contains(id))
}
@@ -176,8 +96,8 @@
// Load the diff from disk and compare.
for twoDigest, dr := range found {
- id := memDiffStore.mapper.DiffID(oneDigest, twoDigest)
- loadedDr, err := memDiffStore.metricsStore.loadDiffMetrics(id)
+ id := mapper.DiffID(oneDigest, twoDigest)
+ loadedDr, err := memDiffStore.metricsStore.LoadDiffMetrics(id)
assert.NoError(t, err)
assert.Equal(t, dr, loadedDr, "Comparing: %s", id)
}
@@ -212,9 +132,74 @@
}
}
-type DummyDiffMetrics struct {
- NumDiffPixels int
- PercentDiffPixels float32
+func TestFailureHandling(t *testing.T) {
+ unittest.MediumTest(t)
+
+ // Get a small tile and get them cached.
+ w, cleanup := testutils.TempDir(t)
+ defer cleanup()
+ baseDir := path.Join(w, d_utils.TEST_DATA_BASE_DIR+"-diffstore-failure")
+ client, tile := d_utils.GetSetupAndTile(t, baseDir)
+
+ m := disk_mapper.New(&diff.DiffMetrics{})
+ diffStore, err := NewMemDiffStore(client, baseDir, []string{d_utils.TEST_GCS_BUCKET_NAME}, d_utils.TEST_GCS_IMAGE_DIR, 10, m)
+ assert.NoError(t, err)
+
+ validDigestSet := types.DigestSet{}
+ for _, trace := range tile.Traces {
+ gTrace := trace.(*types.GoldenTrace)
+ validDigestSet.AddLists(gTrace.Digests)
+ }
+ delete(validDigestSet, types.MISSING_DIGEST)
+
+ invalidDigest_1 := types.Digest("invaliddigest1")
+ invalidDigest_2 := types.Digest("invaliddigest2")
+
+ validDigests := validDigestSet.Keys()
+ mainDigest := validDigests[0]
+ diffDigests := append(validDigests[1:6], invalidDigest_1, invalidDigest_2)
+
+ diffs, err := diffStore.Get(diff.PRIORITY_NOW, mainDigest, diffDigests)
+ assert.NoError(t, err)
+ assert.Equal(t, len(diffDigests)-2, len(diffs))
+
+ unavailableDigests := diffStore.UnavailableDigests()
+ assert.Equal(t, 2, len(unavailableDigests))
+ assert.NotNil(t, unavailableDigests[invalidDigest_1])
+ assert.NotNil(t, unavailableDigests[invalidDigest_2])
+
+ assert.NoError(t, diffStore.PurgeDigests(types.DigestSlice{invalidDigest_1, invalidDigest_2}, true))
+ unavailableDigests = diffStore.UnavailableDigests()
+ assert.Equal(t, 0, len(unavailableDigests))
+}
+
+func TestCodec(t *testing.T) {
+ unittest.MediumTest(t)
+
+ w, cleanup := testutils.TempDir(t)
+ defer cleanup()
+ baseDir := path.Join(w, d_utils.TEST_DATA_BASE_DIR+"-codec")
+ client, _ := d_utils.GetSetupAndTile(t, baseDir)
+
+ // Instantiate a new MemDiffStore with a codec for the test struct defined above.
+ m := disk_mapper.New(&d_utils.DummyDiffMetrics{})
+ diffStore, err := NewMemDiffStore(client, baseDir, []string{d_utils.TEST_GCS_BUCKET_NAME}, d_utils.TEST_GCS_IMAGE_DIR, 10, m)
+ assert.NoError(t, err)
+ memDiffStore := diffStore.(*MemDiffStore)
+
+ diffID := mapper.DiffID(types.Digest("abc"), types.Digest("def"))
+ diffMetrics := &d_utils.DummyDiffMetrics{
+ NumDiffPixels: 100,
+ PercentDiffPixels: 0.5,
+ }
+ err = memDiffStore.metricsStore.SaveDiffMetrics(diffID, diffMetrics)
+ assert.NoError(t, err)
+
+ // Verify the returned diff metrics object has the same type and same contents
+ // as the object that was saved to the metricsStore.
+ metrics, err := memDiffStore.metricsStore.LoadDiffMetrics(diffID)
+ assert.NoError(t, err)
+ assert.Equal(t, diffMetrics, metrics)
}
// Allows for sorting slices of digests by the length (longer slices first)
diff --git a/golden/go/diffstore/metrics_store.go b/golden/go/diffstore/metricsstore/bolt_metricsstore/bolt_metricsstore.go
similarity index 69%
rename from golden/go/diffstore/metrics_store.go
rename to golden/go/diffstore/metricsstore/bolt_metricsstore/bolt_metricsstore.go
index 8c7edb7..331cb08 100644
--- a/golden/go/diffstore/metrics_store.go
+++ b/golden/go/diffstore/metricsstore/bolt_metricsstore/bolt_metricsstore.go
@@ -1,4 +1,4 @@
-package diffstore
+package bolt_metricsstore
import (
"encoding/json"
@@ -10,6 +10,8 @@
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/util"
"go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/diffstore/common"
+ "go.skia.org/infra/golden/go/diffstore/mapper"
"go.skia.org/infra/golden/go/types"
)
@@ -26,18 +28,14 @@
metricsRecIndices = []string{METRICS_DIGEST_INDEX}
)
-// metricsStore stores diff metrics on disk.
-type metricsStore struct {
+// BoltImpl stores diff metrics on disk.
+type BoltImpl struct {
// store stores the diff metrics in a boltdb database.
store *boltutil.IndexedBucket
// codec is used to encode/decode the DiffMetrics field of a metricsRec struct
codec util.LRUCodec
- // TODO(stephana): Remove mapper field once we don't need the legacy code anymore.
- // mapper is an instance of DiffStoreMapper.
- mapper DiffStoreMapper
-
// factory acts as the codec for metrics and is used to create instances of metricsRec.
factory *metricsRecFactory
}
@@ -89,9 +87,9 @@
return ret, nil
}
-// newMetricsStore returns a new instance of metricsStore.
-func newMetricsStore(baseDir string, mapper DiffStoreMapper, codec util.LRUCodec) (*metricsStore, error) {
- db, err := openBoltDB(baseDir, METRICSDB_NAME+".db")
+// New returns a new instance of BoltImpl.
+func New(baseDir string, codec util.LRUCodec) (*BoltImpl, error) {
+ db, err := common.OpenBoltDB(baseDir, METRICSDB_NAME+".db")
if err != nil {
return nil, err
}
@@ -117,16 +115,15 @@
return nil, err
}
- return &metricsStore{
+ return &BoltImpl{
store: store,
codec: codec,
- mapper: mapper,
factory: factoryCodec,
}, nil
}
-// loadDiffMetrics loads diff metrics from disk.
-func (m *metricsStore) loadDiffMetrics(id string) (interface{}, error) {
+// LoadDiffMetrics loads diff metrics from disk.
+func (m *BoltImpl) LoadDiffMetrics(id string) (interface{}, error) {
recs, err := m.store.Read([]string{id})
if err != nil {
return nil, err
@@ -153,8 +150,8 @@
return diffMetrics, nil
}
-// saveDiffMetrics stores diff metrics to disk.
-func (m *metricsStore) saveDiffMetrics(id string, diffMetrics interface{}) error {
+// SaveDiffMetrics stores diff metrics to disk.
+func (m *BoltImpl) SaveDiffMetrics(id string, diffMetrics interface{}) error {
// Serialize the diffMetrics.
bytes, err := m.codec.Encode(diffMetrics)
if err != nil {
@@ -169,10 +166,10 @@
return m.store.Insert([]boltutil.Record{rec})
}
-// purgeDiffMetrics removes all diff metrics based on specific digests.
-func (m *metricsStore) purgeDiffMetrics(digests types.DigestSlice) error {
+// PurgeDiffMetrics removes all diff metrics based on specific digests.
+func (m *BoltImpl) PurgeDiffMetrics(digests types.DigestSlice) error {
updateFn := func(tx *bolt.Tx) error {
- metricIDMap, err := m.store.ReadIndexTx(tx, METRICS_DIGEST_INDEX, asStrings(digests))
+ metricIDMap, err := m.store.ReadIndexTx(tx, METRICS_DIGEST_INDEX, common.AsStrings(digests))
if err != nil {
return err
}
@@ -195,7 +192,7 @@
*diff.DiffMetrics
}
-func (m *metricsStore) fixLegacyRecord(id string, recBytes []byte) *diff.DiffMetrics {
+func (m *BoltImpl) fixLegacyRecord(id string, recBytes []byte) *diff.DiffMetrics {
var newRec *diff.DiffMetrics = nil
// If we have data bytes then we just have to deserialize.
if len(recBytes) > 0 {
@@ -231,7 +228,7 @@
newRec = legRec.DiffMetrics
}
// Regenerate the diffID to filter out the old format.
- newID := m.mapper.DiffID(m.mapper.SplitDiffID(id))
+ newID := mapper.DiffID(mapper.SplitDiffID(id))
// Write the new record to the database in the background.
go func() {
@@ -241,7 +238,7 @@
}
}()
- if err := m.saveDiffMetrics(newID, newRec); err != nil {
+ if err := m.SaveDiffMetrics(newID, newRec); err != nil {
sklog.Errorf("Error writing legacy record to DB: %s", err)
}
@@ -257,58 +254,3 @@
return newRec
}
-
-// convertDatabaseFromLegacy iterates over the entire database in the background
-// and loads every entry, implicitly forcing a conversion to the new serialization format.
-func (m *metricsStore) convertDatabaseFromLegacy() {
- go func() {
- defer func() {
- if r := recover(); r != nil {
- sklog.Errorf("Recovered panic: %s", r)
- }
- }()
-
- ids, err := m.listIDs()
- if err != nil {
- sklog.Errorf("Unable to get the database ids. Got error: %s", err)
- }
-
- sklog.Infof("Processing %d diffmetric records.", len(ids))
-
- for _, id := range ids {
- // The call to loadDiffMetrics will also convert a legacy record to the new record if necessary.
- if _, err := m.loadDiffMetrics(id); err != nil {
- sklog.Errorf("Error trying to convert legacy record: %s", err)
- }
- }
- sklog.Infof("Legacy conversion: Loaded %d records.", len(ids))
- if err := m.store.ReIndex(); err != nil {
- sklog.Errorf("Error re-indexing data store: %s", err)
- }
- sklog.Infof("Legacy conversion completed.")
- }()
-}
-
-// listIDs returns a slice with all the ids in the database.
-func (m *metricsStore) listIDs() ([]string, error) {
- ret := []string{}
- viewFn := func(tx *bolt.Tx) error {
- b := tx.Bucket([]byte(METRICSDB_NAME))
- if b == nil {
- return nil
- }
-
- ret = make([]string, 0, b.Stats().KeyN)
- c := b.Cursor()
-
- for k, _ := c.First(); k != nil; k, _ = c.Next() {
- ret = append(ret, string(append([]byte(nil), k...)))
- }
- return nil
- }
-
- if err := m.store.DB.View(viewFn); err != nil {
- return nil, err
- }
- return ret, nil
-}
diff --git a/golden/go/diffstore/metricsstore/bolt_metricsstore/bolt_metricsstore_test.go b/golden/go/diffstore/metricsstore/bolt_metricsstore/bolt_metricsstore_test.go
new file mode 100644
index 0000000..31d1e64
--- /dev/null
+++ b/golden/go/diffstore/metricsstore/bolt_metricsstore/bolt_metricsstore_test.go
@@ -0,0 +1,38 @@
+package bolt_metricsstore
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "go.skia.org/infra/go/testutils"
+ "go.skia.org/infra/go/testutils/unittest"
+ "go.skia.org/infra/go/util"
+ d_utils "go.skia.org/infra/golden/go/diffstore/testutils"
+)
+
+func TestAddGet(t *testing.T) {
+ unittest.MediumTest(t)
+
+ w, cleanup := testutils.TempDir(t)
+ defer cleanup()
+
+ ms, err := New(w, util.JSONCodec(d_utils.DummyDiffMetrics{}))
+ assert.NoError(t, err)
+
+ id := "abc-def"
+
+ dm, err := ms.LoadDiffMetrics(id)
+ assert.NoError(t, err)
+ assert.Nil(t, dm)
+
+ expected := &d_utils.DummyDiffMetrics{
+ NumDiffPixels: 3,
+ PercentDiffPixels: 0.3,
+ }
+
+ assert.NoError(t, ms.SaveDiffMetrics(id, expected))
+
+ dm, err = ms.LoadDiffMetrics(id)
+ assert.NoError(t, err)
+ assert.Equal(t, expected, dm)
+}
diff --git a/golden/go/diffstore/metricsstore/metricsstore.go b/golden/go/diffstore/metricsstore/metricsstore.go
new file mode 100644
index 0000000..8405832
--- /dev/null
+++ b/golden/go/diffstore/metricsstore/metricsstore.go
@@ -0,0 +1,14 @@
+package metricsstore
+
+import "go.skia.org/infra/golden/go/types"
+
+type MetricsStore interface {
+ // PurgeDiffMetrics removes all diff metrics based on specific digests.
+ PurgeDiffMetrics(digests types.DigestSlice) error
+
+ // SaveDiffMetrics stores diff metrics.
+ SaveDiffMetrics(id string, diffMetrics interface{}) error
+
+ // LoadDiffMetrics loads diff metrics from disk.
+ LoadDiffMetrics(id string) (interface{}, error)
+}
diff --git a/golden/go/diffstore/net_diffstore.go b/golden/go/diffstore/net_diffstore.go
index 6c5d230..e0fa562 100644
--- a/golden/go/diffstore/net_diffstore.go
+++ b/golden/go/diffstore/net_diffstore.go
@@ -10,6 +10,7 @@
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/util"
"go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/diffstore/common"
"go.skia.org/infra/golden/go/types"
"google.golang.org/grpc"
)
@@ -44,7 +45,7 @@
// Get, see the diff.DiffStore interface.
func (n *NetDiffStore) Get(priority int64, mainDigest types.Digest, rightDigests types.DigestSlice) (map[types.Digest]interface{}, error) {
- req := &GetDiffsRequest{Priority: priority, MainDigest: string(mainDigest), RightDigests: asStrings(rightDigests)}
+ req := &GetDiffsRequest{Priority: priority, MainDigest: string(mainDigest), RightDigests: common.AsStrings(rightDigests)}
resp, err := n.serviceClient.GetDiffs(context.Background(), req)
if err != nil {
return nil, err
@@ -74,7 +75,7 @@
// WarmDigests, see the diff.DiffStore interface.
func (n *NetDiffStore) WarmDigests(priority int64, digests types.DigestSlice, sync bool) {
- req := &WarmDigestsRequest{Priority: priority, Digests: asStrings(digests), Sync: sync}
+ req := &WarmDigestsRequest{Priority: priority, Digests: common.AsStrings(digests), Sync: sync}
_, err := n.serviceClient.WarmDigests(context.Background(), req)
if err != nil {
sklog.Errorf("Error warming digests: %s", err)
@@ -83,7 +84,7 @@
// WarmDiffs, see the diff.DiffStore interface.
func (n *NetDiffStore) WarmDiffs(priority int64, leftDigests types.DigestSlice, rightDigests types.DigestSlice) {
- req := &WarmDiffsRequest{Priority: priority, LeftDigests: asStrings(leftDigests), RightDigests: asStrings(rightDigests)}
+ req := &WarmDiffsRequest{Priority: priority, LeftDigests: common.AsStrings(leftDigests), RightDigests: common.AsStrings(rightDigests)}
_, err := n.serviceClient.WarmDiffs(context.Background(), req)
if err != nil {
sklog.Errorf("Error warming diffs: %s", err)
@@ -110,7 +111,7 @@
// PurgeDigests, see the diff.DiffStore interface.
func (n *NetDiffStore) PurgeDigests(digests types.DigestSlice, purgeGCS bool) error {
- req := &PurgeDigestsRequest{Digests: asStrings(digests), PurgeGCS: purgeGCS}
+ req := &PurgeDigestsRequest{Digests: common.AsStrings(digests), PurgeGCS: purgeGCS}
_, err := n.serviceClient.PurgeDigests(context.Background(), req)
if err != nil {
return fmt.Errorf("Error purging digests: %s", err)
diff --git a/golden/go/diffstore/net_diffstore_test.go b/golden/go/diffstore/net_diffstore_test.go
new file mode 100644
index 0000000..4fac853
--- /dev/null
+++ b/golden/go/diffstore/net_diffstore_test.go
@@ -0,0 +1,64 @@
+package diffstore
+
+import (
+ "net"
+ "net/http/httptest"
+ "path"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "go.skia.org/infra/go/testutils"
+ "go.skia.org/infra/go/testutils/unittest"
+ "go.skia.org/infra/golden/go/diff"
+ "go.skia.org/infra/golden/go/diffstore/mapper/disk_mapper"
+ d_utils "go.skia.org/infra/golden/go/diffstore/testutils"
+ "google.golang.org/grpc"
+)
+
+func TestNetDiffStore(t *testing.T) {
+ unittest.LargeTest(t)
+
+ w, cleanup := testutils.TempDir(t)
+ defer cleanup()
+ baseDir := path.Join(w, d_utils.TEST_DATA_BASE_DIR+"-netdiffstore")
+ client, tile := d_utils.GetSetupAndTile(t, baseDir)
+
+ m := disk_mapper.New(&diff.DiffMetrics{})
+ memDiffStore, err := NewMemDiffStore(client, baseDir, []string{d_utils.TEST_GCS_BUCKET_NAME}, d_utils.TEST_GCS_IMAGE_DIR, 10, m)
+ assert.NoError(t, err)
+
+ // Start the server that wraps around the MemDiffStore.
+ codec := MetricMapCodec{}
+ serverImpl := NewDiffServiceServer(memDiffStore, codec)
+ lis, err := net.Listen("tcp", "localhost:0")
+ assert.NoError(t, err)
+
+ // Start the grpc server.
+ server := grpc.NewServer()
+ RegisterDiffServiceServer(server, serverImpl)
+ go func() {
+ _ = server.Serve(lis)
+ }()
+ defer server.Stop()
+
+ // Start the http server.
+ imgHandler, err := memDiffStore.ImageHandler(IMAGE_URL_PREFIX)
+ assert.NoError(t, err)
+
+ httpServer := httptest.NewServer(imgHandler)
+ defer func() { httpServer.Close() }()
+
+ // Create the NetDiffStore.
+ addr := lis.Addr().String()
+ conn, err := grpc.Dial(addr, grpc.WithInsecure())
+ assert.NoError(t, err)
+ defer func() {
+ assert.NoError(t, conn.Close())
+ }()
+
+ netDiffStore, err := NewNetDiffStore(conn, httpServer.Listener.Addr().String(), codec)
+ assert.NoError(t, err)
+
+ // run tests against it.
+ testDiffStore(t, tile, baseDir, netDiffStore, memDiffStore.(*MemDiffStore))
+}
diff --git a/golden/go/diffstore/shared_test.go b/golden/go/diffstore/testutils/testutils_diffstore.go
similarity index 87%
rename from golden/go/diffstore/shared_test.go
rename to golden/go/diffstore/testutils/testutils_diffstore.go
index 18d96d1..8c7e949 100644
--- a/golden/go/diffstore/shared_test.go
+++ b/golden/go/diffstore/testutils/testutils_diffstore.go
@@ -1,4 +1,4 @@
-package diffstore
+package testutils
import (
"net/http"
@@ -34,7 +34,12 @@
TEST_PATH_IMG_1 = "gold-testdata/filediffstore-testdata/10552995703607727960.png"
)
-func getSetupAndTile(t assert.TestingT, baseDir string) (*http.Client, *tiling.Tile) {
+type DummyDiffMetrics struct {
+ NumDiffPixels int
+ PercentDiffPixels float32
+}
+
+func GetSetupAndTile(t assert.TestingT, baseDir string) (*http.Client, *tiling.Tile) {
testDataPath := filepath.Join(baseDir, TEST_DATA_FILE_NAME)
assert.NoError(t, gcs_testutils.DownloadTestDataFile(t, TEST_DATA_STORAGE_BUCKET, TEST_DATA_STORAGE_PATH, testDataPath))
diff --git a/golden/go/diffstore/types.go b/golden/go/diffstore/types.go
index 037ff91..dd51e32 100644
--- a/golden/go/diffstore/types.go
+++ b/golden/go/diffstore/types.go
@@ -1,47 +1,33 @@
package diffstore
import (
- "image"
+ "encoding/json"
- "go.skia.org/infra/go/util"
+ "go.skia.org/infra/golden/go/diff"
"go.skia.org/infra/golden/go/types"
)
-// DiffStoreMapper is the interface to customize the specific behavior of MemDiffStore.
-// It defines what diff metric is calculated and how to translate image ids and diff
-// ids into paths on the file system and in GCS.
-type DiffStoreMapper interface {
- // LRUCodec defines the Encode and Decode functions to serialize/deserialize
- // instances of the diff metrics returned by the DiffFn function below.
- util.LRUCodec
+// MetricMapCodec implements the util.LRUCodec interface by serializing and
+// deserializing generic diff result structs, instances of map[string]interface{}
+type MetricMapCodec struct{}
- // DiffFn calculates the different between two given images and returns a
- // difference image. The type underlying interface{} is the input and output
- // of the LRUCodec above. It is also what is returned by the Get(...) function
- // of the DiffStore interface.
- DiffFn(*image.NRGBA, *image.NRGBA) (interface{}, *image.NRGBA)
+// See util.LRUCodec interface
+func (m MetricMapCodec) Encode(data interface{}) ([]byte, error) {
+ return json.Marshal(data)
+}
- // Takes two image IDs and returns a unique diff ID.
- // Note: DiffID(a,b) == DiffID(b, a) should hold.
- DiffID(leftImgID, rightImgID types.Digest) string
+// See util.LRUCodec interface
+func (m MetricMapCodec) Decode(byteData []byte) (interface{}, error) {
+ dm := map[types.Digest]*diff.DiffMetrics{}
+ err := json.Unmarshal(byteData, &dm)
+ if err != nil {
+ return nil, err
+ }
- // Inverse function of DiffID.
- // SplitDiffID(DiffID(a,b)) should return (a,b) or (b,a).
- SplitDiffID(diffID string) (types.Digest, types.Digest)
-
- // DiffPath returns the local file path for the diff image of two images.
- // This path is used to store the diff image on disk and serve it over HTTP.
- DiffPath(leftImgID, rightImgID types.Digest) string
-
- // ImagePaths returns the storage paths for a given image ID. The first return
- // value is the local file path used to store the image on disk and serve it
- // over HTTP. The second return value is the storage bucket and the third the
- // path within that bucket.
- ImagePaths(imageID types.Digest) (string, string, string)
-
- // IsValidDiffImgID returns true if the given diffImgID is in the correct format.
- IsValidDiffImgID(diffImgID string) bool
-
- // IsValidImgID returns true if the given imgID is in the correct format.
- IsValidImgID(imgID string) bool
+ // Must make result of deserialization generic in order to propagate
+ ret := make(map[types.Digest]interface{}, len(dm))
+ for k, metric := range dm {
+ ret[k] = metric
+ }
+ return ret, nil
}
diff --git a/golden/go/diffstore/common_test.go b/golden/go/diffstore/types_test.go
similarity index 100%
rename from golden/go/diffstore/common_test.go
rename to golden/go/diffstore/types_test.go
diff --git a/golden/go/digesttools/digesttools.go b/golden/go/digesttools/digesttools.go
index 95bbcce..7410f7a 100644
--- a/golden/go/digesttools/digesttools.go
+++ b/golden/go/digesttools/digesttools.go
@@ -98,7 +98,7 @@
} else {
for digest, diffs := range diffMetrics {
dm := diffs.(*diff.DiffMetrics)
- if delta := combinedDiffMetric(dm.PixelDiffPercent, dm.MaxRGBADiffs); delta < ret.Diff {
+ if delta := diff.CombinedDiffMetric(dm, nil, nil); delta < ret.Diff {
ret.Digest = digest
ret.Diff = delta
ret.DiffPixels = dm.PixelDiffPercent
@@ -111,32 +111,13 @@
// ClosestFromDiffMetrics returns an instance of Closest with the values of the
// given diff.DiffMetrics. The Digest field will be left empty.
-func ClosestFromDiffMetrics(diff *diff.DiffMetrics) *Closest {
+func ClosestFromDiffMetrics(dm *diff.DiffMetrics) *Closest {
return &Closest{
- Diff: combinedDiffMetric(diff.PixelDiffPercent, diff.MaxRGBADiffs),
- DiffPixels: diff.PixelDiffPercent,
- MaxRGBA: diff.MaxRGBADiffs,
+ Diff: diff.CombinedDiffMetric(dm, nil, nil),
+ DiffPixels: dm.PixelDiffPercent,
+ MaxRGBA: dm.MaxRGBADiffs,
}
}
-// combinedDiffMetric returns a value in [0, 1] that represents how large
-// the diff is between two images.
-func combinedDiffMetric(pixelDiffPercent float32, maxRGBA []int) float32 {
- if len(maxRGBA) == 0 {
- return 1.0
- }
- // Turn maxRGBA into a percent by taking the root mean square difference from
- // [0, 0, 0, 0].
- sum := 0.0
- for _, c := range maxRGBA {
- sum += float64(c) * float64(c)
- }
- normalizedRGBA := math.Sqrt(sum/float64(len(maxRGBA))) / 255.0
- // We take the sqrt of (pixelDiffPercent * normalizedRGBA) to straigten out
- // the curve, i.e. think about what a plot of x^2 would look like in the
- // range [0, 1].
- return float32(math.Sqrt(float64(pixelDiffPercent) * normalizedRGBA))
-}
-
// Make sure Impl fulfills the ClosestDiffFinder interface
var _ ClosestDiffFinder = (*Impl)(nil)
diff --git a/golden/go/digesttools/internals_test.go b/golden/go/digesttools/internals_test.go
deleted file mode 100644
index 740d67a..0000000
--- a/golden/go/digesttools/internals_test.go
+++ /dev/null
@@ -1,16 +0,0 @@
-package digesttools
-
-import (
- "math"
- "testing"
-
- assert "github.com/stretchr/testify/require"
- "go.skia.org/infra/go/testutils/unittest"
-)
-
-func TestCombinedDiffMetric(t *testing.T) {
- unittest.SmallTest(t)
- assert.InDelta(t, 1.0, combinedDiffMetric(0.0, []int{}), 0.000001)
- assert.InDelta(t, 1.0, combinedDiffMetric(1.0, []int{255, 255, 255, 255}), 0.000001)
- assert.InDelta(t, math.Sqrt(0.5), combinedDiffMetric(0.5, []int{255, 255, 255, 255}), 0.000001)
-}
diff --git a/golden/go/dsutil/dsutil.go b/golden/go/dsutil/dsutil.go
index 3082900..b895246 100644
--- a/golden/go/dsutil/dsutil.go
+++ b/golden/go/dsutil/dsutil.go
@@ -121,8 +121,7 @@
// The Added and Deleted keys are mutually exclusive, so we can only have to
// check the queried keys for recently added and deleted keys.
- var ret []*datastore.Key
- ret = make([]*datastore.Key, 0, len(r.Added)+len(queried))
+ ret := make([]*datastore.Key, 0, len(r.Added)+len(queried))
ret = append(ret, r.Added...)
for _, k := range queried {
if addMap[k.ID] == nil && delMap[k.ID] == nil {
diff --git a/golden/go/types/expectations.go b/golden/go/types/expectations.go
index 01f13f5..a4c1c97 100644
--- a/golden/go/types/expectations.go
+++ b/golden/go/types/expectations.go
@@ -39,19 +39,6 @@
// the ones provided by the passed in parameter overwrite any existing data.
func (e Expectations) MergeExpectations(other Expectations) {
for testName, digests := range other {
- if _, ok := e[testName]; !ok {
- e[testName] = map[Digest]Label{}
- }
- for digest, label := range digests {
- e[testName][digest] = label
- }
- }
-}
-
-// Update updates the current expectations with the expectations in 'right'. Any existing
-// test_name/digest pair will be overwritten.
-func (e Expectations) Update(right Expectations) {
- for testName, digests := range right {
e.AddDigests(testName, digests)
}
}
@@ -59,9 +46,7 @@
// DeepCopy makes a deep copy of the current expectations/baseline.
func (e Expectations) DeepCopy() Expectations {
ret := make(Expectations, len(e))
- for testName, digests := range e {
- ret.AddDigests(testName, digests)
- }
+ ret.MergeExpectations(e)
return ret
}
