Chore: Remove one allocate per hash by using generics (#5829)

algorand · Nov 17, 2023 · 06790fe · 06790fe
1 parent 07e0a1b
commit 06790fe
Show file tree

Hide file tree

Showing 11 changed files with 54 additions and 18 deletions.
diff --git a/cmd/goal/application.go b/cmd/goal/application.go
@@ -488,7 +488,7 @@ var createAppCmd = &cobra.Command{
 				reportErrorf(errorBroadcastingTX, err2)
 			}
 
-			reportInfof("Attempting to create app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), crypto.HashObj(logic.Program(clearProg)))
+			reportInfof("Attempting to create app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), logic.HashProgram(clearProg))
 			reportInfof("Issued transaction from account %s, txid %s (fee %d)", tx.Sender, txid, tx.Fee.Raw)
 
 			if !noWaitAfterSend {
@@ -563,7 +563,7 @@ var updateAppCmd = &cobra.Command{
 				reportErrorf(errorBroadcastingTX, err2)
 			}
 
-			reportInfof("Attempting to update app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), crypto.HashObj(logic.Program(clearProg)))
+			reportInfof("Attempting to update app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), logic.HashProgram(clearProg))
 			reportInfof("Issued transaction from account %s, txid %s (fee %d)", tx.Sender, txid, tx.Fee.Raw)
 
 			if !noWaitAfterSend {
@@ -1455,9 +1455,9 @@ var methodAppCmd = &cobra.Command{
 
 		// Report tx details to user
 		if methodCreatesApp {
-			reportInfof("Attempting to create app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), crypto.HashObj(logic.Program(clearProg)))
+			reportInfof("Attempting to create app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), logic.HashProgram(clearProg))
 		} else if onCompletionEnum == transactions.UpdateApplicationOC {
-			reportInfof("Attempting to update app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), crypto.HashObj(logic.Program(clearProg)))
+			reportInfof("Attempting to update app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), logic.HashProgram(clearProg))
 		}
 
 		reportInfof("Issued %d transaction(s):", len(signedTxnGroup))

diff --git a/cmd/goal/interact.go b/cmd/goal/interact.go
@@ -625,9 +625,9 @@ var appExecuteCmd = &cobra.Command{
 			}
 
 			if appIdx == 0 {
-				reportInfof("Attempting to create app (global ints %d, global blobs %d, local ints %d, local blobs %d, approval size %d, hash %v; clear size %d, hash %v)", globalSchema.NumUint, globalSchema.NumByteSlice, localSchema.NumUint, localSchema.NumByteSlice, len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), crypto.HashObj(logic.Program(clearProg)))
+				reportInfof("Attempting to create app (global ints %d, global blobs %d, local ints %d, local blobs %d, approval size %d, hash %v; clear size %d, hash %v)", globalSchema.NumUint, globalSchema.NumByteSlice, localSchema.NumUint, localSchema.NumByteSlice, len(approvalProg), logic.HashProgram(approvalProg), len(clearProg), crypto.HashObj(logic.Program(clearProg)))
 			} else if onCompletion == transactions.UpdateApplicationOC {
-				reportInfof("Attempting to update app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), crypto.HashObj(logic.Program(clearProg)))
+				reportInfof("Attempting to update app (approval size %d, hash %v; clear size %d, hash %v)", len(approvalProg), crypto.HashObj(logic.Program(approvalProg)), len(clearProg), logic.HashProgram(clearProg))
 			}
 			reportInfof("Issued transaction from account %s, txid %s (fee %d)", tx.Sender, txid, tx.Fee.Raw)
 

diff --git a/cmd/goal/tealsign.go b/cmd/goal/tealsign.go
@@ -139,7 +139,7 @@ The base64 encoding of the signature will always be printed to stdout. Optionall
 				reportErrorf(tealsignEmptyLogic)
 			}
 
-			progHash = crypto.HashObj(logic.Program(stxn.Lsig.Logic))
+			progHash = logic.HashProgram(stxn.Lsig.Logic)
 		} else {
 			// Otherwise, the contract address is the logic hash
 			parsedAddr, err := basics.UnmarshalChecksumAddress(contractAddr)

diff --git a/crypto/hashes_test.go b/crypto/hashes_test.go
@@ -53,7 +53,6 @@ func TestHashSum(t *testing.T) {
 
 	dgst := HashObj(TestingHashable{})
 	a.Equal(GenericHashObj(h, TestingHashable{}), dgst[:])
-
 }
 
 func TestEmptyHash(t *testing.T) {

diff --git a/crypto/util.go b/crypto/util.go
@@ -35,7 +35,7 @@ type Hashable interface {
 }
 
 // HashRep appends the correct hashid before the message to be hashed.
-func HashRep(h Hashable) []byte {
+func HashRep[H Hashable](h H) []byte {
 	hashid, data := h.ToBeHashed()
 	return append([]byte(hashid), data...)
 }
@@ -86,7 +86,7 @@ func Hash(data []byte) Digest {
 }
 
 // HashObj computes a hash of a Hashable object and its type
-func HashObj(h Hashable) Digest {
+func HashObj[H Hashable](h H) Digest {
 	return Hash(HashRep(h))
 }
 

diff --git a/daemon/algod/api/server/v2/test/handlers_test.go b/daemon/algod/api/server/v2/test/handlers_test.go
@@ -22,7 +22,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"golang.org/x/sync/semaphore"
 	"io"
 	"math"
 	"net"
@@ -33,6 +32,8 @@ import (
 	"testing"
 	"time"
 
+	"golang.org/x/sync/semaphore"
+
 	"github.com/algorand/go-algorand/daemon/algod/api/server"
 	"github.com/algorand/go-algorand/ledger/eval"
 	"github.com/algorand/go-algorand/ledger/ledgercore"
@@ -272,8 +273,7 @@ func addBlockHelper(t *testing.T) (v2.Handlers, echo.Context, *httptest.Response
 
 	// make an app call txn with eval delta
 	lsig := transactions.LogicSig{Logic: retOneProgram} // int 1
-	program := logic.Program(lsig.Logic)
-	lhash := crypto.HashObj(&program)
+	lhash := logic.HashProgram(lsig.Logic)
 	var sender basics.Address
 	copy(sender[:], lhash[:])
 	stx := transactions.SignedTxn{

diff --git a/daemon/algod/api/server/v2/test/helpers.go b/daemon/algod/api/server/v2/test/helpers.go
@@ -342,8 +342,7 @@ func testingenvWithBalances(t testing.TB, minMoneyAtStart, maxMoneyAtStart, numA
 
 	genesis[poolAddr] = basics_testing.MakeAccountData(basics.NotParticipating, basics.MicroAlgos{Raw: 100000 * uint64(proto.RewardsRateRefreshInterval)})
 
-	program := logic.Program(retOneProgram)
-	lhash := crypto.HashObj(&program)
+	lhash := logic.HashProgram(retOneProgram)
 	var addr basics.Address
 	copy(addr[:], lhash[:])
 	ad := basics_testing.MakeAccountData(basics.NotParticipating, basics.MicroAlgos{Raw: 100000 * uint64(proto.RewardsRateRefreshInterval)})

diff --git a/data/account/participation.go b/data/account/participation.go
@@ -80,8 +80,8 @@ func (id *ParticipationKeyIdentity) ToBeHashed() (protocol.HashID, []byte) {
 }
 
 // ID creates a ParticipationID hash from the identity file.
-func (id ParticipationKeyIdentity) ID() ParticipationID {
-	return ParticipationID(crypto.HashObj(&id))
+func (id *ParticipationKeyIdentity) ID() ParticipationID {
+	return ParticipationID(crypto.HashObj(id))
 }
 
 // ID computes a ParticipationID.

diff --git a/data/account/participation_test.go b/data/account/participation_test.go
@@ -606,3 +606,13 @@ func BenchmarkParticipationSign(b *testing.B) {
 		_ = part.Voting.Sign(ephID, msg)
 	}
 }
+
+func BenchmarkID(b *testing.B) {
+	pki := ParticipationKeyIdentity{}
+	b.Run("existing", func(b *testing.B) {
+		b.ReportAllocs() // demonstrate this is a single alloc
+		for i := 0; i < b.N; i++ {
+			pki.ID()
+		}
+	})
+}
diff --git a/data/bookkeeping/genesis_test.go b/data/bookkeeping/genesis_test.go
@@ -23,6 +23,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/algorand/go-algorand/crypto"
 	"github.com/algorand/go-algorand/data/basics"
 	"github.com/algorand/go-algorand/test/partitiontest"
 )
@@ -155,3 +156,30 @@ func TestGenesis_Balances(t *testing.T) {
 		})
 	}
 }
+
+func (genesis Genesis) hashOld() crypto.Digest {
+	return hashObjOld(genesis)
+}
+
+// hashObjOld computes a hash of a Hashable object and its type, doing so the
+// "old way" to show it requires an extra allocation in benchmarks.
+func hashObjOld(h crypto.Hashable) crypto.Digest {
+	return crypto.Hash(crypto.HashRep(h))
+}
+
+func BenchmarkGenesisHash(b *testing.B) {
+	b.ReportAllocs()
+	g := Genesis{}
+	b.Run("new", func(b *testing.B) {
+		b.ReportAllocs()
+		for i := 0; i < b.N; i++ {
+			g.Hash()
+		}
+	})
+	b.Run("old", func(b *testing.B) {
+		b.ReportAllocs()
+		for i := 0; i < b.N; i++ {
+			g.hashOld()
+		}
+	})
+}
diff --git a/data/transactions/logic/program.go b/data/transactions/logic/program.go
@@ -33,5 +33,5 @@ func (lsl Program) ToBeHashed() (protocol.HashID, []byte) {
 // This Digest can be used as an Address for a logic controlled account.
 func HashProgram(program []byte) crypto.Digest {
 	pb := Program(program)
-	return crypto.HashObj(&pb)
+	return crypto.HashObj(pb)
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -53,7 +53,6 @@ func TestHashSum(t *testing.T) { @@
     	dgst := HashObj(TestingHashable{})
     	a.Equal(GenericHashObj(h, TestingHashable{}), dgst[:])
     }
     func TestEmptyHash(t *testing.T) {
@@ Expand Down @@