Skip to content
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

Possible defect on malaligned calc #9

Open
thomasmodeneis opened this issue Mar 19, 2018 · 1 comment
Open

Possible defect on malaligned calc #9

thomasmodeneis opened this issue Mar 19, 2018 · 1 comment

Comments

@thomasmodeneis
Copy link

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 ?

@erwanor
Copy link

erwanor commented Apr 1, 2018

@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:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants