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

opt: byte alignment, optimization from 144 to 136 byte #369

Merged
merged 1 commit into from
May 29, 2022
Merged

opt: byte alignment, optimization from 144 to 136 byte #369

merged 1 commit into from
May 29, 2022

Conversation

zhongweikang
Copy link
Contributor


name: Pull request
about: opt
title: 'byte alignment, optimization from 144 to 136 byte'
labels: ''
assignees: ''

1. Are you opening this pull request for bug-fixes, optimizations or new feature?

optimizations

2. Please describe how these code changes achieve your intention.

如题,字节对齐,每创建一个 conn 结构体,可节省 8 字节

3. Please link to the relevant issues (if any).

4. Which documentation changes (if any) need to be made/updated because of this PR?

connection.go

4. Checklist

  • I have squashed all insignificant commits.
  • I have commented my code for explaining package types, values, functions, and non-obvious lines.
  • I have written unit tests and verified that all tests passes (if needed).
  • I have documented feature info on the README (only when this PR is adding a new feature).
  • (optional) I am willing to help maintain this change if there are issues with it later.

@codecov
Copy link

codecov bot commented May 15, 2022

Codecov Report

Merging #369 (e4251b0) into dev (c296922) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev     #369   +/-   ##
=======================================
  Coverage   71.36%   71.36%           
=======================================
  Files          13       13           
  Lines        1390     1390           
=======================================
  Hits          992      992           
  Misses        326      326           
  Partials       72       72           
Flag Coverage Δ
unittests 71.36% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
connection.go 65.61% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c296922...e4251b0. Read the comment docs.

@panjf2000
Copy link
Owner

字节对齐这一块当初我是没打算弄,因为其实也节省不了多少空间,所以索性就按字段名长短规律性排列,如果现在要做对齐,那就得全部都一起对齐,只搞这一个没太大意义。

@zhongweikang
Copy link
Contributor Author

我确实整体检查一遍,只发现这一处地方

1个连接省8个字节,但成千上万上百万的连接就可观了,不必要的消耗,能省点是点?

@panjf2000
Copy link
Owner

你的修改还不是最优的

@panjf2000
Copy link
Owner

你要是想改,就调到最优,用 https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/fieldalignment 这个工具

@zhongweikang
Copy link
Contributor Author

我用 fieldalignment 检测了一下,这里的修改,对于 conn 已经是最优了(都是136 byte)

$ fieldalignment -fix ./
connection.go:38:11: struct of size 144 could be 136
engine.go:31:13: struct of size 128 could be 120
eventloop.go:39:16: struct with 136 pointer bytes could be 88
listener.go:34:15: struct with 104 pointer bytes could be 64
load_balancer.go:49:25: struct with 16 pointer bytes could be 8
options.go:44:14: struct of size 128 could be 104
client_test.go:36:19: struct with 56 pointer bytes could be 48
client_test.go:195:23: struct with 112 pointer bytes could be 72
gnet_test.go:237:17: struct with 96 pointer bytes could be 56
gnet_test.go:476:25: struct with 72 pointer bytes could be 64
gnet_test.go:945:29: struct with 88 pointer bytes could be 80
gnet_test.go:1005:16: struct of size 96 could be 88
listener_test.go:34:16: struct with 24 pointer bytes could be 16

对于其他的结构体,确实省不了几个字节,还会影响美观,建议不动了吧?

@panjf2000
Copy link
Owner

connection.go:38:11: struct with 136 pointer bytes could be 104

@zhongweikang
Copy link
Contributor Author

你用的 fieldalignment 是最新的 0.1.10 版本么,结果怎么还不一样

@panjf2000
Copy link
Owner

fieldalignment -fix . 命令之后的对齐如下:
image

你的 PR 里的修改不是最优的。

@panjf2000
Copy link
Owner

只是不知道为什么会把注释给删掉了

@panjf2000
Copy link
Owner

你也可以用 unsafe.Sizeof() 看看 fix 之后的实际大小是什么

@zhongweikang
Copy link
Contributor Author

使用 unsafe.Sizeof() 对比了我这个 PR,对比了你截图里的fix,以及对比了我自己的 fix,结果都是136 byte

除了两个 bool 字段各1 byte,其余字段都是 8 byte 的整数倍,所以理论上把两个 bool 放一起应该就可以了

@panjf2000
Copy link
Owner

使用 unsafe.Sizeof() 对比了我这个 PR,对比了你截图里的fix,以及对比了我自己的 fix,结果都是136 byte

除了两个 bool 字段各1 byte,其余字段都是 8 byte 的整数倍,所以理论上把两个 bool 放一起应该就可以了

奇怪,我把 isDatagram 移到 opened 下面,然后再跑 fieldalignment,就提示:connection.go:38:11: struct with 136 pointer bytes could be 104,你那边不是吗?

@zhongweikang
Copy link
Contributor Author

使用 unsafe.Sizeof() 对比了我这个 PR,对比了你截图里的fix,以及对比了我自己的 fix,结果都是136 byte
除了两个 bool 字段各1 byte,其余字段都是 8 byte 的整数倍,所以理论上把两个 bool 放一起应该就可以了

奇怪,我把 isDatagram 移到 opened 下面,然后再跑 fieldalignment,就提示:connection.go:38:11: struct with 136 pointer bytes could be 104,你那边不是吗?

isDatagram 移到 opened 下面之后,再跑 fieldalignment,确实变成了这个提升 136 -> 104

但是此时执行fmt.Printf("%v", unsafe.Sizeof(conn{}))结果仍然是 136

@panjf2000
Copy link
Owner

使用 unsafe.Sizeof() 对比了我这个 PR,对比了你截图里的fix,以及对比了我自己的 fix,结果都是136 byte
除了两个 bool 字段各1 byte,其余字段都是 8 byte 的整数倍,所以理论上把两个 bool 放一起应该就可以了

奇怪,我把 isDatagram 移到 opened 下面,然后再跑 fieldalignment,就提示:connection.go:38:11: struct with 136 pointer bytes could be 104,你那边不是吗?

isDatagram 移到 opened 下面之后,再跑 fieldalignment,确实变成了这个提升 136 -> 104

但是此时执行fmt.Printf("%v", unsafe.Sizeof(conn{}))结果仍然是 136

我自己试了一下的确是这样,不知道是这个工具的问题还是什么,不过第一眼粗略看起来这个工具重新排列之后把最小的两个 bool 放到最后的确有可能更节省内存,没仔细去算,最近比较忙,过两天我再准确算一下,或者你这两天有时间也可以再算算。

@zhongweikang
Copy link
Contributor Author

我又看了一下,fieldalignment 除了照顾了内存对齐,还优化了垃圾回收扫描的字节数pointer bytes
对于内存对齐而言,这里已经最优了
对于pointer bytes而言,确实可以继续优化,优化到fieldalignment所描述的数值

@panjf2000
Copy link
Owner

那就按照fieldalignment的结果做吧。

@zhongweikang
Copy link
Contributor Author

可以,但只调整conn吧? 这个pointer bytes扫描对性能影响很小。

@panjf2000
Copy link
Owner

可以,但只调整conn吧? 这个pointer bytes扫描对性能影响很小。

嗯,只优化 conn 就行了。

@zhongweikang
Copy link
Contributor Author

done

@panjf2000
Copy link
Owner

Thank~

@panjf2000 panjf2000 merged commit f70489f into panjf2000:dev May 29, 2022
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

Successfully merging this pull request may close these issues.

None yet

2 participants