Skip to content

reduced padding in some classes/structs#4295

Merged
danmar merged 2 commits intocppcheck-opensource:mainfrom
firewave:padding
Jul 24, 2022
Merged

reduced padding in some classes/structs#4295
danmar merged 2 commits intocppcheck-opensource:mainfrom
firewave:padding

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

Based on reviewing -Wpadded warnings in frequently used classes/structs. There's also some potential candidates in simplecpp.

If wanted to can try to provide sizeof() before and after.

I did a valgrind --tool=massif run on mame_regtest and it showed a small difference in the memory usage of Token.

Before:
2.3312988281 MiB: Token (lib/token.cpp:57)

After:
2.2017822266 MiB: Token (lib/token.cpp:57)

I did not profile this with callgrind (yet).

@danmar danmar merged commit b2f15fd into cppcheck-opensource:main Jul 24, 2022
@firewave firewave deleted the padding branch July 24, 2022 12:24
@firewave
Copy link
Copy Markdown
Collaborator Author

It also improves overall performance a bit. Scanning mame_regtest with DISABLE_VALUEFLOW=1 and --enable=all --inconclusive gives the following:

Clang 13 2,469,746,867 -> 2,458,134,208

But it caused a small visible regression in VariableMap::addVariable(std::__cxx11::basic_string<> const&, bool) with void std::vector<>::_M_realloc_insert<>(__gnu_cxx::__normal_iterator<>, std::pair<>&&) (cppcheck: vector.tcc, ...) from 688 to 1029 per call in average.

firewave added a commit to firewave/cppcheck that referenced this pull request Jul 25, 2022
@firewave
Copy link
Copy Markdown
Collaborator Author

At least the following boundary alignment padding was saved:
Function - 4 bytes
TokenImpl - 4 bytes
ValueFlow::Value - 0 bytes
LifetimeToken - 1 byte

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.

2 participants