Skip to content

Conversation

lpereira
Copy link
Contributor

Regain space lost due to alignment by reordering structs from larger to smaller members and not intertwining bitfields with full-size fields.

With this change, the sizes are now:

Struct       Size before    Size after
assembler    104            96
basicblock_  64             56

Sizes measured on x86-64 Linux, using pahole and this handy script that compares struct sizes between versions.

I don't think this warrants an issue, as it just rearranges members inside the compile.c translation unit.

@ghost
Copy link

ghost commented Apr 11, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

Thanks! Found one minor whitespace issue, otherwise looks good:

Regain space lost due to alignment by reordering structs from larger to
smaller members and not intertwining bitfields with full-size fields.

With this change, the sizes are now:

    Struct       Size before    Size after
    assembler    104            96
    basicblock_  64             56

(Sizes measured on x86-64 Linux, using pahole[1] and this handy
script[2] that compares struct sizes between versions.)

No functional changes expected.  Structs are used only internally.

[1] https://github.com/acmel/dwarves
[2] https://gist.github.com/mdboom/6052da76e152ccc9dc6b9741a7ddd6c6
@lpereira lpereira force-pushed the remove-slops-compiler branch from 655570f to 51f3f3d Compare April 11, 2022 22:02
@brandtbucher
Copy link
Member

You can also add your name to Misc/ACKS too, if you want.

@brandtbucher brandtbucher merged commit 5f056ac into python:main Apr 13, 2022
@bedevere-bot
Copy link

@brandtbucher: Please replace # with GH- in the commit message next time. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants