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

Dtrie #131

Merged
merged 9 commits into from Mar 28, 2016
Merged

Dtrie #131

merged 9 commits into from Mar 28, 2016

Conversation

Theodus
Copy link
Contributor

@Theodus Theodus commented Mar 28, 2016

This is an implementation of a persistent hash array mapped trie that expands or shrinks to provide efficient memory allocation. Details on the papers that this project is based on can be found at github.com/theodus/dtrie.

Benchmarks:
BenchmarkInsert-4 5000000 547 ns/op 164 B/op 2 allocs/op
BenchmarkGet-4 10000000 250 ns/op 8 B/op 1 allocs/op
BenchmarkRemove-4 10000000 253 ns/op 8 B/op 1 allocs/op
BenchmarkUpdate-4 5000000 383 ns/op 55 B/op 3 allocs/op

@alexandercampbell-wk
Copy link
Contributor

@Theodus looks like there's an issue in one of the tests:

=== RUN TestIterate-2
--- FAIL: TestIterate-2 (0.36s)
    Error Trace:    dtrie_test.go:158
    Error:      Should be true

@Theodus
Copy link
Contributor Author

Theodus commented Mar 28, 2016

Sorry about that. I hadn't tested the original for Go 1.3 compatibility.


func popCount(bitmap uint32) int {
// bit population count, see
// http://graphics.stanford.edu/~seander/bithacks.html#CountBitsSetParallel
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for including this link!

@dustinhiatt-wf
Copy link
Contributor

+1

}

func defaultHasher(value interface{}) uint32 {
switch value.(type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to simplify this switch with

switch v := value.(type) {
case uint8:
        return uint32(v)
case uint16:
        return uint32(v)
...
}

@alexandercampbell-wk
Copy link
Contributor

One minor stylistic comment. Otherwise, this looks good to me. Thanks for your PR!

@tylertreat-wf
Copy link
Contributor

lgtm

@alexandercampbell-wk
Copy link
Contributor

+1

@dustinhiatt-wf
Copy link
Contributor

+!

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.

None yet

4 participants