[gold] Performance enhancements for /digests
Wrote two benchmarks to show 1 easy win and 1 hard win
(pre-caching Test() saves ~20ms in GetDigestDetails and
probably less than that elsewhere; If I were to re-write
lots of things to have a slice of traces instead of a
map, it would save about 15ms of overhead on the range)
Implemented a few small improvements, the aformentioned
pre-caching and being more efficient about looking up
if a trace had a given digest.
I hardened the endpoint against garbage data, in that we
bail out sooner if there's an unknown test or digest
(the previous impl did not do the right thing in this case).
The rest is cleanup that I found while scouring through.
In the end, based on my extensive testing (of two random
data samples [1], [2]), the /json/details endpoint response
dropped from ~2 seconds to ~250 milliseconds (on some large
cases, up to 1/3 of that time is gzipping the response, I think).
With that worst-case scenario addressed, I feel confident in
upping the QPS on this endpoint.
[1] https://gold.skia.org/detail?test=blurredclippedcircle&digest=412961706b174f726893ab4a054a658a
[2] https://gold.skia.org/detail?test=hslcolorfilter&digest=daf52d9be7a334fbfbb38e56d51017cd
Bug: skia:9557,skia:9080
Change-Id: Ifaf14917d8fa3fa9b5a22c85b3ab9b2bbef914a4
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/250320
Reviewed-by: Ben Wagner aka dogben <benjaminwagner@google.com>
Commit-Queue: Kevin Lubick <kjlubick@google.com>
diff --git a/golden/go/search/search.go b/golden/go/search/search.go
index b1dcafe..d0a0542 100644
--- a/golden/go/search/search.go
+++ b/golden/go/search/search.go
@@ -15,7 +15,6 @@
"go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/tiling"
- "go.skia.org/infra/go/timer"
"go.skia.org/infra/go/util"
"go.skia.org/infra/golden/go/clstore"
"go.skia.org/infra/golden/go/code_review"
@@ -166,8 +165,16 @@
// GetDigestDetails implements the SearchAPI interface.
func (s *SearchImpl) GetDigestDetails(ctx context.Context, test types.TestName, digest types.Digest) (*frontend.DigestDetails, error) {
defer metrics2.FuncTimer().Stop()
- defer timer.New("GetDigestDetails").Stop()
idx := s.indexSource.GetIndex()
+
+ // Make sure we have valid data, i.e. we know about that test/digest
+ dct := idx.DigestCountsByTest(types.IncludeIgnoredTraces)
+ if digests, ok := dct[test]; !ok {
+ return nil, skerr.Fmt("unknown test %s", test)
+ } else if _, ok := digests[digest]; !ok {
+ return nil, skerr.Fmt("unknown digest %s for test %s", digest, test)
+ }
+
tile := idx.Tile().GetTile(types.IncludeIgnoredTraces)
exp, err := s.getExpectationsFromQuery("", "")
@@ -176,18 +183,14 @@
}
oneInter := newSrIntermediate(test, digest, "", nil, nil)
- // FIXME(kjlubick) Iterating through all the traces is probably slow...
+ byTrace := idx.DigestCountsByTrace(types.IncludeIgnoredTraces)
for traceId, t := range tile.Traces {
gTrace := t.(*types.GoldenTrace)
if gTrace.TestName() != test {
continue
}
-
- for _, val := range gTrace.Digests {
- if val == digest {
- oneInter.add(traceId, t, nil)
- break
- }
+ if _, ok := byTrace[traceId][digest]; ok {
+ oneInter.add(traceId, t, nil)
}
}
@@ -628,7 +631,7 @@
oneTrace := traces[traceID]
tr := &outputTraces[i]
tr.ID = traceID
- tr.Params = oneTrace.Keys
+ tr.Params = oneTrace.Params()
tr.Data = make([]frontend.Point, last+1)
insertNext := last
diff --git a/golden/go/search/search_test.go b/golden/go/search/search_test.go
index b124edf..a686df6 100644
--- a/golden/go/search/search_test.go
+++ b/golden/go/search/search_test.go
@@ -526,6 +526,63 @@
}, details)
}
+func TestDigestDetailsThreeDevicesBadDigest(t *testing.T) {
+ unittest.SmallTest(t)
+
+ const digestWeWantDetailsAbout = types.Digest("invalid digest")
+ const testWeWantDetailsAbout = data.AlphaTest
+
+ mi := &mock_index.IndexSource{}
+ defer mi.AssertExpectations(t)
+
+ fis := makeThreeDevicesIndex()
+ mi.On("GetIndex").Return(fis)
+
+ s := New(nil, nil, mi, nil, nil, everythingPublic)
+
+ _, err := s.GetDigestDetails(context.Background(), testWeWantDetailsAbout, digestWeWantDetailsAbout)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "unknown")
+}
+
+func TestDigestDetailsThreeDevicesBadTest(t *testing.T) {
+ unittest.SmallTest(t)
+
+ const digestWeWantDetailsAbout = data.AlphaGood1Digest
+ const testWeWantDetailsAbout = types.TestName("invalid test")
+
+ mi := &mock_index.IndexSource{}
+ defer mi.AssertExpectations(t)
+
+ fis := makeThreeDevicesIndex()
+ mi.On("GetIndex").Return(fis)
+
+ s := New(nil, nil, mi, nil, nil, everythingPublic)
+
+ _, err := s.GetDigestDetails(context.Background(), testWeWantDetailsAbout, digestWeWantDetailsAbout)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "unknown")
+}
+
+func TestDigestDetailsThreeDevicesBadTestAndDigest(t *testing.T) {
+ unittest.SmallTest(t)
+
+ const digestWeWantDetailsAbout = types.Digest("invalid digest")
+ const testWeWantDetailsAbout = types.TestName("invalid test")
+
+ mi := &mock_index.IndexSource{}
+ defer mi.AssertExpectations(t)
+
+ fis := makeThreeDevicesIndex()
+ mi.On("GetIndex").Return(fis)
+
+ s := New(nil, nil, mi, nil, nil, everythingPublic)
+
+ _, err := s.GetDigestDetails(context.Background(), testWeWantDetailsAbout, digestWeWantDetailsAbout)
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "unknown")
+}
+
var everythingPublic = paramtools.ParamSet{}
// makeThreeDevicesIndex returns a search index corresponding to the three_devices_data
diff --git a/golden/go/search/util.go b/golden/go/search/util.go
index 7e900ee..6b869f3 100644
--- a/golden/go/search/util.go
+++ b/golden/go/search/util.go
@@ -55,7 +55,7 @@
// Check if the query matches.
if tiling.Matches(trace, q.TraceValues) {
fullTr := trace.(*types.GoldenTrace)
- params := fullTr.Keys
+ params := fullTr.Params()
reducedTr := traceView(fullTr)
digests := digestsFromTrace(id, reducedTr, q.Head, lastTraceIdx, digestCountsByTrace)
@@ -134,7 +134,7 @@
ret := func(trace *types.GoldenTrace) *types.GoldenTrace {
return &types.GoldenTrace{
Digests: trace.Digests[startIdx:endIdx],
- Keys: trace.Keys,
+ Keys: trace.Params(),
}
}
diff --git a/golden/go/tracestore/bt_tracestore/bt_tracestore.go b/golden/go/tracestore/bt_tracestore/bt_tracestore.go
index 898538d..b529ab8 100644
--- a/golden/go/tracestore/bt_tracestore/bt_tracestore.go
+++ b/golden/go/tracestore/bt_tracestore/bt_tracestore.go
@@ -352,12 +352,10 @@
// Turn the params into the tiling.TraceId we expect elsewhere.
traceKey := tracestore.TraceIDFromParams(params)
if _, ok := tileTraces[traceKey]; !ok {
- gt := types.NewGoldenTraceN(nCommits)
if opts, ok := options[idx][pair.ID]; ok {
params.Add(opts)
}
-
- gt.Keys = params
+ gt := types.NewGoldenTraceN(nCommits, params)
tileTraces[traceKey] = gt
// Build up the total set of params
@@ -675,7 +673,7 @@
bigtable.ChainFilters(
bigtable.FamilyFilter(traceFamily),
// can be used for local testing to keep RAM usage lower
- // bigtable.RowSampleFilter(0.1),
+ //bigtable.RowSampleFilter(0.1),
bigtable.LatestNFilter(1),
bigtable.CellsPerRowLimitFilter(DefaultTileSize),
),
diff --git a/golden/go/tracestore/bt_tracestore/bt_tracestore_test.go b/golden/go/tracestore/bt_tracestore/bt_tracestore_test.go
index 12037bb..496f313 100644
--- a/golden/go/tracestore/bt_tracestore/bt_tracestore_test.go
+++ b/golden/go/tracestore/bt_tracestore/bt_tracestore_test.go
@@ -8,6 +8,7 @@
"testing"
"time"
+ "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"go.skia.org/infra/go/bt"
@@ -58,10 +59,30 @@
actualTile, actualCommits, err := traceStore.GetTile(ctx, len(commits))
require.NoError(t, err)
- require.Equal(t, data.MakeTestTile(), actualTile)
+ assertTilesEqual(t, data.MakeTestTile(), actualTile)
require.Equal(t, commits, actualCommits)
}
+func assertTilesEqual(t *testing.T, a *tiling.Tile, b *tiling.Tile) {
+ assert.Equal(t, a.ParamSet, b.ParamSet)
+ assert.Equal(t, a.Commits, b.Commits)
+ assert.Equal(t, a.Scale, b.Scale)
+ assert.Equal(t, a.TileIndex, b.TileIndex)
+ // We can't do a naive comparison of the traces because unexported values may not exactly match
+ // and don't care if they do (i.e. the cached values for TestName, Corpus)
+ assert.Equal(t, len(a.Traces), len(b.Traces))
+ for id, tr := range a.Traces {
+ assert.Contains(t, b.Traces, id)
+ gta, ok := tr.(*types.GoldenTrace)
+ require.True(t, ok)
+ gtb, ok := b.Traces[id].(*types.GoldenTrace)
+ require.True(t, ok)
+
+ assert.Equal(t, gta.Keys, gtb.Keys)
+ assert.Equal(t, gta.Digests, gtb.Digests)
+ }
+}
+
func putTestTile(t *testing.T, traceStore tracestore.TraceStore, commits []*tiling.Commit, options bool) {
// This time is an arbitrary point in time
now := time.Date(2019, time.May, 5, 1, 3, 4, 0, time.UTC)
@@ -157,7 +178,7 @@
gt := expectedTile.Traces[data.AnglerAlphaTraceID].(*types.GoldenTrace)
gt.Digests[2] = veryNewDigest
- require.Equal(t, expectedTile, actualTile)
+ assertTilesEqual(t, expectedTile, actualTile)
require.Equal(t, commits, actualCommits)
}
@@ -191,7 +212,7 @@
actualTile, actualCommits, err := traceStore.GetTile(ctx, len(commits))
require.NoError(t, err)
- require.Equal(t, makeTestTileWithOptions(), actualTile)
+ assertTilesEqual(t, makeTestTileWithOptions(), actualTile)
require.Equal(t, commits, actualCommits)
}
@@ -232,7 +253,7 @@
actualTile, actualCommits, err := traceStore.GetTile(ctx, len(commits))
require.NoError(t, err)
- require.Equal(t, data.MakeTestTile(), actualTile)
+ assertTilesEqual(t, data.MakeTestTile(), actualTile)
require.Equal(t, commits, actualCommits)
}
@@ -267,7 +288,7 @@
actualTile, actualCommits, err := traceStore.GetTile(ctx, len(commits))
require.NoError(t, err)
- require.Equal(t, makeTestTileWithOptions(), actualTile)
+ assertTilesEqual(t, makeTestTileWithOptions(), actualTile)
require.Equal(t, commits, actualCommits)
}
@@ -345,7 +366,7 @@
actualTile, actualCommits, err := traceStore.GetTile(ctx, len(commits))
require.NoError(t, err)
- require.Equal(t, data.MakeTestTile(), actualTile)
+ assertTilesEqual(t, data.MakeTestTile(), actualTile)
require.Equal(t, commits, actualCommits)
}
@@ -416,7 +437,7 @@
actualTile, actualCommits, err := traceStore.GetTile(ctx, len(commits))
require.NoError(t, err)
- require.Equal(t, data.MakeTestTile(), actualTile)
+ assertTilesEqual(t, data.MakeTestTile(), actualTile)
require.Equal(t, commits, actualCommits)
}
@@ -568,7 +589,7 @@
}
tile.Commits = commits
- require.Equal(t, tile, actualTile)
+ assertTilesEqual(t, tile, actualTile)
}
// TestBTTraceStoreOverwrite makes sure that options and digests can be overwritten by
@@ -626,7 +647,7 @@
actualTile, actualCommits, err := traceStore.GetTile(ctx, len(commits))
require.NoError(t, err)
- require.Equal(t, makeTestTileWithOptions(), actualTile)
+ assertTilesEqual(t, makeTestTileWithOptions(), actualTile)
require.Equal(t, commits, actualCommits)
}
diff --git a/golden/go/types/complex_tile.go b/golden/go/types/complex_tile.go
index ca1e630..ae0c439 100644
--- a/golden/go/types/complex_tile.go
+++ b/golden/go/types/complex_tile.go
@@ -1,9 +1,6 @@
package types
import (
- "encoding/json"
- "io"
-
"go.skia.org/infra/go/paramtools"
"go.skia.org/infra/go/tiling"
)
@@ -108,53 +105,6 @@
// Make sure ComplexTileImpl fulfills the ComplexTile Interface
var _ ComplexTile = (*ComplexTileImpl)(nil)
-// Same as Tile but instead of Traces we preserve the raw JSON. This is a
-// utility struct that is used to parse a tile where we don't know the
-// Trace type upfront.
-type TileWithRawTraces struct {
- Traces map[tiling.TraceId]json.RawMessage `json:"traces"`
- ParamSet map[string][]string `json:"param_set"`
- Commits []*tiling.Commit `json:"commits"`
- Scale int `json:"scale"`
- TileIndex int `json:"tileIndex"`
-}
-
-// TileFromJson parses a tile that has been serialized to JSON.
-// traceExample has to be an instance of the Trace implementation
-// that needs to be deserialized.
-// Note: Instead of the type switch below we could use reflection
-// to be truly generic, but it makes the code harder to read and
-// currently we only have two types.
-func TileFromJson(r io.Reader, traceExample tiling.Trace) (*tiling.Tile, error) {
- factory := func() tiling.Trace { return NewGoldenTrace() }
-
- // Decode everything, but the traces.
- dec := json.NewDecoder(r)
- var rawTile TileWithRawTraces
- err := dec.Decode(&rawTile)
- if err != nil {
- return nil, err
- }
-
- // Parse the traces.
- traces := map[tiling.TraceId]tiling.Trace{}
- for k, rawJson := range rawTile.Traces {
- newTrace := factory()
- if err = json.Unmarshal(rawJson, newTrace); err != nil {
- return nil, err
- }
- traces[k] = newTrace.(tiling.Trace)
- }
-
- return &tiling.Tile{
- Traces: traces,
- ParamSet: rawTile.ParamSet,
- Commits: rawTile.Commits,
- Scale: rawTile.Scale,
- TileIndex: rawTile.Scale,
- }, nil
-}
-
// TODO(kjlubick): Most (all?) places in gold, we don't look anything up by trace id
// Maps aren't the best choice in those cases, so maybe instead of
// handing around a map of TraceID -> Trace we can hand around a []TracePair
diff --git a/golden/go/types/types.go b/golden/go/types/types.go
index 90fb1c5..22403fd 100644
--- a/golden/go/types/types.go
+++ b/golden/go/types/types.go
@@ -57,6 +57,29 @@
// with JSON already written to disk.
Keys map[string]string `json:"Params_"`
Digests []Digest `json:"Values"`
+
+ // cache these values so as not to incur the non-zero map lookup cost (~15 ns) repeatedly.
+ testName TestName
+ corpus string
+}
+
+// NewGoldenTraceN allocates a new Trace set up for the given number of samples.
+//
+// The Trace Digests are pre-filled in with the missing data sentinel since not
+// all tests will be run on all commits.
+func NewGoldenTraceN(n int, keys map[string]string) *GoldenTrace {
+ g := &GoldenTrace{
+ Digests: make([]Digest, n),
+ Keys: keys,
+
+ // Prefetch these now, while we have the chance.
+ testName: TestName(keys[PRIMARY_KEY_FIELD]),
+ corpus: keys[CORPUS_FIELD],
+ }
+ for i := range g.Digests {
+ g.Digests[i] = MISSING_DIGEST
+ }
+ return g
}
// Params implements the tiling.Trace interface.
@@ -67,13 +90,19 @@
// TestName is a helper for extracting just the test name for this
// trace, of which there should always be exactly one.
func (g *GoldenTrace) TestName() TestName {
- return TestName(g.Keys[PRIMARY_KEY_FIELD])
+ if g.testName == "" {
+ g.testName = TestName(g.Keys[PRIMARY_KEY_FIELD])
+ }
+ return g.testName
}
// Corpus is a helper for extracting just the corpus key for this
// trace, of which there should always be exactly one.
func (g *GoldenTrace) Corpus() string {
- return g.Keys[CORPUS_FIELD]
+ if g.corpus == "" {
+ g.corpus = g.Keys[CORPUS_FIELD]
+ }
+ return g.corpus
}
// Len implements the tiling.Trace interface.
@@ -106,8 +135,7 @@
n := len(g.Digests) + len(nextGold.Digests)
n1 := len(g.Digests)
- merged := NewGoldenTraceN(n)
- merged.Keys = g.Keys
+ merged := NewGoldenTraceN(n, g.Keys)
for k, v := range nextGold.Keys {
merged.Keys[k] = v
}
@@ -170,7 +198,7 @@
if idx := g.LastIndex(); idx >= 0 {
return g.Digests[idx]
}
- return Digest("")
+ return MISSING_DIGEST
}
// LastIndex returns the index of last non-empty value in this trace and -1 if
@@ -188,30 +216,3 @@
func (g *GoldenTrace) String() string {
return fmt.Sprintf("Keys: %#v, Digests: %q", g.Keys, g.Digests)
}
-
-// NewGoldenTrace allocates a new Trace set up for the given number of samples.
-//
-// The Trace Digests are pre-filled in with the missing data sentinel since not
-// all tests will be run on all commits.
-func NewGoldenTrace() *GoldenTrace {
- return NewGoldenTraceN(tiling.TILE_SIZE)
-}
-
-// NewGoldenTraceN allocates a new Trace set up for the given number of samples.
-//
-// The Trace Digests are pre-filled in with the missing data sentinel since not
-// all tests will be run on all commits.
-func NewGoldenTraceN(n int) *GoldenTrace {
- g := &GoldenTrace{
- Digests: make([]Digest, n),
- Keys: make(map[string]string),
- }
- for i := range g.Digests {
- g.Digests[i] = MISSING_DIGEST
- }
- return g
-}
-
-func GoldenTraceBuilder(n int) tiling.Trace {
- return NewGoldenTraceN(n)
-}
diff --git a/golden/go/types/types_test.go b/golden/go/types/types_test.go
index 169cf9b..5137c7c 100644
--- a/golden/go/types/types_test.go
+++ b/golden/go/types/types_test.go
@@ -1,8 +1,12 @@
package types
import (
+ "crypto/rand"
+ "fmt"
"testing"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
"go.skia.org/infra/go/testutils/unittest"
"go.skia.org/infra/go/tiling"
)
@@ -11,82 +15,47 @@
unittest.SmallTest(t)
N := 5
// Test NewGoldenTrace.
- g := NewGoldenTraceN(N)
- if got, want := g.Len(), N; got != want {
- t.Errorf("Wrong Digests Size: Got %v Want %v", got, want)
- }
- if got, want := len(g.Keys), 0; got != want {
- t.Errorf("Wrong Keys initial size: Got %v Want %v", got, want)
- }
-
+ g := NewGoldenTraceN(N, nil)
+ assert.Equal(t, N, g.Len(), "wrong digests size")
+ assert.Equal(t, 0, len(g.Keys), "wrong keys initial size")
g.Digests[0] = "a digest"
- if got, want := g.IsMissing(1), true; got != want {
- t.Errorf("All values should start as missing: Got %v Want %v", got, want)
- }
- if got, want := g.IsMissing(0), false; got != want {
- t.Errorf("Set values shouldn't be missing: Got %v Want %v", got, want)
- }
+ assert.True(t, g.IsMissing(1), "values start missing")
+ assert.False(t, g.IsMissing(0), "set values shouldn't be missing")
// Test Merge.
M := 7
- gm := NewGoldenTraceN(M)
+ gm := NewGoldenTraceN(M, nil)
gm.Digests[1] = "another digest"
g2 := g.Merge(gm)
- if got, want := g2.Len(), N+M; got != want {
- t.Errorf("Merge length wrong: Got %v Want %v", got, want)
- }
- if got, want := g2.(*GoldenTrace).Digests[0], Digest("a digest"); got != want {
- t.Errorf("Digest not copied correctly: Got %v Want %v", got, want)
- }
- if got, want := g2.(*GoldenTrace).Digests[6], Digest("another digest"); got != want {
- t.Errorf("Digest not copied correctly: Got %v Want %v", got, want)
- }
+ assert.Equal(t, N+M, g2.Len(), "merge length wrong")
+ assert.Equal(t, Digest("a digest"), g2.(*GoldenTrace).Digests[0])
+ assert.Equal(t, Digest("another digest"), g2.(*GoldenTrace).Digests[6])
// Test Grow.
- g = NewGoldenTraceN(N)
+ g = NewGoldenTraceN(N, nil)
g.Digests[0] = "foo"
g.Grow(2*N, tiling.FILL_BEFORE)
- if got, want := g.Digests[N], Digest("foo"); got != want {
- t.Errorf("Grow didn't FILL_BEFORE correctly: Got %v Want %v", got, want)
- }
+ assert.Equal(t, Digest("foo"), g.Digests[N], "Grow didn't FILL_BEFORE correctly")
- g = NewGoldenTraceN(N)
+ g = NewGoldenTraceN(N, nil)
g.Digests[0] = "foo"
g.Grow(2*N, tiling.FILL_AFTER)
- if got, want := g.Digests[0], Digest("foo"); got != want {
- t.Errorf("Grow didn't FILL_AFTER correctly: Got %v Want %v", got, want)
- }
+ assert.Equal(t, Digest("foo"), g.Digests[0], "Grow didn't FILL_AFTER correctly")
// Test Trim
- g = NewGoldenTraceN(N)
+ g = NewGoldenTraceN(N, nil)
g.Digests[1] = "foo"
- if err := g.Trim(1, 3); err != nil {
- t.Fatalf("Trim Failed: %s", err)
- }
- if got, want := g.Digests[0], Digest("foo"); got != want {
- t.Errorf("Trim didn't copy correctly: Got %v Want %v", got, want)
- }
- if got, want := g.Len(), 2; got != want {
- t.Errorf("Trim wrong length: Got %v Want %v", got, want)
- }
+ require.NoError(t, g.Trim(1, 3))
+ assert.Equal(t, Digest("foo"), g.Digests[0], "Trim didn't copy correctly")
+ assert.Equal(t, 2, g.Len(), "Trim wrong length")
- if err := g.Trim(-1, 1); err == nil {
- t.Error("Trim failed to error.")
- }
- if err := g.Trim(1, 3); err == nil {
- t.Error("Trim failed to error.")
- }
- if err := g.Trim(2, 1); err == nil {
- t.Error("Trim failed to error.")
- }
+ assert.Error(t, g.Trim(-1, 1))
+ assert.Error(t, g.Trim(1, 3))
+ assert.Error(t, g.Trim(2, 1))
- if err := g.Trim(1, 1); err != nil {
- t.Fatalf("Trim Failed: %s", err)
- }
- if got, want := g.Len(), 0; got != want {
- t.Errorf("Trim wrong length: Got %v Want %v", got, want)
- }
+ require.NoError(t, g.Trim(1, 1))
+ assert.Equal(t, 0, g.Len(), "final size wrong")
}
func TestSetAt(t *testing.T) {
@@ -104,15 +73,115 @@
want: MISSING_DIGEST,
},
}
- tr := NewGoldenTraceN(len(testCases))
+ tr := NewGoldenTraceN(len(testCases), nil)
for i, tc := range testCases {
- if err := tr.SetAt(i, []byte(tc.want)); err != nil {
- t.Fatalf("SetAt(%d, %#v) failed: %s", i, []byte(tc.want), err)
- }
+ require.NoError(t, tr.SetAt(i, []byte(tc.want)))
}
for i, tc := range testCases {
- if got, want := tr.Digests[i], tc.want; got != want {
- t.Errorf("SetAt(%d, %#v)failed: Got %s Want %s", i, []byte(tc.want), got, want)
+ assert.Equal(t, tc.want, tr.Digests[i], "Bad at test case %d", i)
+ }
+}
+
+var _tn TestName
+
+// BenchmarkTraceTestName shows that a map-lookup in go for this example param map is about
+// 15 nanoseconds, whereas pre-caching that value makes it about 0.5 ns.
+func BenchmarkTraceTestName(b *testing.B) {
+ // This is a typical paramset for a Skia trace, grabbed at random from the live data.
+ gt := NewGoldenTraceN(10, map[string]string{
+ "alpha_type": "Premul",
+ "arch": "arm64",
+ "color_depth": "8888",
+ "color_type": "RGBA_8888",
+ "compiler": "Clang",
+ "config": "ddl-vk",
+ "configuration": "Debug",
+ "cpu_or_gpu": "GPU",
+ "cpu_or_gpu_value": "Adreno540",
+ "ext": "png",
+ "extra_config": "Android_DDL1_Vulkan",
+ "gamut": "untagged",
+ "model": "Pixel2XL",
+ "name": "blurredclippedcircle",
+ "os": "Android",
+ "source_type": "gm",
+ "style": "DDL",
+ "transfer_fn": "untagged",
+ })
+
+ var r TestName
+ for n := 0; n < b.N; n++ {
+ // always record the result of TestName to prevent
+ // the compiler eliminating the function call.
+ r = gt.TestName()
+ }
+ // always store the result to a package level variable
+ // so the compiler cannot eliminate the Benchmark itself.
+ _tn = r
+}
+
+// BenchmarkTraceMapIteration shows that iterating through a map of 1.3 million traces
+// takes about 21 milliseconds.
+func BenchmarkTraceMapIteration(b *testing.B) {
+ // This is a typical size of a Skia tile, 1.3 million traces.
+ const numTraces = 1300000
+ // When we make the traces in bt_tracestore, we don't know how big they can be, so
+ // we just start from an empty map
+ traces := map[tiling.TraceId]tiling.Trace{}
+ for i := 0; i < numTraces; i++ {
+ id := randomString()
+ traces[tiling.TraceId(id)] = NewGoldenTraceN(10, map[string]string{
+ "alpha_type": "Premul",
+ "arch": "arm64",
+ "name": id,
+ })
+ }
+
+ b.ResetTimer()
+ for n := 0; n < b.N; n++ {
+ for id, tr := range traces {
+ if id == "blerg" { // should never happen
+ fmt.Println(tr)
+ }
}
}
}
+
+// BenchmarkTraceSliceIteration shows that iterating through a slice of 1.3 million traces
+// takes about 5 milliseconds.
+func BenchmarkTraceSliceIteration(b *testing.B) {
+ // This is a typical size of a Skia tile, 1.3 million traces.
+ const numTraces = 1300000
+ // When we make the traces in bt_tracestore, we wouldn't know how big they can be, so
+ // we just start from an empty slice
+ traces := []TracePair{}
+ for i := 0; i < numTraces; i++ {
+ id := randomString()
+ traces = append(traces, TracePair{
+ ID: tiling.TraceId(id),
+ Trace: NewGoldenTraceN(10, map[string]string{
+ "alpha_type": "Premul",
+ "arch": "arm64",
+ "name": id,
+ }),
+ })
+ }
+
+ b.ResetTimer()
+ for n := 0; n < b.N; n++ {
+ for _, tp := range traces {
+ id, tr := tp.ID, tp.Trace
+ if id == "blerg" { // should never happen
+ fmt.Println(tr)
+ }
+ }
+ }
+}
+
+const stringSize = 64
+
+func randomString() string {
+ b := make([]byte, stringSize)
+ _, _ = rand.Read(b)
+ return string(b)
+}
diff --git a/golden/go/web/web.go b/golden/go/web/web.go
index 688ad6e..13ee22d 100644
--- a/golden/go/web/web.go
+++ b/golden/go/web/web.go
@@ -564,7 +564,7 @@
// DetailsHandler returns the details about a single digest.
func (wh *Handlers) DetailsHandler(w http.ResponseWriter, r *http.Request) {
defer metrics2.FuncTimer().Stop()
- if err := wh.limitForAnonUsers(r); err != nil {
+ if err := wh.cheapLimitForAnonUsers(r); err != nil {
httputils.ReportError(w, err, "Try again later", http.StatusInternalServerError)
return
}
