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

Rearrange struct members to remove padding bytes #2959

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rootkea
Copy link
Contributor

@rootkea rootkea commented Oct 27, 2021

Hello!

On master, each struct has 8 padding bytes.

Found by: scan-build

Thanks!

@elextr
Copy link
Member

elextr commented Oct 27, 2021

There are 1028 lines removed and replaced, at 4 bytes per line thats 4k bytes of program memory which is not material.

The structures are only used for initialisation and configuration, so its not on any fast path, so cache effects are also not material.

It would also take a significant effort to check and verify every language type still works which is not available.

Sorry to sound harsh, but it seems like a lot of noise and effort for no significant gain.

@rootkea rootkea changed the title Rearrange struct memebers to remove padding bytes Rearrange struct members to remove padding bytes Oct 27, 2021
@rootkea
Copy link
Contributor Author

rootkea commented Oct 27, 2021

thats 4k bytes of program memory which is not material.

4k bytes of memory saved without adding any additional code.

It would also take a significant effort to check and verify

This PR could be reviewed over 2-3 days per 10-min a day as it involves only visual inspection.

Anyways, I leave it upto you maintainers wether to accept or reject the suggested change. :)

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