Merge pull request #1098 from jzelinskie/rm-sprintf

Optimize allocations by removing sprintf, using strings.Cut

internal/datasets/subjectsetbytype.go

@@ -1,11 +1,9 @@
 package datasets
 
 import (
-	"fmt"
-	"strings"
-
 	core "github.com/authzed/spicedb/pkg/proto/core/v1"
 	v1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1"
+	"github.com/authzed/spicedb/pkg/tuple"
 )
 
 // TODO(jschorr): See if there is a nice way we can combine this withn ONRByTypeSet and the multimap
@@ -35,12 +33,12 @@ func (s *SubjectByTypeSet) AddConcreteSubject(subject *core.ObjectAndRelation) e
 
 // AddSubject adds the specified subject to the set.
 func (s *SubjectByTypeSet) AddSubject(subject *core.ObjectAndRelation, caveat *core.ContextualizedCaveat) error {
-	typeKey := fmt.Sprintf("%s#%s", subject.Namespace, subject.Relation)
-	if _, ok := s.byType[typeKey]; !ok {
-		s.byType[typeKey] = NewSubjectSet()
+	key := tuple.JoinRelRef(subject.Namespace, subject.Relation)
+	if _, ok := s.byType[key]; !ok {
+		s.byType[key] = NewSubjectSet()
 	}
 
-	return s.byType[typeKey].Add(&v1.FoundSubject{
+	return s.byType[key].Add(&v1.FoundSubject{
 		SubjectId:        subject.ObjectId,
 		CaveatExpression: wrapCaveat(caveat),
 	})
@@ -50,10 +48,10 @@ func (s *SubjectByTypeSet) AddSubject(subject *core.ObjectAndRelation, caveat *c
 // with all IDs of objects of that type.
 func (s *SubjectByTypeSet) ForEachType(handler func(rr *core.RelationReference, subjects SubjectSet)) {
 	for key, subjects := range s.byType {
-		parts := strings.Split(key, "#")
+		ns, rel := tuple.MustSplitRelRef(key)
 		handler(&core.RelationReference{
-			Namespace: parts[0],
-			Relation:  parts[1],
+			Namespace: ns,
+			Relation:  rel,
 		}, subjects)
 	}
 }
@@ -63,19 +61,18 @@ func (s *SubjectByTypeSet) ForEachType(handler func(rr *core.RelationReference,
 func (s *SubjectByTypeSet) Map(mapper func(rr *core.RelationReference) (*core.RelationReference, error)) (*SubjectByTypeSet, error) {
 	mapped := NewSubjectByTypeSet()
 	for key, subjectset := range s.byType {
-		parts := strings.Split(key, "#")
+		ns, rel := tuple.MustSplitRelRef(key)
 		updatedType, err := mapper(&core.RelationReference{
-			Namespace: parts[0],
-			Relation:  parts[1],
+			Namespace: ns,
+			Relation:  rel,
 		})
 		if err != nil {
 			return nil, err
 		}
 		if updatedType == nil {
 			continue
 		}
-		updatedTypeKey := fmt.Sprintf("%s#%s", updatedType.Namespace, updatedType.Relation)
-		mapped.byType[updatedTypeKey] = subjectset
+		mapped.byType[tuple.JoinRelRef(updatedType.Namespace, updatedType.Relation)] = subjectset
 	}
 	return mapped, nil
 }
@@ -92,8 +89,7 @@ func (s *SubjectByTypeSet) Len() int {
 
 // SubjectSetForType returns the subject set associated with the given subject type, if any.
 func (s *SubjectByTypeSet) SubjectSetForType(rr *core.RelationReference) (SubjectSet, bool) {
-	typeKey := fmt.Sprintf("%s#%s", rr.Namespace, rr.Relation)
-	found, ok := s.byType[typeKey]
+	found, ok := s.byType[tuple.JoinRelRef(rr.Namespace, rr.Relation)]
 	return found, ok
 }
 

internal/datastore/common/sql.go

@@ -2,7 +2,6 @@ package common
 
 import (
 	"context"
-	"fmt"
 	"math"
 	"runtime"
 
@@ -110,7 +109,7 @@ func (sqf SchemaQueryFilterer) FilterToResourceIDs(resourceIds []string) (Schema
 		return sqf, spiceerrors.MustBugf("cannot have more than %d resources IDs in a single filter", datastore.FilterMaximumIDCount)
 	}
 
-	inClause := fmt.Sprintf("%s IN (", sqf.schema.ColObjectID)
+	inClause := sqf.schema.ColObjectID + " IN ("
 	args := make([]any, 0, len(resourceIds))
 
 	for index, resourceID := range resourceIds {
@@ -209,7 +208,7 @@ func (sqf SchemaQueryFilterer) FilterWithSubjectsSelectors(selectors ...datastor
 				return sqf, spiceerrors.MustBugf("cannot have more than %d subject IDs in a single filter", datastore.FilterMaximumIDCount)
 			}
 
-			inClause := fmt.Sprintf("%s IN (", sqf.schema.ColUsersetObjectID)
+			inClause := sqf.schema.ColUsersetObjectID + " IN ("
 			args := make([]any, 0, len(selector.OptionalSubjectIds))
 
 			for index, subjectID := range selector.OptionalSubjectIds {

internal/datastore/crdb/crdb.go

@@ -62,9 +62,8 @@ const (
 	errUnableToInstantiate = "unable to instantiate datastore: %w"
 	errRevision            = "unable to find revision: %w"
 
-	querySelectNow          = "SELECT cluster_logical_timestamp()"
-	queryShowZoneConfig     = "SHOW ZONE CONFIGURATION FOR RANGE default;"
-	querySetTransactionTime = "SET TRANSACTION AS OF SYSTEM TIME %s"
+	querySelectNow      = "SELECT cluster_logical_timestamp()"
+	queryShowZoneConfig = "SHOW ZONE CONFIGURATION FOR RANGE default;"
 
 	livingTupleConstraint = "pk_relation_tuple"
 )
@@ -237,7 +236,7 @@ func (cds *crdbDatastore) SnapshotReader(rev datastore.Revision) datastore.Reade
 			}
 		}
 
-		setTxTime := fmt.Sprintf(querySetTransactionTime, rev)
+		setTxTime := "SET TRANSACTION AS OF SYSTEM TIME " + rev.String()
 		if _, err := tx.Exec(ctx, setTxTime); err != nil {
 			if err := tx.Rollback(ctx); err != nil {
 				log.Ctx(ctx).Warn().Err(err).Msg(

internal/datastore/crdb/keys.go

@@ -43,9 +43,9 @@ func appendKeysFromNamespaceComponents(keySet keySet, namespace string) {
 
 // prefix returns the first namespace prefix or the default overlap key
 func prefix(s string) string {
-	parts := strings.Split(s, "/")
-	if len(parts) < 2 {
+	prefix, _, ok := strings.Cut(s, "/")
+	if !ok {
 		return defaultOverlapKey
 	}
-	return parts[0]
+	return prefix
 }

internal/datastore/memdb/schema.go

@@ -1,8 +1,6 @@
 package memdb
 
 import (
-	"fmt"
-
 	v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
 	"github.com/hashicorp/go-memdb"
 	"github.com/jzelinskie/stringz"
@@ -70,19 +68,10 @@ func (cr *contextualizedCaveat) ContextualizedCaveat() (*core.ContextualizedCave
 func (r relationship) String() string {
 	caveat := ""
 	if r.caveat != nil {
-		caveat = fmt.Sprintf("[%s]", r.caveat.caveatName)
+		caveat = "[" + r.caveat.caveatName + "]"
 	}
 
-	return fmt.Sprintf(
-		"%s:%s#%s@%s:%s#%s%s",
-		r.namespace,
-		r.resourceID,
-		r.relation,
-		r.subjectNamespace,
-		r.subjectObjectID,
-		r.subjectRelation,
-		caveat,
-	)
+	return r.namespace + ":" + r.resourceID + "#" + r.relation + "@" + r.subjectNamespace + ":" + r.subjectObjectID + "#" + r.subjectRelation + caveat
 }
 
 func (r relationship) MarshalZerologObject(e *zerolog.Event) {

internal/datastore/mysql/connection.go

@@ -113,7 +113,7 @@ func (s *sessionVariableConnector) Driver() driver.Driver {
 func addSessionVariables(c driver.Connector, variables map[string]string) (driver.Connector, error) {
 	statements := make([]string, 0, len(variables))
 	for sessionVar, value := range variables {
-		statements = append(statements, fmt.Sprintf("SET SESSION %s=%s", sessionVar, value))
+		statements = append(statements, "SET SESSION "+sessionVar+"="+value)
 	}
 
 	return &sessionVariableConnector{

internal/datastore/mysql/datastore.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"strconv"
 	"time"
 
 	sq "github.com/Masterminds/squirrel"
@@ -114,7 +115,7 @@ func newMySQLDatastore(uri string, options ...Option) (*Datastore, error) {
 	if config.lockWaitTimeoutSeconds != nil {
 		log.Info().Uint8("timeout", *config.lockWaitTimeoutSeconds).Msg("overriding innodb_lock_wait_timeout")
 		connector, err = addSessionVariables(connector, map[string]string{
-			"innodb_lock_wait_timeout": fmt.Sprintf("%d", *config.lockWaitTimeoutSeconds),
+			"innodb_lock_wait_timeout": strconv.FormatUint(uint64(*config.lockWaitTimeoutSeconds), 10),
 		})
 		if err != nil {
 			return nil, fmt.Errorf("NewMySQLDatastore: failed to add session variables to connector: %w", err)

internal/datastore/mysql/migrations/driver.go

@@ -59,7 +59,7 @@ func NewMySQLDriverFromDB(db *sql.DB, tablePrefix string) *MySQLDriver {
 
 // revisionToColumnName generates the column name that will denote a given migration revision
 func revisionToColumnName(revision string) string {
-	return fmt.Sprintf("%s%s", migrationVersionColumnPrefix, revision)
+	return migrationVersionColumnPrefix + revision
 }
 
 func columnNameToRevision(columnName string) (string, bool) {

internal/datastore/mysql/migrations/tables.go

@@ -1,7 +1,5 @@
 package migrations
 
-import "fmt"
-
 const (
 	tableNamespaceDefault   = "namespace_config"
 	tableTransactionDefault = "relation_tuple_transaction"
@@ -22,12 +20,12 @@ type tables struct {
 
 func newTables(prefix string) *tables {
 	return &tables{
-		tableMigrationVersion: fmt.Sprintf("%s%s", prefix, tableMigrationVersion),
-		tableTransaction:      fmt.Sprintf("%s%s", prefix, tableTransactionDefault),
-		tableTuple:            fmt.Sprintf("%s%s", prefix, tableTupleDefault),
-		tableNamespace:        fmt.Sprintf("%s%s", prefix, tableNamespaceDefault),
-		tableMetadata:         fmt.Sprintf("%s%s", prefix, tableMetadataDefault),
-		tableCaveat:           fmt.Sprintf("%s%s", prefix, tableCaveatDefault),
+		tableMigrationVersion: prefix + tableMigrationVersion,
+		tableTransaction:      prefix + tableTransactionDefault,
+		tableTuple:            prefix + tableTupleDefault,
+		tableNamespace:        prefix + tableNamespaceDefault,
+		tableMetadata:         prefix + tableMetadataDefault,
+		tableCaveat:           prefix + tableCaveatDefault,
 	}
 }
 

internal/datastore/mysql/stats.go

@@ -16,15 +16,13 @@ const (
 	informationSchemaTablesTable     = "INFORMATION_SCHEMA.TABLES"
 	informationSchemaTableNameColumn = "table_name"
 
-	analyzeTableQuery = "ANALYZE TABLE %s"
-
 	metadataIDColumn       = "id"
 	metadataUniqueIDColumn = "unique_id"
 )
 
 func (mds *Datastore) Statistics(ctx context.Context) (datastore.Stats, error) {
 	if mds.analyzeBeforeStats {
-		_, err := mds.db.ExecContext(ctx, fmt.Sprintf(analyzeTableQuery, mds.driver.RelationTuple()))
+		_, err := mds.db.ExecContext(ctx, "ANALYZE TABLE "+mds.driver.RelationTuple())
 		if err != nil {
 			return datastore.Stats{}, fmt.Errorf("unable to run ANALYZE TABLE: %w", err)
 		}

internal/datastore/postgres/revisions.go

@@ -213,9 +213,9 @@ func (pr postgresRevision) LessThan(rhsRaw datastore.Revision) bool {
 
 func (pr postgresRevision) String() string {
 	if pr.xmin.Status == pgtype.Present {
-		return fmt.Sprintf("%d.%d", pr.tx.Uint, pr.xmin.Uint)
+		return strconv.FormatUint(pr.tx.Uint, 10) + "." + strconv.FormatUint(pr.xmin.Uint, 10)
 	}
-	return fmt.Sprintf("%d", pr.tx.Uint)
+	return strconv.FormatUint(pr.tx.Uint, 10)
 }
 
 func (pr postgresRevision) MarshalBinary() ([]byte, error) {

internal/datastore/postgres/stats.go

@@ -48,7 +48,7 @@ func (pgd *pgDatastore) Statistics(ctx context.Context) (datastore.Stats, error)
 	var relCount int64
 	if err := pgd.dbpool.BeginTxFunc(ctx, pgd.readTxOptions, func(tx pgx.Tx) error {
 		if pgd.analyzeBeforeStatistics {
-			if _, err := tx.Exec(ctx, fmt.Sprintf("ANALYZE %s", tableTuple)); err != nil {
+			if _, err := tx.Exec(ctx, "ANALYZE "+tableTuple); err != nil {
 				return fmt.Errorf("unable to analyze tuple table: %w", err)
 			}
 		}

internal/datastore/spanner/spanner.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"regexp"
+	"strconv"
 	"time"
 
 	"cloud.google.com/go/spanner"
@@ -193,8 +194,7 @@ func (sd spannerDatastore) Close() error {
 func statementFromSQL(sql string, args []interface{}) spanner.Statement {
 	params := make(map[string]interface{}, len(args))
 	for index, arg := range args {
-		paramName := fmt.Sprintf("p%d", index+1)
-		params[paramName] = arg
+		params["p"+strconv.Itoa(index+1)] = arg
 	}
 
 	return spanner.Statement{

internal/developmentmembership/foundsubject.go

@@ -1,7 +1,6 @@
 package developmentmembership
 
 import (
-	"fmt"
 	"sort"
 	"strings"
 
@@ -96,12 +95,12 @@ func (fs FoundSubject) ToValidationString() string {
 	onrString := tuple.StringONR(fs.Subject())
 	validationString := onrString
 	if fs.caveatExpression != nil {
-		validationString = fmt.Sprintf("%s[...]", validationString)
+		validationString = validationString + "[...]"
 	}
 
 	excluded, isWildcard := fs.ExcludedSubjectsFromWildcard()
 	if isWildcard && len(excluded) > 0 {
-		validationString = fmt.Sprintf("%s - {%s}", validationString, strings.Join(fs.excludedSubjectStrings(), ", "))
+		validationString = validationString + " - {" + strings.Join(fs.excludedSubjectStrings(), ", ") + "}"
 	}
 
 	return validationString

internal/developmentmembership/trackingsubjectset.go

@@ -1,9 +1,6 @@
 package developmentmembership
 
 import (
-	"fmt"
-	"strings"
-
 	"github.com/authzed/spicedb/internal/datasets"
 	core "github.com/authzed/spicedb/pkg/proto/core/v1"
 	"github.com/authzed/spicedb/pkg/tuple"
@@ -84,24 +81,19 @@ func (tss *TrackingSubjectSet) Add(subjectsAndResources ...FoundSubject) error {
 	return nil
 }
 
-func keyFor(fs FoundSubject) string {
-	return fmt.Sprintf("%s#%s", fs.subject.Namespace, fs.subject.Relation)
-}
-
 func (tss *TrackingSubjectSet) getSetForKey(key string) datasets.BaseSubjectSet[FoundSubject] {
 	if existing, ok := tss.setByType[key]; ok {
 		return existing
 	}
 
-	parts := strings.Split(key, "#")
-
-	created := datasets.NewBaseSubjectSet[FoundSubject](
+	ns, rel := tuple.MustSplitRelRef(key)
+	created := datasets.NewBaseSubjectSet(
 		func(subjectID string, caveatExpression *core.CaveatExpression, excludedSubjects []FoundSubject, sources ...FoundSubject) FoundSubject {
 			fs := NewFoundSubject(&core.DirectSubject{
 				Subject: &core.ObjectAndRelation{
-					Namespace: parts[0],
+					Namespace: ns,
 					ObjectId:  subjectID,
-					Relation:  parts[1],
+					Relation:  rel,
 				},
 				CaveatExpression: caveatExpression,
 			})
@@ -120,13 +112,12 @@ func (tss *TrackingSubjectSet) getSetForKey(key string) datasets.BaseSubjectSet[
 }
 
 func (tss *TrackingSubjectSet) getSet(fs FoundSubject) datasets.BaseSubjectSet[FoundSubject] {
-	fsKey := keyFor(fs)
-	return tss.getSetForKey(fsKey)
+	return tss.getSetForKey(tuple.JoinRelRef(fs.subject.Namespace, fs.subject.Relation))
 }
 
 // Get returns the found subject in the set, if any.
 func (tss *TrackingSubjectSet) Get(subject *core.ObjectAndRelation) (FoundSubject, bool) {
-	set, ok := tss.setByType[fmt.Sprintf("%s#%s", subject.Namespace, subject.Relation)]
+	set, ok := tss.setByType[tuple.JoinRelRef(subject.Namespace, subject.Relation)]
 	if !ok {
 		return FoundSubject{}, false
 	}
@@ -203,7 +194,7 @@ func (tss *TrackingSubjectSet) ApplyParentCaveatExpression(parentCaveatExpr *cor
 // the exact matching wildcard will be removed.
 func (tss *TrackingSubjectSet) removeExact(subjects ...*core.ObjectAndRelation) {
 	for _, subject := range subjects {
-		if set, ok := tss.setByType[fmt.Sprintf("%s#%s", subject.Namespace, subject.Relation)]; ok {
+		if set, ok := tss.setByType[tuple.JoinRelRef(subject.Namespace, subject.Relation)]; ok {
 			set.UnsafeRemoveExact(FoundSubject{
 				subject: subject,
 			})