-
Notifications
You must be signed in to change notification settings - Fork 388
perf(storer): reuse buffers and eliminate per-chunk allocations in reserve sample #5612
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
base: master
Are you sure you want to change the base?
Changes from all commits
c1d1f5f
abc65ac
1682441
841bbaf
40f51d4
207b420
b9bf777
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,14 @@ type Getter interface { | |
| Get(context.Context, swarm.Address) (swarm.Chunk, error) | ||
| } | ||
|
|
||
| // GetterInto is like Getter but reads chunk data into a caller-provided buffer, | ||
| // avoiding per-call allocations. len(buf) must be at least the chunk size; | ||
| // GetInto never writes past len(buf). Returns the number of bytes read into | ||
| // buf; callers use buf[:n]. | ||
| type GetterInto interface { | ||
| GetInto(ctx context.Context, addr swarm.Address, buf []byte) (int, error) | ||
| } | ||
|
|
||
| // Putter is the interface that wraps the basic Put method. | ||
| type Putter interface { | ||
| // Put a chunk into the store alongside with its postage stamp. | ||
|
|
@@ -73,6 +81,7 @@ type ChunkGetterDeleter interface { | |
|
|
||
| type ChunkStore interface { | ||
| Getter | ||
| GetterInto | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if we need to add it to the ChunkStore interface. Since we only use ReadOnlyChunkStore for sampling, we can only add it to the interface below. I think we can even avoid adding it to transaction.ChunkStore also.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dropping it makes ReadOnlyChunkStore no longer a subset of ChunkStore, which breaks the test storages and the mock storer that return the same value as both. Then even more changes are needed...
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GetterInto was added to the required ChunkStore/ReadOnlyChunkStore interfaces, and |
||
| Putter | ||
| Deleter | ||
| Hasser | ||
|
|
@@ -84,5 +93,6 @@ type ChunkStore interface { | |
|
|
||
| type ReadOnlyChunkStore interface { | ||
| Getter | ||
| GetterInto | ||
| Hasser | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -225,6 +225,13 @@ func (c *chunkStoreTrx) Get(ctx context.Context, addr swarm.Address) (ch swarm.C | |
| return ch, err | ||
| } | ||
|
|
||
| func (c *chunkStoreTrx) GetInto(ctx context.Context, addr swarm.Address, buf []byte) (n int, err error) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check comment above. Since we only need ReadOnlyChunkStore to provide this function, we can avoid adding it here. |
||
| defer handleMetric("chunkstore_get", c.metrics)(&err) | ||
| unlock := c.lock(addr) | ||
| defer unlock() | ||
| return chunkstore.GetInto(ctx, c.indexStore, c.sharkyTrx, addr, buf) | ||
| } | ||
|
|
||
| func (c *chunkStoreTrx) Has(ctx context.Context, addr swarm.Address) (_ bool, err error) { | ||
| defer handleMetric("chunkstore_has", c.metrics)(&err) | ||
| unlock := c.lock(addr) | ||
|
|
||
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.
implementations return a bare fmt.Errorf("chunk store: buffer too small: %d < %d", ...). A caller can't distinguish it from ErrNotFound.
Maybe add: