* both 'to' and 'from' are either Null or Hash160, there is no other
possibility for valid NEP-17. So returning util.Uint160{} in case of
parsing error is wrong.
* but this is what allowed burns/mints to work at the expense of error
allocation inside of util.Uint160DecodeBytesBE()
* Uint160 can technically fit into regular VM integer, so even though it'd be
quite surprising to see it there, TryBytes() is more correct (and easier!)
to use
* same thing with `amount`, we have `TryInteger()` that easily covers all
possible cases and does appropriate error checking inside
Squash (*DAO).StoreAsTransaction and
(*DAO).StoreConflictingTransactions. It's better to keep them this way,
because StoreAsTransaction is always followed by
StoreConflictingTransactions, so it's an atomic operation.
The logic wasn't changed.
We know it already, but with current loading code VM will hash it once
more. It doesn't help a lot and still it costs nothing to avoid this
overhead.
name old time/op new time/op delta
VerifyWitness-8 93.4µs ± 3% 92.7µs ± 2% ~ (p=0.353 n=10+10)
name old alloc/op new alloc/op delta
VerifyWitness-8 3.43kB ± 0% 3.40kB ± 0% -0.70% (p=0.000 n=9+9)
name old allocs/op new allocs/op delta
VerifyWitness-8 67.0 ± 0% 66.0 ± 0% -1.49% (p=0.000 n=10+10)
Block processing consists of:
* saving block/transactions to the DB
* executing blocks/transactions
* processing notifications/saving AERs
* updating MPT
* atomically updating Blockchain state
Of these the first one is completely independent of others, it can be done in
a separate routine easily. The third one technically depends on the second,
it just doesn't have data until something is executed. At the same time it
doesn't affect future executions in any way, so we can offload
AER/notification processing to separate goroutine (while the main thread
proceeds with other transactions).
MPT update depends on all executions, so it can't be offloaded, but it can be
done concurrently to AER processing. And only the last thing actually needs
all previous ones to be finished, so it's a natural synchronization point.
So we spawn two additional routines and let the main one execute transactions
and update MPT as fast as it can. While technically all of these routines
could share single DAO (they are working with different KV sets) benchmarking
shows that using separate DAOs and then persisting them to lower one actually
works about 7-8%% better. At the same time we can simplify DAOs used, Cached
one is only relevant for AER processing because it caches NEP-17 tracking
data, everything else can do just fine with Simple.
The change was tested for performance with neo-bench (single node, 10 workers,
LevelDB) on two machines and block dump processing (RC4 testnet up to 50825
with VerifyBlocks set to false) on i7-8565U. neo-bench creates huge blocks
with lots of transactions while RC4 dump mostly consists of empty blocks.
Reference results (06c3dda5d1):
Ryzen 9 5950X:
RPS ≈ 20059.569 21186.328 20158.983 ≈ 20468 ± 3.05%
TPS ≈ 19544.993 20585.450 19658.338 ≈ 19930 ± 2.86%
CPU ≈ 18.682% 23.877% 22.852% ≈ 21.8 ± 12.62%
Mem ≈ 618.981MB 559.246MB 541.539MB ≈ 573 ± 7.08%
Core i7-8565U:
RPS ≈ 5927.082 6526.739 6372.115 ≈ 6275 ± 4.96%
TPS ≈ 5899.531 6477.187 6329.515 ≈ 6235 ± 4.81%
CPU ≈ 56.346% 61.955% 58.125% ≈ 58.8 ± 4.87%
Mem ≈ 212.191MB 224.974MB 205.479MB ≈ 214 ± 4.62%
DB restore:
real 0m12.683s 0m13.222s 0m13.382s ≈ 13.096 ± 2.80%
user 0m18.501s 0m19.163s 0m19.489s ≈ 19.051 ± 2.64%
sys 0m1.404s 0m1.396s 0m1.666s ≈ 1.489 ± 10.32%
After the change:
Ryzen 9 5950X:
RPS ≈ 23056.899 22822.015 23006.543 ≈ 22962 ± 0.54%
TPS ≈ 22594.785 22292.071 22800.857 ≈ 22562 ± 1.13%
CPU ≈ 24.262% 23.185% 25.921% ≈ 24.5 ± 5.65%
Mem ≈ 614.254MB 613.204MB 555.491MB ≈ 594 ± 5.66%
Core i7-8565U:
RPS ≈ 6378.702 6423.927 6363.788 ≈ 6389 ± 0.49%
TPS ≈ 6327.072 6372.552 6311.179 ≈ 6337 ± 0.50%
CPU ≈ 57.599% 58.622% 59.737% ≈ 58.7 ± 1.82%
Mem ≈ 198.697MB 188.746MB 200.235MB ≈ 196 ± 3.18%
DB restore:
real 0m13.576s 0m13.334s 0m12.757s ≈ 13.222 ± 3.18%
user 0m19.113s 0m19.490s 0m20.197s ≈ 19.600 ± 2.81%
sys 0m2.211s 0m1.558s 0m1.559s ≈ 1.776 ± 21.21%
On Ryzen 9 we've got 12% better RPS, 13% better TPS with 12% CPU and 3% RAM
more used. Core i7-8565U changes don't seem to be statistically significant:
1.8% more RPS, 1.6% more TPS with about the same CPU and 8.5% less RAM
used. It also is 1% worse in DB restore time.
The result is somewhat expected, on a powerful machine with lots of spare
cores we get 10%+ better results while on average resource-constrained laptop it
doesn't change much (the machine is already saturated). Overall, this seems to
be worthwhile.
Request NEP17 balances from a set of NEP17 contracts instead of getting
them from storage. LastUpdatedBlock tracking remains untouched, because
there's no way to retrieve it dynamically.
Balances are to be removed from state.NEP17TransferInfo, so the remnant
fields are NextTransferBatch, NewBatch and a map of LastUpdatedBlocks.
These fields are more staff-related.
Also rename dao.[Get, Put, put]NEP17Balances and STNEP17Balances
preffix.
Also rename NEP17TransferInfo.Trackers to LastUpdatedBlockTrackers
because NEP17TransferInfo.Balances are to be removed.
We have a lot of native contract types that are converted to stack items
before serialization, then deserialized as stack items and converted back to
regular structures. stackitem.Convertible allows to remove a lot of repetitive
io.Serializable code.
This also introduces to/from converter in testserdes which unfortunately
required to change util tests to avoid circular references.
Problem: with StateRootInHeader setting on only one header of height N+1
can be added to the chain of height N, because we need local stateroot
to verify headers (which is calculated for the last stored block N).
Thus, adding chunk of headers starting from the current chain's heigh
is impossible and (*Blockchain).AddHeaders doesn't have much sense.
Solution: verify header.PrevStateRoot only for header N+1. Rest of the
headers should be added without PrevStateRoot verification.
It was completely ruined by bf20db09e0. MPT was
updating bc.dao directly which shouldn't ever happen, it must write into the
same cache and then Persist these KVs as usual.
