-
Notifications
You must be signed in to change notification settings - Fork 111
eth: Add CoinID flags. #1156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
eth: Add CoinID flags. #1156
Conversation
b2afba3
to
2359e9f
Compare
Using pointers to |
Probably go-ethereum works with value args in places for the sweeter syntax (less If we have a |
Do you have some reading material about this? We often store pointers in structs, and in some cases it seems to just be better to use pointers to avoid unexpected bugs like #1124 |
In that issue embedding of a struct by value had a unique set of gotchas, especially when it contained a mutex. Although I'm really thinking about fields that are simple types rather than structs. For a field that is a simple type like an array, it makes sense to have the memory for all of the struct's data, including the array, be contiguous, from a single allocation. I suppose it depends on the specific case though. decred/dcrd@20a8ccc, although I don't uniformly object to passing small arrays by value. |
ty! Garbage collection is the main reason? |
Add TxIDFlag and SwapFlag in order to specify that a coin ID represents either a txid or a swap. IsTxIDCoinID and IsSwapCoinID package methods are included so that callers can easily discern if they have the right type of coin ID for the operation they are performing.
According to the golang docs, passing arrays by pointer is preferred as it does not copy the array every time. Use where possible.
9344f0a
to
22995b2
Compare
GC, but also memory access efficiency. Accessing adjacent memory addresses will often benefit from low-level cache lines, thus speeding up successive access to multiple fields. Go's 64-bit alignment of structs memory and elimination of excess padding for small (<8 byte) fields makes this even more likely. Optimizing for this in this application is surely not critical, but it's a reason. Also if we have a pointer to an array as a struct field, that's a separate heap allocated object. Might as well have the array just be a segment of the struct's memory. Of course there can be reasons why this isn't desirable, but in general I feel it's cleanest to just avoid the extra level of indirection, esp for small arrays like Hash or this common.Address type. However if we find ourselves doing lots of unnecessary copies or obtaining pointers to these fields, we might reconsider this approach. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a few thoughts.
related to #1021