Skip to content

Reorganise structs layout to improve mem padding #437

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

Merged
merged 2 commits into from
Aug 27, 2018
Merged

Reorganise structs layout to improve mem padding #437

merged 2 commits into from
Aug 27, 2018

Conversation

kuba--
Copy link
Contributor

@kuba-- kuba-- commented Aug 24, 2018

Signed-off-by: kuba-- kuba@sourced.tech
Closes #436

This is a small thing, but maybe worth to consider (WDYT)?. As an example I re-organized couple structs to make memory alignment optimal - less work for GC.

Changes:

  • Server size was 192, now 176
  • squashReposIter size was 80, now 72
  • squashRefIter size was size 112, now 104
  • squashCommitFilesIter size was 120, now 112
  • squashIndexCommitFilesIter size was 128, now 120

@kuba-- kuba-- added the proposal proposal for new additions or changes label Aug 24, 2018
@kuba-- kuba-- requested a review from a team August 24, 2018 08:25
Copy link
Contributor

@mcarmonaa mcarmonaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't aware about this and I feel curious, what is the reasoning behind it? How does it work? grouping fields by type?

@kuba--
Copy link
Contributor Author

kuba-- commented Aug 24, 2018

@mcarmonaa - couple readings:

(or just by searching about go memory alignment), but briefly - memory aligns to 8 bytes (most likely but it depends on architecture), so if you layout your structure like:

type MyData struct {
	a bool
	b int64
	c int32
	d bool
	e string
}

because of 8bytes alignment, compiler will introduce memory padding, so it will look:

MyData.a bool: 0-1 (size 1, align 1)
padding: 1-8 (size 7, align 0) # <---------------------
MyData.b int64: 8-16 (size 8, align 8)
MyData.c int32: 16-20 (size 4, align 4)
MyData.d bool: 20-21 (size 1, align 1)
padding: 21-24 (size 3, align 0) # <--------------------
MyData.e string: 24-40 (size 16, align 8)

and thats why you'll loose 10 bytes.

But if you layout your structure for instance as follows:

type MyData struct {
	e string
	b int64
	c int32
	a bool
	d bool
}

you'll loose just 2 bytes (I think the last padding is OK):

MyData.e string: 0-16 (size 16, align 8)
MyData.b int64: 16-24 (size 8, align 8)
MyData.c int32: 24-28 (size 4, align 4)
MyData.a bool: 28-29 (size 1, align 1)
MyData.d bool: 29-30 (size 1, align 1)
padding: 30-32 (size 2, align 0) # <----------

Generally, the simplest thing what I do I just layout structure from the biggest type to the smallest.

@mcarmonaa
Copy link
Contributor

Thank you very much @kuba-- for such a good explanation and the links! ✨

engine *sqle.Engine
pool *gitbase.RepositoryPool
name string
SkipGitErrors bool // SkipGitErrors disables failing when Git errors are found.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is changing the output of gitbase server --help command? If yes, could you change it on the documentation too please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ajnavarro - done

kuba-- and others added 2 commits August 27, 2018 11:20
@ajnavarro ajnavarro merged commit f4e1c09 into src-d:master Aug 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal proposal for new additions or changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants