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

Updating std::hast<EntityId_t> to get a better unordered containers distribution [6895] #872

Merged
merged 2 commits into from
Nov 25, 2019

Conversation

MiguelBarro
Copy link
Contributor

Internally STL implementation of hash tables (unordered_map and set) use modulus operation to choose in which bucket put each element. Using an uniform distributed hash guarantees that the buckets get evenly filled and access time is minimum.
Until now we were using the whole EntityId_t interpret as a number as hash. That led to a non-uniform, unsuitable distribution because within each EntityId_t are kept flags not merely a counter. Now, only the counter value is used solving the speed issue.
This pull request contents has already been updated on 1.9.x (see #868).

@richiware
Copy link
Member

Build status:

  • Linux Build Status
  • Mac Build Status
  • Windows Build Status
  • Performance Build Status

@MiguelBarro MiguelBarro changed the title Updating std::hast<EntityId_t> to get a better unordered containers distribution Updating std::hast<EntityId_t> to get a better unordered containers distribution [6895] Nov 20, 2019
Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

LGTM.

@MiguelCompany
Copy link
Member

Failed test is known to be flaky.

@MiguelCompany MiguelCompany merged commit 08908a5 into master Nov 25, 2019
@MiguelCompany MiguelCompany deleted the hotfix/master/hash_EntityId_t branch November 25, 2019 13:27
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