field alignment for generated structs

[napei]

• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

Because of how memory allocation is done in go, it's possible to optimise the amount of memory that a struct takes by ordering fields based on their size. In the case of structs inside ./ent there is possibly some performance/memory benefits by ordering these during codegen.

Consider this struct (comments removed):

type Address struct { // 176 pointer bytes
	config     `json:"-"`
	ID         uuid.UUID    `json:"id"`
	CreateTime time.Time    `json:"create_time,omitempty"`
	UpdateTime time.Time    `json:"update_time,omitempty"`
	Line1      string       `json:"line1"`
	Suburb     string       `json:"suburb"`
	State      string       `json:"state"`
	Postcode   string       `json:"postcode"`
	Edges      AddressEdges `json:"edges"`
}

type Address struct { // 160 pointer bytes
	config     `json:"-"`
	CreateTime time.Time    `json:"create_time,omitempty"`
	UpdateTime time.Time    `json:"update_time,omitempty"`
	Postcode   string       `json:"postcode"`
	Line1      string       `json:"line1"`
	Suburb     string       `json:"suburb"`
	State      string       `json:"state"`
	Edges      AddressEdges `json:"edges"`
	ID         uuid.UUID    `json:"id"`
}

Not exactly a dramatic change, but quite possible could be a helpful change with more complex structs especially when part of the usage of ent is to load data into memory and do stuff with it.

Because it's an internal package inside golang, it's hard to use programmatically, instead needing to be made into a cli tool..perhaps the algorithms can be taken and re-used internally in ent.

package main

import (
	"golang.org/x/tools/go/analysis/multichecker"
	"golang.org/x/tools/go/analysis/passes/fieldalignment"
)

func main() {
	multichecker.Main(fieldalignment.Analyzer)
}

Example: running on an ent directory

$ align -fieldalignment ./ent

/home/napei/crvc-backend/ent/address.go:16:14: struct with 176 pointer bytes could be 160
/home/napei/crvc-backend/ent/address_delete.go:17:20: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/address_query.go:22:19: struct with 152 pointer bytes could be 136
/home/napei/crvc-backend/ent/address_query.go:479:21: struct with 104 pointer bytes could be 88
/home/napei/crvc-backend/ent/address_update.go:19:20: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/address_update.go:275:23: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/allergy.go:17:14: struct with 168 pointer bytes could be 136
/home/napei/crvc-backend/ent/allergy_delete.go:17:20: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/allergy_query.go:21:19: struct with 160 pointer bytes could be 136
/home/napei/crvc-backend/ent/allergy_query.go:482:21: struct with 104 pointer bytes could be 88
/home/napei/crvc-backend/ent/allergy_update.go:19:20: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/allergy_update.go:252:23: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/config.go:14:13: struct with 40 pointer bytes could be 32
/home/napei/crvc-backend/ent/ent.go:124:22: struct with 32 pointer bytes could be 24
/home/napei/crvc-backend/ent/ent.go:216:22: struct with 32 pointer bytes could be 24
/home/napei/crvc-backend/ent/flag.go:16:11: struct with 160 pointer bytes could be 144
/home/napei/crvc-backend/ent/flag_delete.go:17:17: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/flag_query.go:22:16: struct with 152 pointer bytes could be 136
/home/napei/crvc-backend/ent/flag_query.go:479:18: struct with 104 pointer bytes could be 88
/home/napei/crvc-backend/ent/flag_update.go:19:17: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/flag_update.go:262:20: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/mutation.go:41:22: struct with 168 pointer bytes could be 152
/home/napei/crvc-backend/ent/mutation.go:708:22: struct with 152 pointer bytes could be 136
/home/napei/crvc-backend/ent/mutation.go:1319:19: struct with 160 pointer bytes could be 144
/home/napei/crvc-backend/ent/mutation.go:1931:20: struct of size 216 could be 208
/home/napei/crvc-backend/ent/mutation.go:2821:18: struct of size 240 could be 232
/home/napei/crvc-backend/ent/mutation.go:3882:21: struct of size 192 could be 176
/home/napei/crvc-backend/ent/owner.go:18:12: struct with 256 pointer bytes could be 232
/home/napei/crvc-backend/ent/owner_delete.go:17:18: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/owner_query.go:22:17: struct with 168 pointer bytes could be 144
/home/napei/crvc-backend/ent/owner_query.go:544:19: struct with 104 pointer bytes could be 88
/home/napei/crvc-backend/ent/owner_update.go:20:18: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/owner_update.go:381:21: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/pet.go:17:10: struct with 280 pointer bytes could be 248
/home/napei/crvc-backend/ent/pet.go:50:15: struct with 32 pointer bytes could be 16
/home/napei/crvc-backend/ent/pet_delete.go:17:16: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/pet_query.go:23:15: struct with 168 pointer bytes could be 144
/home/napei/crvc-backend/ent/pet_query.go:549:17: struct with 104 pointer bytes could be 88
/home/napei/crvc-backend/ent/pet_update.go:21:16: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/pet_update.go:478:19: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/record.go:17:13: struct with 176 pointer bytes could be 152
/home/napei/crvc-backend/ent/record.go:32:18: struct with 56 pointer bytes could be 40
/home/napei/crvc-backend/ent/record_delete.go:17:19: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/record_query.go:24:18: struct with 176 pointer bytes could be 152
/home/napei/crvc-backend/ent/record_query.go:615:20: struct with 104 pointer bytes could be 88
/home/napei/crvc-backend/ent/record_update.go:21:19: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/record_update.go:375:22: struct with 72 pointer bytes could be 56
/home/napei/crvc-backend/ent/tx.go:13:9: struct with 184 pointer bytes could be 144

It's an extra checker for gopls that I only recently turned on and has fixed a few things for me so far.

[a8m]

Thanks for this proposal @napei.

I find it cleaner to provide this functionality using a standalone CLI tool that can be executed with another go generate rule. In this case, it will also work if additional fields will be added to the different structs with external templates.

Is there any CLI that uses fieldalignment and provides this reformatting?

[napei]

Is there any CLI that uses fieldalignment and provides this reformatting?

This code generates a CLI application which can run the analysis on a file. The only thing it doesn't do well is preserving comments.

Unfortunately fieldalignment is an internal package, or at least the ability to manually run analysers programmatically is internal and a pain.

[komuw]

https://github.com/orijtech/structslop possibly can

structslop --help
  -apply
    	apply suggested fixes (using -fix won't work)

[frederikhors]

Launching fieldalignment at the end of generation is a problem because it obviously changes the order of some structs generating issues like this:

In this case err is needed before err.Error() now.

And as @napei said it also deletes some comments (the ones on structs it aligns).

[vtolstov]

Comments are problem, but issues like you provide is not a problem. I think that generated code must use struct field names

[a8m]

Comments are problem, but issues like you provide is not a problem. I think that generated code must use struct field names

Agree. Feel free to send a patch or I can do it later. Thanks 🙏

[giautm]

Sent a patch: https://github.com/ent/ent/pull/2648

[frederikhors]

Can we check if there are PRs on golang repository for fieldalignment removing comments? Where to look?

[giautm]

I tested the PR with my project, now the fieldalignment can run success without any issue.

[frederikhors]

I tested the PR with my project, now the fieldalignment can run success without any issue.

Except for comments, right?

What do you think about align struct before applying comments during generation steps?

[giautm]

I tested the PR with my project, now the fieldalignment can run success without any issue.

Except for comments, right?

What do you think about align struct before applying comments during generation steps?

Yes, fieldalignment remove all comments on the struct fields. The comment and code was generate by Go template as GQLGen/other tools did. Do you have any idea to align the struct with Go template?

But, imo the comment should keep for public field only, because the private field was common to grouped by the intent - which may not in the right order after struct align.