Drop some useless allocations #2136
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It makes no sense and we're using Bytes() pretty often.
* 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
Functions are usually immediately replaced (and it's OK for them to be nil, searching through an array with length of zero is fine), Notifications are usually appended to (and are absolutely useless in verification contexts).
fyrchik
approved these changes
Aug 20, 2021
Comment on lines
-1050
to
-1057
| var to []byte | ||
| toValue := arr[1].Value() | ||
| // we don't have `to` set when we are burning tokens | ||
| if toValue != nil { | ||
| to, ok = toValue.([]byte) | ||
| if !ok { | ||
| return | ||
| } |
There was a problem hiding this comment.
Initial intention was to explicitly prohibit all other item types (loose type check). But I agree the amount of such events is negligible.
There was a problem hiding this comment.
Lacking neo-project/neo#1879 we really can get just about anything here. Also:
https://github.com/neo-project/neo-modules/blob/ef59a0eb27fb23c49610c0362e1fe7d9d0ec7839/src/RpcNep17Tracker/RpcNep17Tracker.cs#L135-L138
IIRC GetSpan() will do the same and extract uint160 from integer if it's there.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TXs ≈ 1000000
RPS ≈ 31678.809
RPC Errors ≈ 0 / 0.000%
TPS ≈ 31219.756
DefaultMSPerBlock = 1000
CPU ≈ 34.589%
Mem ≈ 528.872MB
😉
The key to single-node performance is allocations now (and a bit of locking of course).