Skip to content

Align structures 8 byte for 64-bit platforms #1702

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 1 commit into from
Jun 5, 2025

Conversation

GermanAizek
Copy link
Contributor

Migrated from here: https://salsa.debian.org/debian/htop/-/merge_requests/7

This PR will decrease costs copying, moving, and creating object-structures only for common 64bit processors due to the 8-byte data alignment.

Smaller size structure or class, higher chance putting into CPU cache. Most processors are already 64 bit, so the change won't make it any worse.

Pahole example:

  • Comment /* XXX {n} bytes hole, try to pack */ shows where optimization is possible by rearranging the order of fields structures and classes

Master branch

struct ScreenManager_ {
        int                        x1;                   /*     0     4 */
        int                        y1;                   /*     4     4 */
        int                        x2;                   /*     8     4 */
        int                        y2;                   /*    12     4 */
        Vector *                   panels;               /*    16     8 */
        const char  *              name;                 /*    24     8 */
        int                        panelCount;           /*    32     4 */

        /* XXX 4 bytes hole, try to pack */

        Header *                   header;               /*    40     8 */
        Machine *                  host;                 /*    48     8 */
        State *                    state;                /*    56     8 */
        /* --- cacheline 1 boundary (64 bytes) --- */
        _Bool                      allowFocusChange;     /*    64     1 */

        /* size: 72, cachelines: 2, members: 11 */
        /* sum members: 61, holes: 1, sum holes: 4 */
        /* padding: 7 */
        /* last cacheline: 8 bytes */
};

This PR

struct ScreenManager_ {
        int                        x1;                   /*     0     4 */
        int                        y1;                   /*     4     4 */
        int                        x2;                   /*     8     4 */
        int                        y2;                   /*    12     4 */
        _Bool                      allowFocusChange;     /*    16     1 */

        /* XXX 3 bytes hole, try to pack */

        int                        panelCount;           /*    20     4 */
        Vector *                   panels;               /*    24     8 */
        const char  *              name;                 /*    32     8 */
        Header *                   header;               /*    40     8 */
        Machine *                  host;                 /*    48     8 */
        State *                    state;                /*    56     8 */

        /* size: 64, cachelines: 1, members: 11 */
        /* sum members: 61, holes: 1, sum holes: 3 */
};

Info about technique:

https://hpc.rz.rptu.de/Tutorials/AVX/alignment.shtml

https://wr.informatik.uni-hamburg.de/_media/teaching/wintersemester_2013_2014/epc-14-haase-svenhendrik-alignmentinc-presentation.pdf

https://en.wikipedia.org/wiki/Data_structure_alignment

https://stackoverflow.com/a/20882083

https://zijishi.xyz/post/optimization-technique/learning-to-use-data-alignment/

Affected structs:

  • ScreenManager 72 to 64 bytes
  • Screen/DynamicIterator 24 to 16 bytes
  • Meter/DynamicIterator 24 to 16 bytes
  • IncMode 152 to 144 bytes
  • TraceScreen 64 to 56 bytes
  • LinuxProcessTable 120 to 112 bytes
  • FunctionBar 40 to 32 bytes

@BenBE BenBE added enhancement Extension or improvement to existing feature code quality ♻️ Code quality enhancement labels May 20, 2025
@BenBE BenBE added this to the 3.5.0 milestone May 20, 2025
natoscott added a commit to natoscott/htop that referenced this pull request May 21, 2025
No-op change in terms of generated code, but better to be consistent -
thanks to BenBE for noticing this.

Relates to htop-dev#1702
@GermanAizek
Copy link
Contributor Author

Fixed merge conflicts.

@BenBE
Copy link
Member

BenBE commented May 22, 2025

Please use rebase to update your PR. TIA.

Also note the comment regarding the ordering of struct TraceScreen_.

@fasterit
Copy link
Member

fasterit commented Jun 4, 2025

@GermanAizek can you please rebase and fix the comment as per above before we change more? Otherwise your PR will age (too) fast.
Thanks /DLange

references:
- https://hpc.rz.rptu.de/Tutorials/AVX/alignment.shtml
- https://wr.informatik.uni-hamburg.de/_media/teaching/wintersemester_2013_2014/epc-14-haase-svenhendrik-alignmentinc-presentation.pdf
- https://en.wikipedia.org/wiki/Data_structure_alignment
- https://stackoverflow.com/a/20882083
- https://zijishi.xyz/post/optimization-technique/learning-to-use-data-alignment/

affected structs:
- ScreenManager 72 to 64 bytes
- Screen/DynamicIterator 24 to 16 bytes
- Meter/DynamicIterator 24 to 16 bytes
- IncMode 152 to 144 bytes
- TraceScreen 64 to 56 bytes
- LinuxProcessTable 120 to 112 bytes
- FunctionBar 40 to 32 bytes

Co-authored-by: BenBE <[email protected]>
@fasterit
Copy link
Member

fasterit commented Jun 5, 2025

In file included from DisplayOptionsPanel.h:10,
from DisplayOptionsPanel.c:10:
FunctionBar.h:14:4: error: unknown type name ‘uint32_t’
14 | uint32_t size;
| ^~~~~~~~
In file included from InfoScreen.h:13,
from CommandScreen.h:11,
from CommandScreen.c:11:
FunctionBar.h:14:4: error: unknown type name ‘uint32_t’
14 | uint32_t size;
| ^~~~~~~~

--> missing #include <stdint.h>, fixing this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality ♻️ Code quality enhancement enhancement Extension or improvement to existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants