Panic on division by zero

[chfast]

Not compatible with EVM but useful if someone wants to use this library for other purposes.

[codecov-io]

Codecov Report

Merging #62 into master will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master       #62   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines          669       673    +4     
=========================================
+ Hits           669       673    +4     

[holiman]

Not compatible with EVM but useful if someone wants to use this library for other purposes.

Well... are you sure we want this then? I mean, we'll have to add the corresponding check in the evm-layer, and the panic prevents compiler inlining

[chfast]

Well... are you sure we want this then? I mean, we'll have to add the corresponding check in the evm-layer, and the panic prevents compiler inlining

I'm not sure, but wanted to know how big the change is. But I don't believe panic prevents inlining.
Do you want me to add benchmarks for "no-panic" wrappers of these?

[chfast]

• master: version prior the change
• panic: the new implementation
• nopanic: the new implementation wrapped to return 0 on division by 0

master to panic

name                  old time/op  new time/op  delta
Div/small/uint256-6   11.6ns ± 1%  11.4ns ± 0%  -1.94%  (p=0.000 n=8+8)
Div/mod64/uint256-6   62.7ns ± 0%  62.7ns ± 0%    ~     (p=0.206 n=9+9)
Div/mod128/uint256-6  98.0ns ± 0%  97.6ns ± 0%  -0.45%  (p=0.000 n=9+10)
Div/mod192/uint256-6  88.1ns ± 0%  87.8ns ± 0%  -0.30%  (p=0.000 n=9+9)
Div/mod256/uint256-6  74.6ns ± 0%  74.3ns ± 0%  -0.40%  (p=0.000 n=9+8)
Mod/small/uint256-6   13.9ns ± 0%  13.7ns ± 0%  -1.44%  (p=0.000 n=10+9)
Mod/mod64/uint256-6   65.0ns ± 0%  65.0ns ± 0%    ~     (p=0.218 n=10+8)
Mod/mod128/uint256-6   101ns ± 0%   100ns ± 0%  -0.99%  (p=0.000 n=10+10)
Mod/mod192/uint256-6  90.7ns ± 0%  90.3ns ± 0%  -0.44%  (p=0.000 n=8+8)
Mod/mod256/uint256-6  77.4ns ± 1%  77.4ns ± 0%    ~     (p=0.121 n=10+9)

panic to nopanic

name                  old time/op  new time/op  delta
Div/small/uint256-6   11.4ns ± 0%  13.0ns ± 0%  +14.04%  (p=0.000 n=8+10)
Div/mod64/uint256-6   62.7ns ± 0%  63.4ns ± 0%   +1.12%  (p=0.000 n=9+9)
Div/mod128/uint256-6  97.6ns ± 0%  98.2ns ± 0%   +0.71%  (p=0.000 n=10+10)
Div/mod192/uint256-6  87.8ns ± 0%  88.4ns ± 0%   +0.68%  (p=0.000 n=9+8)
Div/mod256/uint256-6  74.3ns ± 0%  75.1ns ± 0%   +1.08%  (p=0.000 n=8+9)
Mod/small/uint256-6   13.7ns ± 0%  14.1ns ± 0%   +2.92%  (p=0.000 n=9+10)
Mod/mod64/uint256-6   65.0ns ± 0%  65.7ns ± 0%   +1.08%  (p=0.000 n=8+10)
Mod/mod128/uint256-6   100ns ± 0%   101ns ± 0%   +1.00%  (p=0.000 n=10+9)
Mod/mod192/uint256-6  90.3ns ± 0%  91.2ns ± 0%   +0.96%  (p=0.000 n=8+9)
Mod/mod256/uint256-6  77.4ns ± 0%  77.9ns ± 0%   +0.65%  (p=0.000 n=9+8)

master to nopanic

name                  old time/op  new time/op  delta
Div/small/uint256-6   11.6ns ± 1%  13.0ns ± 0%  +11.83%  (p=0.000 n=8+10)
Div/mod64/uint256-6   62.7ns ± 0%  63.4ns ± 0%   +1.17%  (p=0.000 n=9+9)
Div/mod128/uint256-6  98.0ns ± 0%  98.2ns ± 0%   +0.26%  (p=0.000 n=9+10)
Div/mod192/uint256-6  88.1ns ± 0%  88.4ns ± 0%   +0.38%  (p=0.000 n=9+8)
Div/mod256/uint256-6  74.6ns ± 0%  75.1ns ± 0%   +0.67%  (p=0.000 n=9+9)
Mod/small/uint256-6   13.9ns ± 0%  14.1ns ± 0%   +1.44%  (p=0.000 n=10+10)
Mod/mod64/uint256-6   65.0ns ± 0%  65.7ns ± 0%   +1.12%  (p=0.000 n=10+10)
Mod/mod128/uint256-6   101ns ± 0%   101ns ± 0%     ~     (all equal)
Mod/mod192/uint256-6  90.7ns ± 0%  91.2ns ± 0%   +0.51%  (p=0.000 n=8+9)
Mod/mod256/uint256-6  77.4ns ± 1%  77.9ns ± 0%   +0.66%  (p=0.000 n=10+8)

[holiman]

Alternative idea: have DivNoPanic be the basic one that returns 0 on division by zero, and add Div as a wrapper which checks the zero and panics, if needed.
Then we can use DivNoPanic from the evm, and have it be the fastest implementation.

[chfast]

Alternative idea: have DivNoPanic be the basic one that returns 0 on division by zero, and add Div as a wrapper which checks the zero and panics, if needed.
Then we can use DivNoPanic from the evm, and have it be the fastest implementation.

Started doing this in #64, but that's a lot of changes. There are SDiv(), SMod(), AddMod() and MulMod() left.

I think I would leave it as is.

[holiman]

I'm fine with leaving it as is
(if we change it to be as it is in this PR, doesn't that also mean that we start getting panics in SDiv, SMod et al?)

[chfast]

I'm fine with leaving it as is
(if we change it to be as it is in this PR, doesn't that also mean that we start getting panics in SDiv, SMod et al?)

In this PR we are getting panics for all 6 division methods (if not that's a bug).

But by "as is" I mean dropping this PR and go with what we currently have in master.

[holiman]

In this PR we are getting panics for all 6 division methods (if not that's a bug).

Sounds very much like something we don't want in the evm, so I think we should leave it as is on master, and not add complexity just for the sake of potential other users of the library