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

small optimizations for AqlItemBlock::toVelocyPack() #20739

Merged

Conversation

jsteemann
Copy link
Contributor

@jsteemann jsteemann commented Mar 13, 2024

Scope & Purpose

small optimizations for AqlItemBlock::toVelocyPack()

  • use FlatHashMap and not std::unordered_map to remember already seen objects. FlatHashMap should be slightly faster because the table is stored in contiguous memory
  • instead of using a sequence of table.find(value) and table.emplace(value, pos), we are now doing unconditional try_emplaces into the table. in case the target element was already in the table, this does nothing. in case the target element was not yet in the table, the call will create it, and we will have saved the additional call to find()
  • simplify the API signature of sanitizeNonClientTypes by removing the boolean parameters that were always set to true anyway on all call sites. Additionally, pass the velocypack::Options parameter by reference so that we can avoid nullptr-checks later.

these changes should not alter the observable behavior in any way.

observed performance improvements for toVelocyPack() are around ~1% with these changes for a test set of values.

  • 💩 Bugfix
  • 🍕 New feature
  • 🔥 Performance improvement
  • 🔨 Refactoring/simplification

Checklist

  • Tests
    • Regression tests
    • C++ Unit tests
    • integration tests
    • resilience tests
  • 📖 CHANGELOG entry made
  • 📚 documentation written (release notes, API changes, ...)
  • Backports
    • Backport for 3.12.0: -
    • Backport for 3.11: -
    • Backport for 3.10: -

Related Information

  • Docs PR:
  • Enterprise PR:
  • GitHub issue / Jira ticket:
  • Design document:

- use FlatHashMap and not std::unordered_map to remember already seen
  objects. FlatHashMap should be slightly faster because the table is
  stored in contiguous memory
- instead of using a sequence of table.find(value) and
  table.emplace(value, pos), we are now doing unconditional try_emplaces
  into the table. in case the target element was already in the table,
  this does nothing. in case the target element was not yet in the
  table, the call will create it, and we will have saved the additional
  call to find()
@jsteemann jsteemann added this to the devel milestone Mar 13, 2024
@jsteemann jsteemann requested a review from a team as a code owner March 13, 2024 18:48
@cla-bot cla-bot bot added the cla-signed label Mar 13, 2024
Copy link
Member

@neunhoef neunhoef left a comment

Choose a reason for hiding this comment

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

LGTM

@mchacki mchacki merged commit a87a852 into devel May 14, 2024
6 checks passed
@mchacki mchacki deleted the performance/small-optimization-for-aqlitemblock-tovelocypack branch May 14, 2024 15:14
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.

None yet

4 participants