From c3ec05a58f53165462978842141b29bab1394cff Mon Sep 17 00:00:00 2001 From: grandizzy <38490174+grandizzy@users.noreply.github.com> Date: Mon, 24 Apr 2023 06:52:33 +0300 Subject: [PATCH] Fenwick gas improvements (#761) * Calcualte current index only once * Declare vars outside for / while loops --- src/libraries/internal/Deposits.sol | 48 ++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/src/libraries/internal/Deposits.sol b/src/libraries/internal/Deposits.sol index 57f721e82..a9b90e19c 100644 --- a/src/libraries/internal/Deposits.sol +++ b/src/libraries/internal/Deposits.sol @@ -40,13 +40,17 @@ library Deposits { // 2- We often need to precisely change the value in the tree, avoiding the rounding that dividing by scale(index). // This is more relevant to unscaledRemove(...), where we need to ensure the value is precisely set to 0, but we // also prefer it here for consistency. - + + uint256 value; + uint256 scaling; + uint256 newValue; + while (index_ <= SIZE) { - uint256 value = deposits_.values[index_]; - uint256 scaling = deposits_.scaling[index_]; + value = deposits_.values[index_]; + scaling = deposits_.scaling[index_]; // Compute the new value to be put in location index_ - uint256 newValue = value + unscaledAddAmount_; + newValue = value + unscaledAddAmount_; // Update unscaledAddAmount to propogate up the Fenwick tree // Note: we can't just multiply addAmount_ by scaling[i_] due to rounding @@ -81,22 +85,27 @@ library Deposits { // We construct the target sumIndex_ bit by bit, from MSB to LSB. lowerIndexSum_ always maintains the sum // up to the current value of sumIndex_ uint256 lowerIndexSum; + uint256 curIndex; + uint256 value; + uint256 scaling; + uint256 scaledValue; while (i > 0) { // Consider if the target index is less than or greater than sumIndex_ + i - uint256 value = deposits_.values[sumIndex_ + i]; - uint256 scaling = deposits_.scaling[sumIndex_ + i]; + curIndex = sumIndex_ + i; + value = deposits_.values[curIndex]; + scaling = deposits_.scaling[curIndex]; // Compute sum up to sumIndex_ + i - uint256 scaledValue = + scaledValue = lowerIndexSum + (scaling != 0 ? (runningScale * scaling * value + 5e35) / 1e36 : Maths.wmul(runningScale, value)); if (scaledValue < targetSum_) { // Target value is too small, need to consider increasing sumIndex_ still - if (sumIndex_ + i <= MAX_FENWICK_INDEX) { + if (curIndex <= MAX_FENWICK_INDEX) { // sumIndex_+i is in range of Fenwick prices. Target index has this bit set to 1. - sumIndex_ += i; + sumIndex_ = curIndex; lowerIndexSum = scaledValue; } } else { @@ -227,23 +236,26 @@ library Deposits { // Used to terminate loop. We don't need to consider final 0 bits of sumIndex_ uint256 indexLSB = lsb(sumIndex_); + uint256 curIndex; while (j >= indexLSB) { + curIndex = index + j; + // Skip considering indices outside bounds of Fenwick tree - if (index + j > SIZE) continue; + if (curIndex > SIZE) continue; // We are considering whether to include node index + j in the sum or not. Either way, we need to scaling[index + j], // either to increment sum_ or to accumulate in runningScale - uint256 scaled = deposits_.scaling[index + j]; + uint256 scaled = deposits_.scaling[curIndex]; if (sumIndex_ & j != 0) { // node index + j of tree is included in sum - uint256 value = deposits_.values[index + j]; + uint256 value = deposits_.values[curIndex]; // Accumulate in sum_, recall that scaled==0 means that the scale factor is actually 1 sum_ += scaled != 0 ? (runningScale * scaled * value + 5e35) / 1e36 : Maths.wmul(runningScale, value); // Build up index bit by bit - index += j; + index = curIndex; // terminate if we've already matched sumIndex_ if (index == sumIndex_) break; @@ -352,9 +364,15 @@ library Deposits { // 2- We may already have computed the scale factor, so we can avoid duplicate traversal unscaledDepositValue_ = deposits_.values[index_]; + uint256 curIndex; + uint256 value; + uint256 scaling; + while (j & index_ == 0) { - uint256 value = deposits_.values[index_ - j]; - uint256 scaling = deposits_.scaling[index_ - j]; + curIndex = index_ - j; + + value = deposits_.values[curIndex]; + scaling = deposits_.scaling[curIndex]; unscaledDepositValue_ -= scaling != 0 ? Maths.wmul(scaling, value) : value; j = j << 1;