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

swarm/storage: Pyramid chunker re-write #14382

Merged
merged 1 commit into from Sep 21, 2017

Conversation

Projects
None yet
4 participants
@jmozah
Contributor

jmozah commented Apr 26, 2017

Complete re-write of pyramid chunker.

  • It doesn't take size now.
  • This is the base for implementing "append"
    It can now be used to chunk streams with unknown size.
@jmozah

This comment has been minimized.

Show comment
Hide comment
@jmozah

jmozah Apr 27, 2017

Contributor

Benchmark result in my machine

$go test -cpu 4 -bench=. -run no

BenchmarkSplitTree_2-4 50000 29320 ns/op 3795 B/op 27 allocs/op
BenchmarkSplitTree_2h-4 30000 60731 ns/op 4253 B/op 27 allocs/op
BenchmarkSplitTree_3-4 20000 95937 ns/op 4773 B/op 27 allocs/op
BenchmarkSplitTree_3h-4 3000 398683 ns/op 10928 B/op 40 allocs/op
BenchmarkSplitTree_4-4 2000 757910 ns/op 17536 B/op 46 allocs/op
BenchmarkSplitTree_4h-4 500 3646963 ns/op 72240 B/op 106 allocs/op
BenchmarkSplitTree_5-4 200 7207745 ns/op 140208 B/op 178 allocs/op
BenchmarkSplitTree_6-4 20 71692609 ns/op 1370048 B/op 1512 allocs/op
BenchmarkSplitTree_7-4 2 721174723 ns/op 13666048 B/op 14820 allocs/op
BenchmarkSplitTree_8-4 1 7237853346 ns/op 136621344 B/op 147871 allocs/op

BenchmarkSplitPyramid_2-4 100000 36174 ns/op 16221 B/op 30 allocs/op
BenchmarkSplitPyramid_2h-4 30000 71389 ns/op 16048 B/op 30 allocs/op
BenchmarkSplitPyramid_3-4 20000 82921 ns/op 16494 B/op 30 allocs/op
BenchmarkSplitPyramid_3h-4 5000 316495 ns/op 21684 B/op 37 allocs/op
BenchmarkSplitPyramid_4-4 3000 463458 ns/op 24468 B/op 40 allocs/op
BenchmarkSplitPyramid_4h-4 1000 1939678 ns/op 52308 B/op 70 allocs/op
BenchmarkSplitPyramid_5-4 500 3705756 ns/op 85716 B/op 106 allocs/op
BenchmarkSplitPyramid_6-4 100 36133736 ns/op 701512 B/op 775 allocs/op
BenchmarkSplitPyramid_7-4 5 431053261 ns/op 8240521 B/op 8922 allocs/op
BenchmarkSplitPyramid_8-4 100 4269283181 ns/op 68536216 B/op 74062 allocs/op
PASS
ok github.com/ethereum/go-ethereum/swarm/storage 494.413s

Contributor

jmozah commented Apr 27, 2017

Benchmark result in my machine

$go test -cpu 4 -bench=. -run no

BenchmarkSplitTree_2-4 50000 29320 ns/op 3795 B/op 27 allocs/op
BenchmarkSplitTree_2h-4 30000 60731 ns/op 4253 B/op 27 allocs/op
BenchmarkSplitTree_3-4 20000 95937 ns/op 4773 B/op 27 allocs/op
BenchmarkSplitTree_3h-4 3000 398683 ns/op 10928 B/op 40 allocs/op
BenchmarkSplitTree_4-4 2000 757910 ns/op 17536 B/op 46 allocs/op
BenchmarkSplitTree_4h-4 500 3646963 ns/op 72240 B/op 106 allocs/op
BenchmarkSplitTree_5-4 200 7207745 ns/op 140208 B/op 178 allocs/op
BenchmarkSplitTree_6-4 20 71692609 ns/op 1370048 B/op 1512 allocs/op
BenchmarkSplitTree_7-4 2 721174723 ns/op 13666048 B/op 14820 allocs/op
BenchmarkSplitTree_8-4 1 7237853346 ns/op 136621344 B/op 147871 allocs/op

BenchmarkSplitPyramid_2-4 100000 36174 ns/op 16221 B/op 30 allocs/op
BenchmarkSplitPyramid_2h-4 30000 71389 ns/op 16048 B/op 30 allocs/op
BenchmarkSplitPyramid_3-4 20000 82921 ns/op 16494 B/op 30 allocs/op
BenchmarkSplitPyramid_3h-4 5000 316495 ns/op 21684 B/op 37 allocs/op
BenchmarkSplitPyramid_4-4 3000 463458 ns/op 24468 B/op 40 allocs/op
BenchmarkSplitPyramid_4h-4 1000 1939678 ns/op 52308 B/op 70 allocs/op
BenchmarkSplitPyramid_5-4 500 3705756 ns/op 85716 B/op 106 allocs/op
BenchmarkSplitPyramid_6-4 100 36133736 ns/op 701512 B/op 775 allocs/op
BenchmarkSplitPyramid_7-4 5 431053261 ns/op 8240521 B/op 8922 allocs/op
BenchmarkSplitPyramid_8-4 100 4269283181 ns/op 68536216 B/op 74062 allocs/op
PASS
ok github.com/ethereum/go-ethereum/swarm/storage 494.413s

@zelig zelig referenced this pull request Apr 29, 2017

Closed

Pyramid chunker 1st iteration #67

@jmozah

This comment has been minimized.

Show comment
Hide comment
@jmozah

jmozah May 3, 2017

Contributor

BenchmarkAppendPyramid_2-4 10000 176024 ns/op 54344 B/op 83 allocs/op
BenchmarkAppendPyramid_2h-4 10000 166854 ns/op 54342 B/op 82 allocs/op
BenchmarkAppendPyramid_3-4 10000 193190 ns/op 55459 B/op 83 allocs/op
BenchmarkAppendPyramid_4-4 5000 372089 ns/op 59208 B/op 84 allocs/op
BenchmarkAppendPyramid_4h-4 5000 382823 ns/op 59207 B/op 84 allocs/op
BenchmarkAppendPyramid_5-4 5000 377257 ns/op 59210 B/op 84 allocs/op
BenchmarkAppendPyramid_6-4 5000 376271 ns/op 59206 B/op 84 allocs/op
BenchmarkAppendPyramid_7-4 5000 405490 ns/op 59208 B/op 84 allocs/op
BenchmarkAppendPyramid_8-4 5000 377806 ns/op 59208 B/op 84 allocs/op

Contributor

jmozah commented May 3, 2017

BenchmarkAppendPyramid_2-4 10000 176024 ns/op 54344 B/op 83 allocs/op
BenchmarkAppendPyramid_2h-4 10000 166854 ns/op 54342 B/op 82 allocs/op
BenchmarkAppendPyramid_3-4 10000 193190 ns/op 55459 B/op 83 allocs/op
BenchmarkAppendPyramid_4-4 5000 372089 ns/op 59208 B/op 84 allocs/op
BenchmarkAppendPyramid_4h-4 5000 382823 ns/op 59207 B/op 84 allocs/op
BenchmarkAppendPyramid_5-4 5000 377257 ns/op 59210 B/op 84 allocs/op
BenchmarkAppendPyramid_6-4 5000 376271 ns/op 59206 B/op 84 allocs/op
BenchmarkAppendPyramid_7-4 5000 405490 ns/op 59208 B/op 84 allocs/op
BenchmarkAppendPyramid_8-4 5000 377806 ns/op 59208 B/op 84 allocs/op

@jmozah

This comment has been minimized.

Show comment
Hide comment
@jmozah

jmozah May 8, 2017

Contributor

@zelig Added pyramid chunker with BMT.. by mind voice says i have not used your interface for BMT properly.. Also the results i shared with you are too good to believe... please check

Contributor

jmozah commented May 8, 2017

@zelig Added pyramid chunker with BMT.. by mind voice says i have not used your interface for BMT properly.. Also the results i shared with you are too good to believe... please check

Show outdated Hide outdated swarm/storage/chunker.go Outdated
Show outdated Hide outdated swarm/storage/pyramid.go Outdated
Show outdated Hide outdated swarm/storage/pyramid.go Outdated
Show outdated Hide outdated swarm/storage/pyramid.go Outdated
Show outdated Hide outdated swarm/storage/pyramid.go Outdated
@jmozah

This comment has been minimized.

Show comment
Hide comment
@jmozah

jmozah May 13, 2017

Contributor
  • Fixed a bug when the chunk ends in multiple of 4096
  • Added test cases for TreeChunker Vs BMTChunker correctness test
  • Fixed some review changes specified above
Contributor

jmozah commented May 13, 2017

  • Fixed a bug when the chunk ends in multiple of 4096
  • Added test cases for TreeChunker Vs BMTChunker correctness test
  • Fixed some review changes specified above
Show outdated Hide outdated bmt/bmt.go Outdated
Show outdated Hide outdated bmt/bmt.go Outdated
Show outdated Hide outdated bmt/bmt.go Outdated
Show outdated Hide outdated bmt/bmt.go Outdated

@zelig zelig referenced this pull request Jun 26, 2017

Closed

swarm related PRs - Q2 merge plan #14706

9 of 13 tasks complete

@ethereum ethereum deleted a comment from GitCop Sep 5, 2017

@ethereum ethereum deleted a comment from GitCop Sep 8, 2017

@zelig

zelig approved these changes Sep 8, 2017

@lmars

I have reviewed the Go code from a high level, I think a lot of the code should be documented as it is difficult to understand what a lot of the methods are actually doing.

Show outdated Hide outdated swarm/fuse/fuse_file.go Outdated
Show outdated Hide outdated swarm/storage/chunker.go Outdated
Show outdated Hide outdated swarm/storage/chunker.go Outdated
Show outdated Hide outdated swarm/storage/chunker.go Outdated
Show outdated Hide outdated swarm/storage/chunker_test.go Outdated
if err != nil {
return nil, err
}
case <-time.NewTimer(splitTimeout).C:

This comment has been minimized.

@lmars

lmars Sep 8, 2017

Contributor

Same as the tree chunker, should an error be returned on timeout?

@lmars

lmars Sep 8, 2017

Contributor

Same as the tree chunker, should an error be returned on timeout?

This comment has been minimized.

@jmozah

jmozah Sep 12, 2017

Contributor

same answer as the tree chunker.

@jmozah

jmozah Sep 12, 2017

Contributor

same answer as the tree chunker.

// send off new chunk to storage
if chunkC != nil {
if swg != nil {
swg.Add(1)

This comment has been minimized.

@lmars

lmars Sep 8, 2017

Contributor

If we are sending chunks to a channel here, why do we also have to increment a "storage" wait group? Shouldn't it be the receiver of the channel that is actually going to store the chunk who should also manage this wait group?

@lmars

lmars Sep 8, 2017

Contributor

If we are sending chunks to a channel here, why do we also have to increment a "storage" wait group? Shouldn't it be the receiver of the channel that is actually going to store the chunk who should also manage this wait group?

This comment has been minimized.

@jmozah

jmozah Sep 12, 2017

Contributor
  • The receiver function "storeWorker" in dpa uses only the channel to store the chunk.
  • The swg in the above snippet is the chunk's own wg, which signals that the chunk is stored. This is used in testing to simulate a put.

The actual use of this is while storing in memstore, it is incremented and after it is stored in DB, it is decremented indicating that the operation is done. per chunk vise.

@jmozah

jmozah Sep 12, 2017

Contributor
  • The receiver function "storeWorker" in dpa uses only the channel to store the chunk.
  • The swg in the above snippet is the chunk's own wg, which signals that the chunk is stored. This is used in testing to simulate a put.

The actual use of this is while storing in memstore, it is incremented and after it is stored in DB, it is decremented indicating that the operation is done. per chunk vise.

func (self *PyramidChunker) prepareChunks(isAppend bool, chunkLevel [][]*TreeEntry, data io.Reader, rootKey []byte, quitC chan bool, wg *sync.WaitGroup, jobC chan *chunkJob, processorWG *sync.WaitGroup, chunkC chan *Chunk, errC chan error, storageWG *sync.WaitGroup) {
defer wg.Done()
chunkWG := &sync.WaitGroup{}

This comment has been minimized.

@lmars

lmars Sep 8, 2017

Contributor

What is this wait group used for?

@lmars

lmars Sep 8, 2017

Contributor

What is this wait group used for?

This comment has been minimized.

@jmozah

jmozah Sep 12, 2017

Contributor

This is where the parent node waits for all the children nodes (128 branches) to complete OR for the input to get exhausted. Only when all the 128 hashes of the child nodes are processed.. the parent chunk can be constructed.

@jmozah

jmozah Sep 12, 2017

Contributor

This is where the parent node waits for all the children nodes (128 branches) to complete OR for the input to get exhausted. Only when all the 128 hashes of the child nodes are processed.. the parent chunk can be constructed.

Show outdated Hide outdated swarm/storage/pyramid.go Outdated
Show outdated Hide outdated swarm/storage/pyramid.go Outdated

@zelig zelig added merge imminent and removed in progress labels Sep 21, 2017

@fjl fjl merged commit d558a59 into ethereum:master Sep 21, 2017

1 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
commit-message-check/gitcop All commit messages are valid
Details

vincentserpoul added a commit to vincentserpoul/go-ethereum that referenced this pull request Oct 4, 2017

vincentserpoul added a commit to vincentserpoul/go-ethereum that referenced this pull request Oct 28, 2017

vincentserpoul added a commit to vincentserpoul/go-ethereum that referenced this pull request Nov 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment