Skip to content

Commit

Permalink
Fenwick gas improvements (#761)
Browse files Browse the repository at this point in the history
* Calcualte current index only once

* Declare vars outside for / while loops
  • Loading branch information
grandizzy authored Apr 24, 2023
1 parent 48791b5 commit c3ec05a
Showing 1 changed file with 33 additions and 15 deletions.
48 changes: 33 additions & 15 deletions src/libraries/internal/Deposits.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit c3ec05a

Please sign in to comment.