Fast add stuff

[whyrusleeping]

This is based on the flushing disable PR i put up the other day since I needed some changes from there.

This makes ipfs add use the mfs code under the hood to make adds a bit faster.

[jbenet]

we should sort the files, otherwise non-determinism.

[whyrusleeping]

they are sorted on input, but yeah. probably makes sense to sort it in the tests

[jbenet]

If the outputs are based on concurrent behavior then I guess we have to give up sorting in the output and that's probably ok.

[whyrusleeping]

right, but theres no concurrency here at this point. Its pretty fast without it

[jbenet]

yeah let's sort on tests. we should probably warn in ipfs add --help that the output order is not deterministic.

[whyrusleeping]

@jbenet could use some CR here. I'll squash and rework the commits while addressing CR

[jbenet]

👏 thank you very much for the simplification

[jbenet]

why is this removed? not understanding why it no longer removes this root node

[jbenet]

@whyrusleeping some comments above -- otherwise this all LGTM. fantastic improvement!

[jbenet]

ok so i think directories should add every file in a new goroutine (with rate limiting). i had started on this earlier, but aborted. the general idea was every dir -> files fan out uses goroutines, like:

ctx, cancel := context.WithCancel(params.ctx)
defer cancel()
// this context will cancel out all the children if we error out early.
// would need to wire the addFile funcs with a ctx. OR we _could_ cancel 
// the entire context of thea adder instead...

errs := make(chan error, 10) // some room to avoid waiting for errors
                // dont think we have access to read len(dir) though

for {
  file, err := dir.NextFile()
  if err != nil && err != io.EOF {
    return err // defer cancel unblocks other children
  }
  if file == nil {
    break // done with files
  }

  <-params.concurrency // adder-global parameter
  go func(file) {
    defer func() {
      params.concurrency<- struct{}{} // done
    }()

    err := params.addFile(ctx, file) // ctx cancel should unblock addFile

    // return even nil errors, so we force the parent to wait
    // until _all_ children are done during non-error ops.
    // but bail on ctx.Done.
    select {
    case errs<- err:
    case ctx.Done():
    }
  }(file)
}

for err := range errs {
  if err == nil {
    continue
  }
  if _, ok := err.(*hiddenFileError); ok {
    continue // hidden file error, skip file
  }
  // error!
  return err // defer cancel unblocks + terminates all children.
}

// all children done.
return nil

[whyrusleeping]

this is hard to do right, this code will deadlock when it encounters a directory deeper than your concurrency factor.

[whyrusleeping]

we could add something like:

if file.IsDir() {
    params.addFile(ctx, file)
} else {
   // concurrency thing
}

[jbenet]

yeah i realized that after posting, we can fix that by bumping up the concurrency level one notch in dirs (i.e. semaup at the top of inside addDir, and defer semadown) -- better than this one o/ because this doesn't allow concurrent add of dirs-- and complicates the code by doing two different things (double the error handling, etc).

[jbenet]

@whyrusleeping general comment: i think adding files should be concurrent because a single add should leverage as much concurrent io + hashing as possible (right now it sequentially trades off between io and hashing).

I think adding https://github.com/ipfs/go-ipfs/pull/2039/files#r46767388 will fix it

[whyrusleeping]

@jbenet would you be opposed to having the concurrency stuff done in a separate PR? This one is already getting pretty large

[jbenet]

@whyrusleeping my thought is "it's almost there!" -- but sure, if it's easier for you

[jbenet]

still needs sorting?

[jbenet]

or, i guess it doesn't matter what order they're in? maybe just restore the original order to make sure/

[whyrusleeping]

good to merge?

[jbenet]

@whyrusleeping Still missing:

• Fast add stuff #2039 (comment) - https://github.com/ipfs/go-ipfs/pull/2039/files#r46766019
• Fast add stuff #2039 (comment) - https://github.com/ipfs/go-ipfs/pull/2039/files#r46766078
• Fast add stuff #2039 (comment) - https://github.com/ipfs/go-ipfs/pull/2039/files#r46766194

[rht]

It sure is fast

(filesize 1KB each)

Memory growth:

However, it is still possible to get to ~O(git) soon if intermediate root nodes are not created (even if the hashings are delayed in this PR) #1964 (comment).

[jbenet]

great stats, thanks @rht.

would be awesome to get these auto-generated by running one script. could
add it under tests/bench/ or something

On Tue, Dec 15, 2015 at 10:28 PM rht [email protected] wrote:

It sure is fast
[image: outdata]
https://cloud.githubusercontent.com/assets/395821/11830563/f2d7ad10-a3d8-11e5-868b-f43bce3e4441.png
(filesize 1KB each)

Memory growth:
[image: memory]
https://cloud.githubusercontent.com/assets/395821/11830592/3a881550-a3d9-11e5-877d-a85d6b11f485.png

However, it is still possible to get to ~O(git) soon if intermediate root
nodes are not created (even if the hashings are delayed in this PR) #1964
(comment)
#1964 (comment).

—
Reply to this email directly or view it on GitHub
#2039 (comment).