maligned icon indicating copy to clipboard operation
maligned copied to clipboard

Possible defect on malaligned calc

Open thomasmodeneis opened this issue 6 years ago • 1 comments

Given the code:

model.go

package addrs

// ASDExchanger bla bla
type ASDExchanger struct {
	ASDBtcExchangeRate    string `mapstructure:"asd_btc_exchange_rate"`
	ASDBtcExchangeRateUSD string `mapstructure:"asd_btc_exchange_rate_usd"`
	ASDBtcExchangeLabel   string `mapstructure:"asd_btc_exchange_label"`
	ASDBtcExchangeEnabled bool   `mapstructure:"asd_btc_exchange_enabled"`

	ASDEthExchangeRate    string `mapstructure:"asd_eth_exchange_rate"`
	ASDEthExchangeRateUSD string `mapstructure:"asd_eth_exchange_rate_usd"`
	ASDEthExchangeLabel   string `mapstructure:"asd_eth_exchange_label"`
	ASDEthExchangeEnabled bool   `mapstructure:"asd_eth_exchange_enabled"`

	ASDSkyExchangeRate    string `mapstructure:"asd_sky_exchange_rate"`
	ASDSkyExchangeRateUSD string `mapstructure:"asd_sky_exchange_rate_usd"`
	ASDSkyExchangeLabel   string `mapstructure:"asd_sky_exchange_label"`
	ASDSkyExchangeEnabled bool   `mapstructure:"asd_sky_exchange_enabled"`

	ASDWavesExchangeRate    string `mapstructure:"asd_waves_exchange_rate"`
	ASDWavesExchangeRateUSD string `mapstructure:"asd_waves_exchange_rate_usd"`
	ASDWavesExchangeLabel   string `mapstructure:"asd_waves_exchange_label"`
	ASDWavesExchangeEnabled bool   `mapstructure:"asd_waves_exchange_enabled"`

	ASDWavesASDExchangeRate    string `mapstructure:"asd_waves_asd_exchange_rate"`
	ASDWavesASDExchangeRateUSD string `mapstructure:"asd_waves_asd_exchange_rate_usd"`
	ASDWavesASDExchangeLabel   string `mapstructure:"asd_waves_asd_exchange_label"`
	ASDWavesASDExchangeEnabled bool   `mapstructure:"asd_waves_asd_exchange_enabled"`
}

When I run gometalinter and maligned:

gometalinter --deadline=3m -j 2 --disable-all --tests --vendor \                                                                                             
        -E maligned \       
        ./...

Then throws me err:

model.go:4:19:warning: struct of size 280 could be 248 (maligned)

When I remove the bool and or replace with string, it works with no errs.

Any ideas ?

thomasmodeneis avatar Mar 19 '18 11:03 thomasmodeneis

@thomasmodeneis The package is working as intended. It warns you that your ASDExchanger struct layout is inefficient.

Long story short, the fields of your struct are word aligned. On a x64 machines that means they are padded to 8 bytes. What maligned is telling you is that your struct size is 280 bytes while it could be 248 if you re-organized your fields.

The reason you do not get a warning if you replace bool by string is that in that case there are no possible optimizations.

If you do not want to get a warning for that structure, you should move the bool fields to the end of your struct.

Now, the question is whether you actually want to do that. Grouping fields together as you did improve readability. If performance isn't an absolute priority then I would just leave it that way. It's a good trade-off.

You should read the following if you want to know more:

  • https://dave.cheney.net/2015/10/09/padding-is-hard

  • https://go101.org/article/memory-layout.html

erwanor avatar Apr 01 '18 11:04 erwanor