Skip to content

add xxHash64 wrapper and convert most hashing#1627

Merged
danielhochman merged 2 commits intomasterfrom
hashlib
Sep 12, 2017
Merged

add xxHash64 wrapper and convert most hashing#1627
danielhochman merged 2 commits intomasterfrom
hashlib

Conversation

@danielhochman
Copy link
Copy Markdown
Contributor

@danielhochman danielhochman commented Sep 12, 2017

followup to #1624

left hashing in redis path alone for now. will figure out an internal strategy for migrating, possibly compiling in what std::hash is using so we don't have to worry about rehashing all of our cache data on deploy.

fixes #1452

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

looks good, small nits

class HashUtil {
public:
/**
* Return 64-bit hash with seed of 0 from the xxHash algorithm.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: Can you link to repo/docs so people can read about it if they want.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

struct LbContextImpl : public Upstream::LoadBalancerContext {
LbContextImpl(const std::string& hash_key) : hash_key_(std::hash<std::string>()(hash_key)) {}

// TODO(danielhochman): convert to HashUtil::xxHash64 when we have a migration strategy
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: period end of sentence

for (const auto& host : hosts) {
for (uint64_t i = 0; i < hashes_per_host; i++) {
std::string hash_key(host->address()->asString() + "_" + std::to_string(i));
// TODO(danielhochman): convert to HashUtil::xxHash64 when we have a migration strategy
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: period end of sentence

Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Great, please close out #1452 when this merges!

@danielhochman danielhochman merged commit 8a09d86 into master Sep 12, 2017
@danielhochman danielhochman deleted the hashlib branch September 12, 2017 21:11
mathetake added a commit that referenced this pull request Mar 3, 2026
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.

Add stronger hash or comparison to xDS APIs

3 participants