From 115f1c2cc9f95ccf0d0b28a058f71f62424f2d0f Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Tue, 9 Nov 2021 09:45:36 +0000 Subject: [PATCH] operations: speed up hash checking by aborting the other hash if first returns nothing This speeds up hash checks when a Hash() function returns "" - this means that the hash can be canceled for the other side. In the common case of local hash vs remote hash empty this saves a lot of time. See: https://forum.rclone.org/t/rclone-s3-backend-copy-is-2x-slower-than-aws-s3-cp/27321/9 --- fs/operations/operations.go | 46 ++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/fs/operations/operations.go b/fs/operations/operations.go index c34cdf97d..c21675443 100644 --- a/fs/operations/operations.go +++ b/fs/operations/operations.go @@ -62,37 +62,51 @@ func CheckHashes(ctx context.Context, src fs.ObjectInfo, dst fs.Object) (equal b return equal, ht, err } +var errNoHash = errors.New("no hash available") + // checkHashes does the work of CheckHashes but takes a hash.Type and // returns the effective hash type used. func checkHashes(ctx context.Context, src fs.ObjectInfo, dst fs.Object, ht hash.Type) (equal bool, htOut hash.Type, srcHash, dstHash string, err error) { // Calculate hashes in parallel g, ctx := errgroup.WithContext(ctx) + var srcErr, dstErr error g.Go(func() (err error) { - srcHash, err = src.Hash(ctx, ht) - if err != nil { - err = fs.CountError(err) - fs.Errorf(src, "Failed to calculate src hash: %v", err) + srcHash, srcErr = src.Hash(ctx, ht) + if srcErr != nil { + return srcErr } - return err + if srcHash == "" { + fs.Debugf(src, "Src hash empty - aborting Dst hash check") + return errNoHash + } + return nil }) g.Go(func() (err error) { - dstHash, err = dst.Hash(ctx, ht) - if err != nil { - err = fs.CountError(err) - fs.Errorf(dst, "Failed to calculate dst hash: %v", err) + dstHash, dstErr = dst.Hash(ctx, ht) + if dstErr != nil { + return dstErr } - return err + if dstHash == "" { + fs.Debugf(src, "Dst hash empty - aborting Src hash check") + return errNoHash + } + return nil }) err = g.Wait() + if err == errNoHash { + return true, hash.None, srcHash, dstHash, nil + } + if srcErr != nil { + err = fs.CountError(srcErr) + fs.Errorf(dst, "Failed to calculate src hash: %v", err) + } + if dstErr != nil { + err = fs.CountError(dstErr) + fs.Errorf(src, "Failed to calculate dst hash: %v", err) + } if err != nil { return false, ht, srcHash, dstHash, err } - if srcHash == "" { - return true, hash.None, srcHash, dstHash, nil - } - if dstHash == "" { - return true, hash.None, srcHash, dstHash, nil - } if srcHash != dstHash { fs.Debugf(src, "%v = %s (%v)", ht, srcHash, src.Fs()) fs.Debugf(dst, "%v = %s (%v)", ht, dstHash, dst.Fs())