8000 slightly reorder boolean members to reduce struct sizes by jsteemann · Pull Request #9762 · arangodb/arangodb · GitHub
[go: up one dir, main page]

Skip to content

slightly reorder boolean members to reduce struct sizes #9762

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 5 commits into from
Aug 20, 2019

Conversation

jsteemann
Copy link
Contributor

Scope & Purpose

Slightly reorder instance members so boolean values are placed adjacently and make structs smaller.
Also make several functions const or noexcept.

  • Bug-Fix for devel-branch (i.e. no need for backports?)
  • The behavior change can be verified via automatic tests

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/

@jsteemann jsteemann added this to the devel milestone Aug 20, 2019
@jsteemann jsteemann requested a review from dothebart August 20, 2019 08:53
size_t _numScanned;

/// @brief set of already returned documents. Used to make the result distinct
std::unordered_set<TRI_voc_rid_t> _alreadyReturned;
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this be
std::unord...
size_t ...
bool ...
here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can have 5x anywhere, and you can add more.
The effect will be exactly the same.

@jsteemann
Copy link
Contributor Author

bool getProduceResult() const { return _produceResult; }
bool getUseRawDocumentPointers() const { return _useRawDocumentPointers; }
bool getRandom() const { return _random; }
RegisterId getOutputRegisterId() const { return _outputRegisterId; }

private:
RegisterId _outputRegisterId;
Copy link
Contributor

Choose a reason for hiding this comment

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

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

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,
Copy link
Contributor

Choose a reason for hiding this comment

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

Line 132 contains another RegisterId that should be moved here.

@jsteemann
Copy link
Contributor Author
< 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">

@jsteemann jsteemann merged commit 6ffb606 into devel Aug 20, 2019
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)
  ...
@fceller fceller deleted the bug-fix/reorder-aql-executor-members branch September 6, 2019 11:41
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.

3 participants
0