types: {Int, Uint}.MarshalTo use an expensive (math/big.Int).Bytes() to detect if isZero instead of the cheaper .BitLen()

[odeke-em]

Summary of Bug

While auditing I noticed this code

cosmos-sdk/types/uint.go

Lines 160 to 176 in f8a6987

	func (u *Uint) MarshalTo(data []byte) (n int, err error) {
	if u.i == nil {
	u.i = new(big.Int)
	}
	if len(u.i.Bytes()) == 0 {
	copy(data, []byte{0x30})
	return 1, nil
	}
	bz, err := u.Marshal()
	if err != nil {
	return 0, err
	}
	copy(data, bz)
	return len(bz), nil
	}

for Int.MarshalTo and Uint.MarshalTo, but notice that it tries to check if a value is zero. That code invokes .Bytes() and checks the length, an allocation, just to only check if a value is zero. The math/big package provides a way to trivially check if a value is zero with .BitLen() == 0

Results

With just this small change, we see some good benefits

and then when we apply benchmarks over the entire project there are benefits

name                                                      old time/op    new time/op    delta
UintMarshal-8                                               11.0µs ±56%     8.1µs ±16%  -25.84%  (p=0.004 n=10+10)
RejectUnknownFields_parallel-8                               156ns ± 4%     158ns ± 1%   +1.87%  (p=0.043 n=10+7)
BcryptGenerateFromPassword/benchmark-security-param-9-8     30.6ms ± 1%    29.8ms ± 2%   -2.61%  (p=0.000 n=10+9)
BcryptGenerateFromPassword/benchmark-security-param-10-8    61.1ms ± 1%    58.9ms ± 0%   -3.68%  (p=0.000 n=10+8)
BcryptGenerateFromPassword/benchmark-security-param-11-8     123ms ± 1%     118ms ± 1%   -3.61%  (p=0.000 n=10+9)
BcryptGenerateFromPassword/benchmark-security-param-12-8     245ms ± 0%     236ms ± 1%   -3.41%  (p=0.000 n=9+10)
BcryptGenerateFromPassword/benchmark-security-param-13-8     489ms ± 0%     476ms ± 3%   -2.58%  (p=0.001 n=8+10)
BcryptGenerateFromPassword/benchmark-security-param-14-8     979ms ± 1%     940ms ± 1%   -4.00%  (p=0.000 n=10+9)
BcryptGenerateFromPassword/benchmark-security-param-15-8     1.97s ± 2%     1.90s ± 3%   -3.61%  (p=0.000 n=10+10)
CacheKVStoreIterator1000-8                                  76.0µs ± 0%   124.4µs ±40%  +63.63%  (p=0.003 n=8+10)
CacheKVStoreIterator100000-8                                33.3ms ± 2%    29.7ms ±13%  -10.78%  (p=0.004 n=9+8)
CacheKVStoreGetNoKeyFound-8                                  733ns ± 5%    1284ns ± 8%  +75.12%  (p=0.000 n=9+8)
IAVLIteratorNext-8                                           487ns ± 3%     524ns ±13%   +7.59%  (p=0.001 n=9+10)
MultistoreSnapshot100K-8                                     3.12s ± 1%     3.17s ± 1%   +1.48%  (p=0.000 n=8+10)
MultistoreSnapshot1M-8                                       31.5s ± 0%     31.8s ± 2%   +0.91%  (p=0.004 n=8+10)
MultistoreSnapshotRestore100K-8                              5.17s ± 1%     5.21s ± 1%   +0.71%  (p=0.016 n=8+10)
CoinsAdditionIntersect/sizes:_A_1,_B_1000-8                 14.0µs ± 2%    13.8µs ± 0%   -1.65%  (p=0.000 n=10+9)
CoinsAdditionIntersect/sizes:_A_2,_B_1000-8                 14.2µs ± 1%    14.0µs ± 2%   -1.18%  (p=0.021 n=8+9)
CoinsAdditionNoIntersect/sizes:_A_1,_B_1-8                  48.3ns ± 1%    48.7ns ± 1%   +0.98%  (p=0.003 n=9+9)
CoinsAdditionNoIntersect/sizes:_A_5,_B_5-8                   496ns ± 1%     505ns ± 2%   +1.76%  (p=0.000 n=9+9)
CoinsAdditionNoIntersect/sizes:_A_1,_B_1000-8               14.2µs ± 3%    14.0µs ± 2%   -1.72%  (p=0.014 n=10+10)
Bech32ifyPubKey-8                                           9.09µs ± 1%    9.39µs ± 2%   +3.36%  (p=0.000 n=10+10)
GetPubKeyFromBech32-8                                       14.2µs ± 2%    14.7µs ± 4%   +3.44%  (p=0.001 n=10+10)
ParseCoin-8                                                 8.51µs ± 0%    8.69µs ± 3%   +2.13%  (p=0.000 n=9+10)
AccountMapperSetAccount-8                                   4.86µs ± 1%    4.92µs ± 3%   +1.38%  (p=0.008 n=9+9)
BlockProvision-8                                             391ns ± 0%     396ns ± 2%   +1.27%  (p=0.001 n=9+9)
NextInflation-8                                             1.34µs ± 0%    1.34µs ± 0%   +0.32%  (p=0.031 n=8+9)

name                                                      old speed      new speed      delta
UintMarshal-8                                              680kB/s ±40%   872kB/s ±17%  +28.24%  (p=0.003 n=10+10)
RejectUnknownFields_parallel-8                             605MB/s ± 4%   593MB/s ± 1%   -1.87%  (p=0.043 n=10+7)

name                                                      old alloc/op   new alloc/op   delta
UintMarshal-8                                                 440B ± 0%      392B ± 0%  -10.91%  (p=0.000 n=10+10)
CacheKVStoreIterator1000-8                                  17.5kB ± 0%    17.5kB ± 0%   +0.15%  (p=0.028 n=10+10)
CacheKVStoreIterator100000-8                                6.46MB ± 1%    6.31MB ±11%   -2.36%  (p=0.022 n=10+9)
CacheKVStoreGetNoKeyFound-8                                   220B ± 1%      258B ± 0%  +17.21%  (p=0.000 n=9+8)

name                                                      old allocs/op  new allocs/op  delta
UintMarshal-8                                                 31.0 ± 0%      25.0 ± 0%  -19.35%  (p=0.000 n=10+10)
CacheKVStoreIterator1000-8                                    23.0 ± 0%      23.6 ± 3%   +2.61%  (p=0.011 n=10+10)
CacheKVStoreIterator100000-8                                 32.2k ± 5%     29.4k ±43%   -8.44%  (p=0.014 n=10+9)

Let's use the cheaper and more efficient alternative

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned