-
Notifications
You must be signed in to change notification settings - Fork 37
Conversation
It has the same properites as Rabin. Benchmark results: ``` name time/op Buzhash-4 14.3ms ± 7% Rabin-4 94.1ms ± 3% Default-4 1.74ms ± 7% name speed Buzhash-4 1.18GB/s ± 7% Rabin-4 178MB/s ± 3% Default-4 9.63GB/s ± 6% name alloc/op Buzhash-4 14.0kB ±48% Rabin-4 19.2MB ± 0% Default-4 474B ± 6% name allocs/op Buzhash-4 1.00 ± 0% Rabin-4 196 ±12% Default-4 2.00 ± 0% ``` License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does look a lot simpler. My main concern is configurability. We don't need arbitrary block sizes but having options is still useful.
|
||
const ( | ||
buzMin = 128 << 10 | ||
buzMax = 512 << 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we configure these? Can we configure the expected/average chunk size?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will check what is the perf penalty for making these configurable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6%:
name old time/op new time/op delta
Buzhash-4 14.2ms ± 7% 15.1ms ± 6% +6.36% (p=0.000 n=20+19)
name old speed new speed delta
Buzhash-4 1.18GB/s ± 6% 1.11GB/s ±14% -6.64% (p=0.000 n=20+20)
Your call
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's annoying. I'm fine leaving that off for now.
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai>
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai>
By eliminating Reader interface and implementing buzhash chunking on a
|
Don't return them either in benchmarks License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai>
buzhash.go
Outdated
return nil, b.err | ||
} | ||
|
||
buf := b.buf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: buf
is always b.buf
. IMO, we should just use b.buf
direct.y.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good nit, it didn't use to be like that.
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai>
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai> License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai>
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai>
License: MIT Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai>
Implement buzhash This commit was moved from ipfs/go-ipfs-chunker@21b0c06
It has the same properites as Rabin.
Benchmark results:
License: MIT