Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache the length of actions when looping over them #3

Open
cleanunicorn opened this issue Jul 27, 2021 · 0 comments
Open

Cache the length of actions when looping over them #3

cleanunicorn opened this issue Jul 27, 2021 · 0 comments

Comments

@cleanunicorn
Copy link
Member

cleanunicorn commented Jul 27, 2021

Description

When the total reported amount of assets is estimated by the actions, the storage variable actions.length is used. This value does not change over time (in this loop) and can be cached in a local variable, instead of retrieving the value from the storage.

https://github.com/monoceros-alpha/review-opyn-perp-vault-templates-2021-07/blob/518e4f6d174cae6ee75e316ad56789aaeb695069/code/contracts/core/OpynPerpVault.sol#L393-L395

Similarly for _closeAndWithdraw():

https://github.com/monoceros-alpha/review-opyn-perp-vault-templates-2021-07/blob/3d44603300dd9abffe5a1c1e1c2647e9f6b80c7b/code/contracts/core/OpynPerpVault.sol#L420

Recommendation

Cache the length locally and use the local variable in the loop.

Reference

An example was created to illustrate the gas difference with or without length cache.

Not using a cache, for a set of 10 iterations uses 49157 gas.

contract SumNumbers {
    uint[] public numbers;
    
    constructor(uint size) {
        // Add some data to work with
        for (uint i = 0; i < size; i++) {
            numbers.push(i);
        }
    }
    
    function sumNumbers() public view returns (uint) {
        uint sum;
        for(uint i = 0; i < numbers.length; i++) {
            sum += numbers[i];
        }
        
        return sum;
    }
}

Using a cache, for the same set of 10 iterations uses 48168 gas.

contract SumNumbersWithCache {
    uint[] public numbers;
    
    constructor(uint size) {
        // Add some data to work with
        for (uint i = 0; i < size; i++) {
            numbers.push(i);
        }
    }
    
    function sumNumbers() public view returns (uint) {
        uint sum;
        uint size = numbers.length;
        
        for(uint i = 0; i < size; i++) {
            sum += numbers[i];
        }
        
        return sum;
    }
}

Both contracts were compiled with 200 optimization rounds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants