perf: avoid copy of period locks in epoch (backport #7192)

x/incentives/keeper/distribute.go

@@ -374,7 +374,7 @@ func (k Keeper) doDistributionSends(ctx sdk.Context, distrs *distributionInfo) e
 // the distrInfo struct. It also updates the gauge for the distribution.
 // locks is expected to be the correct set of lock recipients for this gauge.
 func (k Keeper) distributeSyntheticInternal(
-	ctx sdk.Context, gauge types.Gauge, locks []lockuptypes.PeriodLock, distrInfo *distributionInfo,
+	ctx sdk.Context, gauge types.Gauge, locks []*lockuptypes.PeriodLock, distrInfo *distributionInfo,
 ) (sdk.Coins, error) {
 	qualifiedLocks := k.lk.GetLocksLongerThanDurationDenom(ctx, gauge.DistributeTo.Denom, gauge.DistributeTo.Duration)
 
@@ -398,12 +398,17 @@ func (k Keeper) distributeSyntheticInternal(
 		}
 	}
 
-	sortedAndTrimmedQualifiedLocks := make([]lockuptypes.PeriodLock, curIndex)
+	sortedAndTrimmedQualifiedLocks := make([]*lockuptypes.PeriodLock, curIndex)
+	// This is not an issue because we directly
+	// use v.index and &v.locks. However, we must be careful not to
+	// take the address of &v.
+	// nolint: exportloopref
 	for _, v := range qualifiedLocksMap {
+		v := v
 		if v.index < 0 {
 			continue
 		}
-		sortedAndTrimmedQualifiedLocks[v.index] = v.lock
+		sortedAndTrimmedQualifiedLocks[v.index] = &v.lock
 	}
 
 	return k.distributeInternal(ctx, gauge, sortedAndTrimmedQualifiedLocks, distrInfo)
@@ -556,7 +561,7 @@ func (k Keeper) syncVolumeSplitGroup(ctx sdk.Context, group types.Group) error {
 //
 // CONTRACT: gauge passed in as argument must be an active gauge.
 func (k Keeper) distributeInternal(
-	ctx sdk.Context, gauge types.Gauge, locks []lockuptypes.PeriodLock, distrInfo *distributionInfo,
+	ctx sdk.Context, gauge types.Gauge, locks []*lockuptypes.PeriodLock, distrInfo *distributionInfo,
 ) (sdk.Coins, error) {
 	totalDistrCoins := sdk.NewCoins()
 
@@ -757,10 +762,10 @@ func (k Keeper) handleGroupPostDistribute(ctx sdk.Context, groupGauge types.Gaug
 }
 
 // getDistributeToBaseLocks takes a gauge along with cached period locks by denom and returns locks that must be distributed to
-func (k Keeper) getDistributeToBaseLocks(ctx sdk.Context, gauge types.Gauge, cache map[string][]lockuptypes.PeriodLock) []lockuptypes.PeriodLock {
+func (k Keeper) getDistributeToBaseLocks(ctx sdk.Context, gauge types.Gauge, cache map[string][]lockuptypes.PeriodLock) []*lockuptypes.PeriodLock {
 	// if gauge is empty, don't get the locks
 	if gauge.Coins.Empty() {
-		return []lockuptypes.PeriodLock{}
+		return []*lockuptypes.PeriodLock{}
 	}
 	// Confusingly, there is no way to get all synthetic lockups. Thus we use a separate method `distributeSyntheticInternal` to separately get lockSum for synthetic lockups.
 	// All gauges have a precondition of being ByDuration.

x/incentives/keeper/iterator.go

@@ -63,11 +63,11 @@ func (k Keeper) FinishedGaugesIterator(ctx sdk.Context) sdk.Iterator {
 }
 
 // FilterLocksByMinDuration returns locks whose lock duration is greater than the provided minimum duration.
-func FilterLocksByMinDuration(locks []lockuptypes.PeriodLock, minDuration time.Duration) []lockuptypes.PeriodLock {
-	filteredLocks := make([]lockuptypes.PeriodLock, 0, len(locks))
-	for _, lock := range locks {
-		if lock.Duration >= minDuration {
-			filteredLocks = append(filteredLocks, lock)
+func FilterLocksByMinDuration(locks []lockuptypes.PeriodLock, minDuration time.Duration) []*lockuptypes.PeriodLock {
+	filteredLocks := make([]*lockuptypes.PeriodLock, 0, len(locks))
+	for i := range locks {
+		if locks[i].Duration >= minDuration {
+			filteredLocks = append(filteredLocks, &locks[i])
 		}
 	}
 	return filteredLocks

x/incentives/keeper/iterator_test.go

@@ -0,0 +1,37 @@
+package keeper_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/osmosis-labs/osmosis/v21/x/incentives/keeper"
+	lockuptypes "github.com/osmosis-labs/osmosis/v21/x/lockup/types"
+)
+
+// This test validates that the FilterLocksByMinDuration function
+// copies the correct locks when filtering.
+// It helps to ensure that we do not use a wrong pointer by referencing
+// a loop variable.
+func TestFilterLocksByMinDuration(t *testing.T) {
+	const (
+		numLocks    = 3
+		minDuration = 2
+	)
+
+	locks := make([]lockuptypes.PeriodLock, numLocks)
+	for i := 0; i < numLocks; i++ {
+		locks[i] = lockuptypes.PeriodLock{
+			ID:       uint64(i + 1),
+			Duration: minDuration,
+		}
+	}
+
+	filteredLocks := keeper.FilterLocksByMinDuration(locks, minDuration)
+
+	require.Equal(t, len(locks), len(filteredLocks))
+
+	for i := 0; i < len(locks); i++ {
+		require.Equal(t, locks[i].ID, filteredLocks[i].ID)
+	}
+}

x/lockup/types/lock.go

@@ -65,7 +65,7 @@ func (p PeriodLock) SingleCoin() (sdk.Coin, error) {
 
 // TODO: Can we use sumtree instead here?
 // Assumes that caller is passing in locks that contain denom
-func SumLocksByDenom(locks []PeriodLock, denom string) osmomath.Int {
+func SumLocksByDenom(locks []*PeriodLock, denom string) osmomath.Int {
 	sumBi := big.NewInt(0)
 	// validate the denom once, so we can avoid the expensive validate check in the hot loop.
 	err := sdk.ValidateDenom(denom)