[perf] Make types for trace id and source file id.
Also use stronger types more consistenly.
Change-Id: I72122718d21627b449f397c89654c5e5dd8a1cba
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/305682
Commit-Queue: Joe Gregorio <jcgregorio@google.com>
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Reviewed-by: Joe Gregorio <jcgregorio@google.com>
diff --git a/perf/go/tracestore/sqltracestore/sqltracestore.go b/perf/go/tracestore/sqltracestore/sqltracestore.go
index f00ad8d..fcd9d10 100644
--- a/perf/go/tracestore/sqltracestore/sqltracestore.go
+++ b/perf/go/tracestore/sqltracestore/sqltracestore.go
@@ -152,10 +152,22 @@
// per instance.
const cacheSize = 20 * 1000 * 1000
-// Ingest instances have around 8 cores and many ingested files have ~10K values, so
-// pick a batch size that allows roughly one request per core.
+// Ingest instances have around 8 cores and many ingested files have ~10K
+// values, so pick a batch size that allows roughly one request per core.
const traceValuesInsertBatchSize = 1000
+// traceIDFromSQL is the type of the IDs that are used in the SQL database for
+// traces.
+type traceIDFromSQL int64
+
+const badTraceIDFromSQL traceIDFromSQL = -1
+
+// sourceFileIDFromSQL is the type of the IDs that are used in the SQL database
+// for source files.
+type sourceFileIDFromSQL int64
+
+const badSourceFileIDFromSQL sourceFileIDFromSQL = -1
+
// statement is an SQL statement or fragment of an SQL statement.
type statement int
@@ -201,15 +213,15 @@
type insertIntoPostingsValue struct {
TileNumber types.TileNumber
KeyValue string
- TraceID int64
+ TraceID traceIDFromSQL
}
// replaceTraceValue is used in the replaceTraceValues template.
type replaceTraceValue struct {
- TraceID int64
+ TraceID traceIDFromSQL
CommitNumber types.CommitNumber
Val float32
- SourceFileID int64
+ SourceFileID sourceFileIDFromSQL
}
var statementsByDialect = map[perfsql.Dialect]statements{
@@ -303,8 +315,8 @@
// unpreparedStatements are parsed templates that can be used to construct SQL statements.
unpreparedStatements map[statement]*template.Template
- // cache is an LRU cache used to store the ids (int64) of trace names (string):
- // "traceName" -> int64
+ // cache is an LRU cache used to store the ids (traceIDFromSQL) of trace names (string):
+ // "traceName" -> traceIDFromSQL
// and if a traces postings have been written for a tile (bool).
// "traceID-tileNumber" -> bool
cache *lru.Cache
@@ -368,11 +380,11 @@
// GetLatestTile implements the tracestore.TraceStore interface.
func (s *SQLTraceStore) GetLatestTile() (types.TileNumber, error) {
- var tileNumber int64
+ tileNumber := types.BadTileNumber
if err := s.preparedStatements[getLatestTile].QueryRowContext(context.TODO()).Scan(&tileNumber); err != nil {
- return types.BadTileNumber, skerr.Wrap(err)
+ return tileNumber, skerr.Wrap(err)
}
- return types.TileNumber(tileNumber), nil
+ return tileNumber, nil
}
func (s *SQLTraceStore) paramSetForTile(tileNumber types.TileNumber) (paramtools.ParamSet, error) {
@@ -495,19 +507,19 @@
for rows.Next() {
var traceName string
- var commitNumber int64
+ var commitNumber types.CommitNumber
var val float64
if err := rows.Scan(&traceName, &commitNumber, &val); err != nil {
return nil, skerr.Wrap(err)
}
if _, ok := ret[traceName]; ok {
- ret[traceName][commitNumber%int64(s.tileSize)] = float32(val)
+ ret[traceName][s.OffsetFromCommitNumber(commitNumber)] = float32(val)
} else {
// TODO(jcgregorio) Replace this vec32.New() with a
// https://golang.org/pkg/sync/#Pool since this is our most used/reused
// type of memory.
ret[traceName] = vec32.New(int(s.tileSize))
- ret[traceName][commitNumber%int64(s.tileSize)] = float32(val)
+ ret[traceName][s.OffsetFromCommitNumber(commitNumber)] = float32(val)
}
}
if err := rows.Err(); err != nil {
@@ -571,6 +583,7 @@
AND `, beginCommit, endCommit)
// TODO(jcgregorio) Break out into own function.
+ // TODO(jcgregorio) Convert to use text/templates.
whereClauses := make([]string, len(plan))
clauseIndex := 0
for key, values := range plan {
@@ -662,12 +675,12 @@
}
for rows.Next() {
var traceName string
- var commitNumber int64
+ var commitNumber types.CommitNumber
var val float64
if err := rows.Scan(&traceName, &commitNumber, &val); err != nil {
return nil, skerr.Wrap(err)
}
- ret[traceName][commitNumber%int64(s.tileSize)] = float32(val)
+ ret[traceName][s.OffsetFromCommitNumber(commitNumber)] = float32(val)
}
if err := rows.Err(); err != nil {
return nil, skerr.Wrapf(err, "tileNumber=%d", tileNumber)
@@ -701,9 +714,9 @@
}
// updateSourceFile writes the filename into the SourceFiles table and returns
-// the int64 id of that filename.
-func (s *SQLTraceStore) updateSourceFile(filename string) (int64, error) {
- ret := int64(-1)
+// the sourceFileIDFromSQL of that filename.
+func (s *SQLTraceStore) updateSourceFile(filename string) (sourceFileIDFromSQL, error) {
+ ret := badSourceFileIDFromSQL
_, err := s.preparedStatements[insertIntoSourceFiles].ExecContext(context.TODO(), filename)
if err != nil {
return ret, skerr.Wrap(err)
@@ -719,7 +732,7 @@
// updatePostings writes all the entries into our inverted index in Postings for
// the given traceID and tileNumber. The Params given are from the parse trace
// name.
-func (s *SQLTraceStore) updatePostings(p paramtools.Params, tileNumber types.TileNumber, traceID int64) error {
+func (s *SQLTraceStore) updatePostings(p paramtools.Params, tileNumber types.TileNumber, traceID traceIDFromSQL) error {
// Clean the data to avoid SQL injection attacks.
p = query.ForceValid(p)
@@ -748,24 +761,24 @@
return nil
}
-func getPostingsCacheEntryKey(traceID int64, tileNumber types.TileNumber) string {
+func getPostingsCacheEntryKey(traceID traceIDFromSQL, tileNumber types.TileNumber) string {
return fmt.Sprintf("%d-%d", traceID, tileNumber)
}
-// writeTraceIDAndPostings writes the trace name into the TraceIDs table and returns the
-// int64 id of that trace name. This operation will happen repeatedly as data is
-// ingested so we cache the results in the LRU cache.
-func (s *SQLTraceStore) writeTraceIDAndPostings(traceNameAsParams paramtools.Params, tileNumber types.TileNumber) (int64, error) {
- traceID := int64(-1)
+// writeTraceIDAndPostings writes the trace name into the TraceIDs table and
+// returns the traceIDFromSQL of that trace name. This operation will happen
+// repeatedly as data is ingested so we cache the results in the LRU cache.
+func (s *SQLTraceStore) writeTraceIDAndPostings(traceNameAsParams paramtools.Params, tileNumber types.TileNumber) (traceIDFromSQL, error) {
+ traceID := badTraceIDFromSQL
traceName, err := query.MakeKey(traceNameAsParams)
if err != nil {
return traceID, skerr.Wrap(err)
}
- // Get an int64 trace id for the traceName.
+ // Get a traceIDFromSQL for the traceName.
if iret, ok := s.cache.Get(traceName); ok {
- traceID = iret.(int64)
+ traceID = iret.(traceIDFromSQL)
} else {
_, err := s.preparedStatements[insertIntoTraceIDs].ExecContext(context.TODO(), traceName)
if err != nil {
diff --git a/perf/go/tracestore/sqltracestore/sqltracestore_test.go b/perf/go/tracestore/sqltracestore/sqltracestore_test.go
index 737cdd8..cf7c3eb 100644
--- a/perf/go/tracestore/sqltracestore/sqltracestore_test.go
+++ b/perf/go/tracestore/sqltracestore/sqltracestore_test.go
@@ -62,8 +62,8 @@
func testWriteTraceIDAndPostings(t *testing.T, s *SQLTraceStore) {
const traceName = ",arch=x86,config=8888,"
- const tileNumber types.TileNumber = 1
p := paramtools.NewParams(traceName)
+ const tileNumber types.TileNumber = 1
// Do each update twice to ensure the IDs don't change.
traceID, err := s.writeTraceIDAndPostings(p, tileNumber)
@@ -76,7 +76,7 @@
// Confirm the cache entries exist.
got, ok := s.cache.Get(traceName)
assert.True(t, ok)
- assert.Equal(t, traceID, got.(int64))
+ assert.Equal(t, traceID, got.(traceIDFromSQL))
assert.True(t, s.cache.Contains(getPostingsCacheEntryKey(traceID, tileNumber)))
const traceName2 = ",arch=arm,config=8888,"
@@ -359,13 +359,14 @@
}
func testParamSetForTile(t *testing.T, s *SQLTraceStore) {
- _, err := s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=x86,"), 1)
+ const tileNumber types.TileNumber = 1
+ _, err := s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=x86,"), tileNumber)
assert.NoError(t, err)
- _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=565,arch=arm,"), 1)
+ _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=565,arch=arm,"), tileNumber)
assert.NoError(t, err)
- _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=arm64,"), 1)
+ _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=arm64,"), tileNumber)
assert.NoError(t, err)
- _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=gpu,arch=x86_64,"), 1)
+ _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=gpu,arch=x86_64,"), tileNumber)
assert.NoError(t, err)
ps, err := s.paramSetForTile(1)
@@ -385,11 +386,11 @@
}
func testGetLatestTile(t *testing.T, s *SQLTraceStore) {
- _, err := s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=x86,"), 1)
+ _, err := s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=x86,"), types.TileNumber(1))
assert.NoError(t, err)
- _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=arm64,"), 5)
+ _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=arm64,"), types.TileNumber(5))
assert.NoError(t, err)
- _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=gpu,arch=x86_64,"), 7)
+ _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=gpu,arch=x86_64,"), types.TileNumber(7))
assert.NoError(t, err)
tileNumber, err := s.GetLatestTile()
@@ -407,14 +408,15 @@
func testGetOrderedParamSet(t *testing.T, s *SQLTraceStore) {
ctx := context.Background()
+ const tileNumber types.TileNumber = 1
// Now add some trace ids.
- _, err := s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=x86,"), 1)
+ _, err := s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=x86,"), tileNumber)
assert.NoError(t, err)
- _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=565,arch=arm,"), 1)
+ _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=565,arch=arm,"), tileNumber)
assert.NoError(t, err)
- _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=arm64,"), 1)
+ _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=arm64,"), tileNumber)
assert.NoError(t, err)
- _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=gpu,arch=x86_64,"), 1)
+ _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=gpu,arch=x86_64,"), tileNumber)
assert.NoError(t, err)
ops, err := s.GetOrderedParamSet(ctx, 1)
@@ -439,14 +441,15 @@
func testCountIndices(t *testing.T, s *SQLTraceStore) {
ctx := context.Background()
+ const tileNumber types.TileNumber = 1
// Now add some trace ids.
- _, err := s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=x86,"), 1)
+ _, err := s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=x86,"), tileNumber)
assert.NoError(t, err)
- _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=565,arch=arm,"), 1)
+ _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=565,arch=arm,"), tileNumber)
assert.NoError(t, err)
- _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=arm64,"), 1)
+ _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=8888,arch=arm64,"), tileNumber)
assert.NoError(t, err)
- _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=gpu,arch=x86_64,"), 1)
+ _, err = s.writeTraceIDAndPostings(paramtools.NewParams(",config=gpu,arch=x86_64,"), tileNumber)
assert.NoError(t, err)
count, err := s.CountIndices(ctx, 1)
