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

Allowing seeding the builder to get reproducible data #20

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

Jille
Copy link
Contributor

@Jille Jille commented Jul 8, 2024

I'm using this to get the byte-for-byte same result if the input is the same, so my rsync-like tool doesn't need to retransmit the data.

also switch to rand.Uint64() which is probably a little cheaper.

I'm using this to get the byte-for-byte same result if the input is the
same, so my rsync-like tool doesn't need to retransmit the data.
@Jille
Copy link
Contributor Author

Jille commented Sep 4, 2024

Friendly ping @alecthomas

@@ -41,13 +41,21 @@ func (b bucketVector) Swap(i, j int) { b[i], b[j] = b[j], b[i] }
type CHDBuilder struct {
keys [][]byte
values [][]byte
seed int64
seeded bool
}

// Create a new CHD hash table builder.
func Builder() *CHDBuilder {
return &CHDBuilder{}
Copy link
Owner

Choose a reason for hiding this comment

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

I think this needs to be updated to set seed to a random value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given that the constructor just allocates an empty struct, I wasn't sure if everyone used the constructor or whether some users just allocate themselves.

So I went with an extra boolean and did the seeding when needed rather than up front.

Copy link
Owner

Choose a reason for hiding this comment

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

That's a better idea than my one, thanks!

@alecthomas
Copy link
Owner

Apologies for the delay!

@alecthomas alecthomas merged commit 2a0c46a into alecthomas:master Sep 4, 2024
1 check passed
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.

2 participants