Skip to content

Don't lock up 256KiB buffers when adding small files #4508

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

Merged
merged 4 commits into from
Dec 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/coreunix/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func (adder *Adder) SetMfsRoot(r *mfs.Root) {
}

// Constructs a node from reader's data, and adds it. Doesn't pin.
func (adder Adder) add(reader io.Reader) (node.Node, error) {
func (adder *Adder) add(reader io.Reader) (node.Node, error) {
chnk, err := chunk.FromString(reader, adder.Chunker)
if err != nil {
return nil, err
Expand Down
2 changes: 1 addition & 1 deletion importer/chunk/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
func FromString(r io.Reader, chunker string) (Splitter, error) {
switch {
case chunker == "" || chunker == "default":
return NewSizeSplitter(r, DefaultBlockSize), nil
return DefaultSplitter(r), nil

case strings.HasPrefix(chunker, "size-"):
sizeStr := strings.Split(chunker, "-")[1]
Expand Down
26 changes: 16 additions & 10 deletions importer/chunk/splitting.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"io"

logging "gx/ipfs/QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52/go-log"
mpool "gx/ipfs/QmWBug6eBS7AxRdCDVuSY5CnSit7cS2XnPFYJWqWDumhCG/go-msgio/mpool"
)

var log = logging.Logger("chunk")
Expand Down Expand Up @@ -51,32 +52,37 @@ func Chan(s Splitter) (<-chan []byte, <-chan error) {

type sizeSplitterv2 struct {
r io.Reader
size int64
size uint32
err error
}

func NewSizeSplitter(r io.Reader, size int64) Splitter {
return &sizeSplitterv2{
r: r,
size: size,
size: uint32(size),
}
}

func (ss *sizeSplitterv2) NextBytes() ([]byte, error) {
if ss.err != nil {
return nil, ss.err
}
buf := make([]byte, ss.size)
n, err := io.ReadFull(ss.r, buf)
if err == io.ErrUnexpectedEOF {

full := mpool.ByteSlicePool.Get(ss.size).([]byte)[:ss.size]
n, err := io.ReadFull(ss.r, full)
switch err {
case io.ErrUnexpectedEOF:
ss.err = io.EOF
err = nil
}
if err != nil {
small := make([]byte, n)
copy(small, full)
mpool.ByteSlicePool.Put(ss.size, full)
return small, nil
case nil:
return full, nil
default:
mpool.ByteSlicePool.Put(ss.size, full)
return nil, err
}

return buf[:n], nil
}

func (ss *sizeSplitterv2) Reader() io.Reader {
Expand Down
14 changes: 14 additions & 0 deletions importer/chunk/splitting_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,20 @@ func copyBuf(buf []byte) []byte {
return cpy
}

func TestSizeSplitterOverAllocate(t *testing.T) {
max := 1000
r := bytes.NewReader(randBuf(t, max))
chunksize := int64(1024 * 256)
splitter := NewSizeSplitter(r, chunksize)
chunk, err := splitter.NextBytes()
if err != nil {
t.Fatal(err)
}
if cap(chunk) > len(chunk) {
t.Fatal("chunk capacity too large")
}
}

func TestSizeSplitterIsDeterministic(t *testing.T) {
if testing.Short() {
t.SkipNow()
Expand Down
2 changes: 1 addition & 1 deletion importer/importer.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func BuildDagFromFile(fpath string, ds dag.DAGService) (node.Node, error) {
}
defer f.Close()

return BuildDagFromReader(ds, chunk.NewSizeSplitter(f, chunk.DefaultBlockSize))
return BuildDagFromReader(ds, chunk.DefaultSplitter(f))
}

func BuildDagFromReader(ds dag.DAGService, spl chunk.Splitter) (node.Node, error) {
Expand Down
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,12 @@
"hash": "QmYmhgAcvmDGXct1qBvc1kz9BxQSit1XBrTeiGZp2FvRyn",
"name": "go-libp2p-blankhost",
"version": "0.2.3"
},
{
"author": "jbenet",
"hash": "QmWBug6eBS7AxRdCDVuSY5CnSit7cS2XnPFYJWqWDumhCG",
"name": "go-msgio",
"version": "0.0.3"
}
],
"gxVersion": "0.10.0",
Expand Down