Uniswap / v2-core

🦄 🦄 Core smart contracts of Uniswap V2
https://uniswap.org/docs
GNU General Public License v3.0
2.97k stars 3.18k forks source link

Optimization on Solidity constant #64

Closed k06a closed 4 years ago

k06a commented 4 years ago

Solidity constants are actually pure functions. I suspect you wish to avoid recalculating keccak256 on each constant access?

MicahZoltu commented 4 years ago

Where are the scripts that are generating these changes living at?

MrChico commented 4 years ago

With the solidity optimizer activated this constant is already precalculated:

Screen Shot 2020-04-16 at 12 46 43
d-xo commented 4 years ago

very curious that this change seems to produce a reduction in gas usage in the tests, even though the optimiser is already precomputing the selector.

NoahZinsmeister commented 4 years ago

this does seem to reduce gas usage per the tests, which is slightly frustrating given @MrChico 's observation. however, the change would be consistent with how PERMIT_TYPEHASH is precomputed in UniswapV2ERC20.sol.

livnev commented 4 years ago

this does seem to reduce gas usage per the tests, which is slightly frustrating given @MrChico 's observation. however, the change would be consistent with how PERMIT_TYPEHASH is precomputed in UniswapV2ERC20.sol.

Further investigation: @MrChico is correct that with the optimiser enabled, solc precomputes the hash and inlines it in the bytecode, thus avoiding a SHA3 op. However, solc still has junk code lying around left over from the unoptimised path, namely loading the string "transfer(address,uint256)" into memory. e.g. if we step back a bit from where @MrChico's screenshot was:

screenshot2678

the string "transfer(address,uint256)" is about to be loaded into memory, for no good reason. Later, you can see that string still sitting in memory when the precomputed hash is pushed:

screenshot2679

The version where the hash is simply inlined in the source code doesn't have this dead code, therefore saving a bit of gas from not doing the redundant stack and memory manipulations. 51 gas per _safeTransfer call is thus saved when hardcoding the constant in the source (I double checked this figure with hevm as well).