optimization: save 800 gas on update calls

[prestwich]

Solidity doesn't optimize these SSTORE calls for you, you have to manually tell it not to call SSTORE if the data is not changing

[NoahZinsmeister]

does this not get metered out, because the storage slot value isn't changing?

[prestwich]

Not last time I checked

[prestwich]

From the yellowpaper.

[NoahZinsmeister]

ah, but note that these three variables live in a single storage slot. so while i believe this code actually produces 3 SSTOREs, the resulting gas cost should be 5000 regardless of whether or not we guard blockTimestampLast. verifying this at the moment!

[prestwich]

if 3 variables live in the slot, you'll still pay an SSTORE every time you update 1 of those variables

[prestwich]

Which is to say, this code saves you an SSTORE regardless of what else is packed in that slot.

[NoahZinsmeister]

my understanding is that this is no longer the case in istanbul because of the changes in EIP-2200

[prestwich]

Someone should update the yellowpaper then :)

Under 2200 semantics this still saves 800 gas

[NoahZinsmeister]

in theory i agree, but in practice, with compiler optimization on, i'm not sure it does.

see this test of the existing behavior vs the proposed. regardless, appreciate the PR, and we will do some digging to make sure we get this right!

[prestwich]

I would be pleasantly surprised if I could stop caring about some of these hand optimizations

[livnev]

@prestwich I think what you missed is that reserve0 and reserve1 and blockTimestampLast are packged into slot 8 in storage and are all written in one SSTORE, when the solc optimiser is enabled. Therefore, since we still have to write the former two, there will be an SSTORE in either case, so we do not save 800 gas. If you wanted to save gas, you would have to short circuit writing the entire slot, by also checking if reserve0 and reserve1 are both unchanged.

EDIT: for reference, the bytecode of that section looks like this (program counter in []):

[9365] SLOAD
[9398] PUSH32 0xffffffffffffffffffffffffffffffffffff0000000000000000000000000000
[9399] AND
[9414] PUSH14 0xffffffffffffffffffffffffffff
[9415] DUP9
[9416] DUP2
[9417] AND
[9418] SWAP2
[9419] SWAP1
[9420] SWAP2
[9421] OR
[9454] PUSH32 0xffffffff0000000000000000000000000000ffffffffffffffffffffffffffff
[9455] AND
[9471] PUSH15 0x010000000000000000000000000000
[9472] DUP9
[9473] DUP4
[9474] AND
[9475] DUP2
[9476] MUL
[9477] SWAP2
[9478] SWAP1
[9479] SWAP2
[9480] OR
[9509] PUSH28 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffff
[9510] AND
[9540] PUSH29 0x0100000000000000000000000000000000000000000000000000000000
[9545] PUSH4 0xffffffff
[9546] DUP8
[9547] AND
[9548] MUL
[9549] OR
[9550] SWAP3
[9551] DUP4
[9552] SWAP1
[9553] SSTORE

The first SLOAD is to retrieve the existing packed contents of the slot, after which the new values for each of the three variables are masked in and then stored with the single SSTORE at the end.

[prestwich]

Ah that's nice. When did that get updated?

[NoahZinsmeister]

closing as this is no longer relevant for reasons outlined in the comments.