docs: correct package usage and backend claims - #28
Conversation
|
|
||
| ### faster_keccak vs x/crypto/sha3 | ||
|
|
||
| | Size | faster_keccak | x/crypto | Speedup | |
|
|
||
| ```go | ||
| import "github.com/Giulio2002/faster_keccak" | ||
| import "github.com/erigontech/fastkeccak" |
There was a problem hiding this comment.
The snippet imports the package without an alias but then calls keccak.Sum256 / keccak.Hasher. That compiles (the package clause is package keccak), but the import path's last element is fastkeccak, so a reader who copies this block and types what the path suggests gets undefined: fastkeccak. The PR's own example_test.go aliases the import; the README should match it.
| import "github.com/erigontech/fastkeccak" | |
| import keccak "github.com/erigontech/fastkeccak" |
| Compare benchmarks with the same one-shot or streaming call shape. Supplying | ||
| `nil` to `hash.Hash.Sum` asks it to allocate an output slice and should not be | ||
| used to infer whether the hash implementation itself allocates. |
There was a problem hiding this comment.
The advice cannot be followed with the command above it. go test -bench . in this repo yields exactly the mismatched shapes this paragraph warns about: BenchmarkFasterKeccak is a one-shot Sum256(data), while BenchmarkXCrypto is Reset+Write+Sum(nil); on the streaming side BenchmarkFasterKeccakHasher ends with Sum256() (no alloc) but BenchmarkKeccakStreaming_Sha3 ends with Read(buf[:]) and only runs one size. So a reader who runs the suggested command still reads "x/crypto allocates 32 B/op" off Sum(nil) — the exact artifact this PR set out to stop presenting as library behavior.
Either adjust the benchmarks so a comparable pair exists (x/crypto one-shot, and a Sum(out[:0]) form), or name the pair that is comparable today.
Summary
Sum(nil)is not a fair allocation comparisonWhy
The current README imports a previous module path, does not compile when copied as one example, describes only part of the backend selection, and presents benchmark artifacts as library allocation behavior.
This is an independent alternative to #11. It deliberately avoids another static benchmark table because those numbers depend on the processor, OS, toolchain, and benchmark call shape.
This is a documentation correction, so TDD is not applicable. The new Go example compiles and checks both the one-shot and streaming output.
Verification
go test ./...go test -race ./...go test -tags purego ./...go vet ./...go mod tidy -diffgolangci-lint run(twice)