Uniswap / v3-periphery

🦄 🦄 🦄 Peripheral smart contracts for interacting with Uniswap v3
https://uniswap.org
GNU General Public License v2.0
1.15k stars 1.06k forks source link

move loop invariant variables out from loop to save gas #341

Closed molly-ting closed 9 months ago

molly-ting commented 1 year ago

Moving loop invariant codes out from loop can save gas. In the following demo, it can save 265 units of gas when the length of arr is 100.

function test1(uint256[] memory arr) public {     
        for(uint256 i = 0; i < arr.length; i++) {
        }
    }

    function test2(uint256[] memory arr) public {
        uint256 len = arr.length;
        for(uint256 i = 0; i < len; i++) {
        }
    }
stale[bot] commented 1 year ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

songlh commented 1 year ago

@moodysalem @hensha256 @NoahZinsmeister Can you help take a look at this pull request? I think the pull request should be merged, since it can save gases while keeping the original functionality.

molly-ting commented 1 year ago

Initializing variables before the loop instead of inside the loop can also save gas. In the example below, function test4 consumes 553 units less gas than function test3 when the length of arr is 100.

    function test3(uint256[] memory arr) public {
        for(uint256 i = 0; i < arr.length; i++) {
            uint256 a = arr[i];
        }
    }

    function test4(uint256[] memory arr) public {
        uint256 a;
        for(uint256 i = 0; i < arr.length; i++) {
            a = arr[i];
        }
    }

I committed some changes to move these variable definitions out of the loop. Can someone take a look?

stale[bot] commented 11 months ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Toluphene commented 11 months ago

Anyway out or solution

Toluphene commented 11 months ago

The procedure shows it has been renew and remove from stale

stale[bot] commented 9 months ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.