slightly reorder boolean members to reduce struct sizes #9762
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dothebart
reviewed
Aug 20, 2019
| size_t _numScanned; | ||
|
|
||
| /// @brief set of already returned documents. Used to make the result distinct | ||
| std::unordered_set<TRI_voc_rid_t> _alreadyReturned; |
There was a problem hiding this comment.
shouldn't this be
std::unord...
size_t ...
bool ...
here?
There was a problem hiding this comment.
why?
the crucial thing is that the boolean values are placed adjacently, as they can be 1-byte aligned.
all other members have an alignment of 8 bytes.
There was a problem hiding this comment.
ok, in this case its 5x bool, so the slack-space will be used anyways, however, I'd prefer to have them at the end, so its a non-brainer to add more there.
There was a problem hiding this comment.
You can have 5x anywhere, and you can add more.
The effect will be exactly the same.
…rder-aql-executor-members
mpoeter
reviewed
Aug 20, 2019
| bool getProduceResult() const { return _produceResult; } | ||
| bool getUseRawDocumentPointers() const { return _useRawDocumentPointers; } | ||
| bool getRandom() const { return _random; } | ||
| RegisterId getOutputRegisterId() const { return _outputRegisterId; } | ||
|
|
||
| private: | ||
| RegisterId _outputRegisterId; |
There was a problem hiding this comment.
RegisterId is an unsigned int (i.e., usually 32bit), so it should be moved before the bools.
| @@ -177,23 +178,22 @@ struct DocumentProducingFunctionContext { | |||
| InputAqlItemRow const& _inputRow; | |||
| OutputAqlItemRow* _outputRow; | |||
| RegisterId const _outputRegister; | |||
There was a problem hiding this comment.
RegisterId is an unsigned int (i.e., usually 32bit), so it should be moved before the bools.
| std::vector<std::unique_ptr<NonConstExpression>> _nonConstExpression; | ||
| /// @brief true if one of the indexes uses more than one expanded attribute, |
There was a problem hiding this comment.
Line 132 contains another RegisterId that should be moved here.
dothebart
approved these changes
Aug 20, 2019
…rder-aql-executor-members
<
8000
form class="js-comment-update" id="issuecomment-523017353-edit-form" data-turbo="false" action="/arangodb/arangodb/issue_comments/523017353" accept-charset="UTF-8" method="post">
Sorry, something went wrong.
ObiWahn
added a commit
that referenced
this pull request
Aug 22, 2019
…ture/mimalloc * 'devel' of https://github.com/arangodb/arangodb: (83 commits) Bug fix/internal issue #622 (#9781) mark AQL functions FULLTEXT, NEAR, WITHIN, WITHIN_RECTANGLE as cacheable (#9771) reduce wait timeout, use move Bug fix/implement windows maintenance tests (#9763) show query string length and cacheability in explain output (#9767) fix potential spurious wakeups in scheduler code (#9770) Bug fix/issue #9612 (#9764) downgrade WARN messages to INFO level (#9761) slightly reorder boolean members to reduce struct sizes (#9762) make index selection more deterministic (#9735) Update PULL_REQUEST_TEMPLATE.md (#9758) [devel] Move Shard Bug 4567124 (#9746) Bug fix/fix signed int overflow (#9717) Bug fix/fix invalid cast (#9755) Enforce stricter transaction limits (#9740) Check scheduler queue return value (#9754) dont fill cache on truncate (#9721) fix pasting from the documentation (#9742) tell that procdump is gone - it seems this happenes in reality without coredumps being written (#9748) issue #9654: make `--rocksdb.max-write-buffer-number` work (#9750) ...
ObiWahn
added a commit
that referenced
this pull request
Aug 23, 2019
…ture/one-shard-db * 'devel' of https://github.com/arangodb/arangodb: (53 commits) remove 404-ed callbacks from agency (#9709) AQL date functions improvements (#9714) Bug fix/internal issue #622 (#9781) mark AQL functions FULLTEXT, NEAR, WITHIN, WITHIN_RECTANGLE as cacheable (#9771) reduce wait timeout, use move Bug fix/implement windows maintenance tests (#9763) show query string length and cacheability in explain output (#9767) fix potential spurious wakeups in scheduler code (#9770) Bug fix/issue #9612 (#9764) downgrade WARN messages to INFO level (#9761) slightly reorder boolean members to reduce struct sizes (#9762) make index selection more deterministic (#9735) Update PULL_REQUEST_TEMPLATE.md (#9758) [devel] Move Shard Bug 4567124 (#9746) Bug fix/fix signed int overflow (#9717) Bug fix/fix invalid cast (#9755) Enforce stricter transaction limits (#9740) Check scheduler queue return value (#9754) dont fill cache on truncate (#9721) fix pasting from the documentation (#9742) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Scope & Purpose
Slightly reorder instance members so boolean values are placed adjacently and make structs smaller.
Also make several functions const or noexcept.
Testing & Verification
This change is a trivial rework / code cleanup without any test coverage.
However, if something is terribly wrong, the AQL tests should reveal it.
https://jenkins01.arangodb.biz/view/PR/job/arangodb-matrix-pr/5786/