bbolt icon indicating copy to clipboard operation
bbolt copied to clipboard

Improve memory alignment

Open mrueg opened this issue 1 year ago • 5 comments

I run https://github.com/dkorunic/betteralign against the codebase and it showed the following improvements for memory alignment

    bbolt/db.go:38:9: struct of size 528 could be 496
    bbolt/db.go:1012:12: struct with 40 pointer bytes could be 24
    bbolt/db.go:1280:14: struct of size 104 could be 80
    bbolt/freelist.go:24:15: struct with 136 pointer bytes could be 112
    bbolt/node.go:12:11: struct with 88 pointer bytes could be 72
    bbolt/tx.go:27:9: struct with 192 pointer bytes could be 88
    bbolt/internal/common/inode.go:8:12: struct with 48 pointer bytes could be 32
    bbolt/internal/common/page.go:324:15: struct with 16 pointer bytes could be 8
    bbolt/cmd/bbolt/main.go:445:22: struct with 16 pointer bytes could be 8
    bbolt/cmd/bbolt/main.go:1489:19: struct with 160 pointer bytes could be 104
    bbolt/cmd/bbolt/main.go:1546:16: struct with 24 pointer bytes could be 16
    bbolt/internal/btesting/btesting.go:28:9: struct with 48 pointer bytes could be 40
    bbolt/tests/dmflakey/dmflakey.go:133:13: struct with 64 pointer bytes could be 56

mrueg avatar Jan 10 '24 23:01 mrueg

Thanks for the improvement.

  • For the DB struct: Let's keep it unchanged. All unexported fields should be put at the bottom. The stats Stats is just a special case.
  • For the BenchOptions , flakey and Tx: Suggest to keep them unchanged. It makes more sense to group the parameters together as they are.

ahrtr avatar Jan 12 '24 14:01 ahrtr

Please let me know if you have bandwidth to resolve my above comment https://github.com/etcd-io/bbolt/pull/673#issuecomment-1889409582

ahrtr avatar Feb 03 '24 14:02 ahrtr

Please let me know if you have bandwidth to resolve my above comment #673 (comment)

Thanks! I should have more bandwidth in the upcoming weeks. Before this gets merged, I would want to check https://github.com/etcd-io/bbolt/pull/691 and see if the change is actually improving anything.

mrueg avatar Feb 08 '24 22:02 mrueg

@mrueg if you don't mind, I'm currently test running this with @ahrtr's suggestions applied in my own fork with https://github.com/openshift/etcd/pull/278 - will report any e2e improvement I'm seeing.

I think the benchmark tooling will need a little more time :)

tjungblu avatar Jun 07 '24 08:06 tjungblu

Just to leave some quick feedback here. I had some trouble with Go 1.22 and our build pipelines, so my test basically just consisted of running the installation routines (aka an IDLE cluster).

There was no measurable improvement in memory usage or apiserver/etcd latency, I would only expect that under higher load though. That was also a bad apples/oranges test, because etcd 3.5 uses a different version of bbolt than what we're using here. I'll do another test run when I find some more time.

tjungblu avatar Jun 24 '24 07:06 tjungblu