Optimize control code #13354
Optimize control code #13354
Conversation
JoergAtGithub
force-pushed
the
optimize_control_code
branch
from
bc7700e
to
d986c3c
Compare
|
The reason that ControlDoublePrivate is so big is a nasty regression. It should be only for bites. The fix is here: #13355 |
| // The control value. | ||
| ControlValueAtomic<double> m_value; | ||
| // The default control value. | ||
| ControlValueAtomic<double> m_defaultValue; |
There was a problem hiding this comment.
How do we protect that for get cluttered again? Maybe define regions via comments?
There was a problem hiding this comment.
There was a problem hiding this comment.
I tried -Wpadded on CI, but it does not only warn about padding inside of the data structure, but also if the overall size is not matching the byte alignment size, e.g. any class with 6 bytes would need to filled up with 2 explicit padding bytes.
I found also a currently unused macro in
Line 15 in 15b4444
Maybe someone on Linux could try: https://linux.die.net/man/1/pahole
There was a problem hiding this comment.
Okay that makes sense I guess when the compiler expects you to nest these packed data structures. The only other potential solution I found was to generate two versions of the type, one packed (potentially with suboptimal alignment and then static_assert(sizeof(T) == sizeof(_T_packed)) (potentially with a bit of rounding to manually account for the overall size). I wouldn't know how to accomplish that without copy pasting or a macro (doing the copy-pasting automatically) though.
There was a problem hiding this comment.
Let's not over engineer it. Just a comment state informs there reader that the oder of the variables has been chosen to avoid memory gaps due to alignment is probably sufficient.
There was a problem hiding this comment.
It's already added:
Lines 203 to 205 in 6728c91
|
I merged in the changes from #13355, but I noticed, that the memory consumption of ControlDoublePrivate is much bigger in this Main branch, than in the 2.4 branch. 2.4: Main: This PR: |
|
I assume you build main with Qt6 and 2.4 with Qt5. Maybe the |
Yes, I did! |
|
Qt6 has no SSO. The number increased due to the use of a common QArrayDataPointer with holding two pointers one the header for implicit sharing, one to the actual data and one to the size which is now also 64 bit. This allows faster read only access and allows to the store strings longer than 2.147.483.648 characters (more than 480 Bibles) |
JoergAtGithub
force-pushed
the
optimize_control_code
branch
from
c3afbb8
to
e9c5c77
Compare
… to 448 byte First fields fit in 128 byte SSE register now
Co-authored-by: Daniel Schürmann <daschuer@mixxx.org>
JoergAtGithub
force-pushed
the
optimize_control_code
branch
from
e9c5c77
to
a1b5c25
Compare
…nction and made use of it where applicable. Unified the two ButtonMode enums to a common enum class.
github-actions
bot
added
effects
library
engine
ui
build
autodj
code quality
sync
broadcast
|
@daschuer Friendly Ping |
JoergAtGithub
force-pushed
the
optimize_control_code
branch
from
c11b5c5
to
0fb3ed0
Compare
…RT for unexpected ControlButtonMode values. Replaced header guard with #pragma once. Removed author and GPL comment blocks.
|
@daschuer please resolve your review |
|
Thank you! |
This PR collects several low level optimizations for our control code. See the single commit messages.
We have currently 36816 ControlDoublePrivate instances in Mixxx the memory realignment reduces the size from 464 byte to 448 byte, this translates to 0.5 MegaByte heap memory reduction.
Furthermore the first fields are aligned to fit into a 128 byte SSE register. But I guess there are very few operation, which can benefit from an SSE copy etc. (in theory we could further optimize for SSE, by reducing ControlValueAtomic from 136byte to 128byte).
Old memory alignment:
New memory alignment:
