Re: [PATCH] Btrfs : send, truncate first to enhance many small files

[Filipe Manana <fdmanana@gmail.com>]

On Mon, Dec 4, 2017 at 7:19 AM, Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>
> On 2017年12月04日 15:02, robbieko wrote:
>> From: Robbie Ko <robbieko@synology.com>
>>
>> The commands generated by send contain the following step:
>> 1. mkfile o1851-19-0
>> 2. rename o1851-19-0 -> alsa-driver/alsa-kernel/isa/es1688/es1688.c
>> 3. set_xattr alsa-driver/alsa-kernel/isa/es1688/es1688.c - name=user.xattr data_len=4 data=test
>> 4. write alsa-driver/alsa-kernel/isa/es1688/es1688.c - offset=0, len=10458
>> 5. truncate alsa-driver/alsa-kernel/isa/es1688/es1688.c size=10458
>> 6. chown alsa-driver/alsa-kernel/isa/es1688/es1688.c - uid=1024, gid=100
>> 7. chmod alsa-driver/alsa-kernel/isa/es1688/es1688.c - mode=0644
>> 8. utimes alsa-driver/alsa-kernel/isa/es1688/es1688.c
>>
>> After writing file content, it will truncate file to the correct size.
>> Btrfs truncate will flush last page if size does not align to sectorsize,
>> and this will cause receive process to wait until flush finishes.
>> In order to avoid waiting flushing data.This patch changes the order so
>> that truncate command is sent before write command.
>
> Personally speaking, it's better to optimize the receive side.
>
> For example, at receive side, if we already know that the file size is
> not changed at all, then just skip the truncate command.
>
> In your send dump, step 5 is not needed at all, and can be skipped to
> speed up the receive procedure.

Better yet, is to not send the truncate commands at all, reduces the
stream size and saves all receiver implementations
from having such logic (which further requires stat calls to figure
out the current file size).

I still have a local branch around with several optimizations (from
the time I was a volunteer), with one of them being precisely to avoid
unnecessary truncate commands, however I never ended up getting to the
point of doing benchmarks.
Pasted below and at https://www.friendpaste.com/1J1TuXf2wlU1125mX57k4r


commit 362290ea03b52e4ce4edb43453a51a3b1f1cd33f
Author: Filipe David Borba Manana <fdmanana@gmail.com>
Date:   Sat Apr 5 22:34:45 2014 +0100

    Btrfs: send, don't issue unnecessary file truncate commands

    Every time a file is created or updated, send (either incremental
or not) always
    issues a file truncate command. In several commons scenarios this
truncate is not
    needed and yet it was still issued all the time. The unnecessary
cases where the
    truncate command is not needed are:

    * We have a new file, and its last extent isn't a hole, so that
its last write
      command ends at the file's size;

    * For an incremental send, we updated in place an existing file,
without changing
      its size;

    * For an incremental send, we updated the file, increased its
size, and the last
      file extent item doesn't correspond to a hole. In this case the last write
      command against the file ends at the file's new size;

    * For an incremental send, we didn't update the file's data at
all, we only changed
      its mode, group or access/update timestamps.

    Therefore don't send truncate commands for these cases, reducing
the stream's size
    and saving time.

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index fa378c7ebffd..46f52b4bbc21 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -120,6 +120,7 @@ struct send_ctx {
  u64 cur_inode_mode;
  u64 cur_inode_rdev;
  u64 cur_inode_last_extent;
+ u64 cur_inode_max_write_end;

  u64 send_progress;
  u64 total_data_size;
@@ -4494,6 +4495,8 @@ static int send_hole(struct send_ctx *sctx, u64 end)
  break;
  offset += len;
  }
+ sctx->cur_inode_max_write_end = max(offset,
+    sctx->cur_inode_max_write_end);
 tlv_put_failure:
  fs_path_free(p);
  return ret;
@@ -4529,6 +4532,10 @@ static int send_write_or_clone(struct send_ctx *sctx,
  len = btrfs_file_extent_num_bytes(path->nodes[0], ei);
  }

+ if (offset >= sctx->cur_inode_size) {
+ ret = 0;
+ goto out;
+ }
  if (offset + len > sctx->cur_inode_size)
  len = sctx->cur_inode_size - offset;
  if (len == 0) {
@@ -4560,6 +4567,8 @@ static int send_write_or_clone(struct send_ctx *sctx,
  }
  ret = 0;
  }
+ sctx->cur_inode_max_write_end = max(offset + len,
+    sctx->cur_inode_max_write_end);
 out:
  return ret;
 }
@@ -4982,6 +4991,7 @@ static int finish_inode_if_needed(struct
send_ctx *sctx, int at_end)
  u64 right_gid;
  int need_chmod = 0;
  int need_chown = 0;
+ int need_truncate = 1;
  int pending_move = 0;
  int refs_processed = 0;

@@ -5022,9 +5032,13 @@ static int finish_inode_if_needed(struct
send_ctx *sctx, int at_end)
  need_chown = 1;
  if (!S_ISLNK(sctx->cur_inode_mode))
  need_chmod = 1;
+ if (sctx->cur_inode_max_write_end == sctx->cur_inode_size)
+ need_truncate = 0;
  } else {
+ u64 old_size;
+
  ret = get_inode_info(sctx->parent_root, sctx->cur_ino,
- NULL, NULL, &right_mode, &right_uid,
+ &old_size, NULL, &right_mode, &right_uid,
  &right_gid, NULL);
  if (ret < 0)
  goto out;
@@ -5033,6 +5047,13 @@ static int finish_inode_if_needed(struct
send_ctx *sctx, int at_end)
  need_chown = 1;
  if (!S_ISLNK(sctx->cur_inode_mode) && left_mode != right_mode)
  need_chmod = 1;
+
+ if (old_size == sctx->cur_inode_size &&
+    sctx->cur_inode_max_write_end <= sctx->cur_inode_size)
+ need_truncate = 0;
+ else if (sctx->cur_inode_size > old_size &&
+ sctx->cur_inode_max_write_end == sctx->cur_inode_size)
+ need_truncate = 0;
  }

 truncate_inode:
@@ -5052,10 +5073,13 @@ static int finish_inode_if_needed(struct
send_ctx *sctx, int at_end)
  goto out;
  }
  }
- ret = send_truncate(sctx, sctx->cur_ino, sctx->cur_inode_gen,
- sctx->cur_inode_size);
- if (ret < 0)
- goto out;
+ if (need_truncate) {
+ ret = send_truncate(sctx, sctx->cur_ino,
+    sctx->cur_inode_gen,
+    sctx->cur_inode_size);
+ if (ret < 0)
+ goto out;
+ }
  }

  if (need_chown) {
@@ -5110,6 +5134,7 @@ static int changed_inode(struct send_ctx *sctx,
  sctx->cur_ino = key->objectid;
  sctx->cur_inode_new_gen = 0;
  sctx->cur_inode_last_extent = (u64)-1;
+ sctx->cur_inode_max_write_end = 0;

  /*
  * Set send_progress to current inode. This will tell all get_cur_xxx

>
> Thanks,
> Qu
>
>>
>> Overall performance improves by 102 percent when sending 790000 small files.
>> original: 32m45.311s
>> patch: 16m8.387s
>>
>> Signed-off-by: Robbie Ko <robbieko@synology.com>
>> ---
>>  fs/btrfs/send.c | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
>> index 20d3300..7ae2347 100644
>> --- a/fs/btrfs/send.c
>> +++ b/fs/btrfs/send.c
>> @@ -5857,10 +5857,6 @@ static int finish_inode_if_needed(struct send_ctx *sctx, int at_end)
>>                                       goto out;
>>                       }
>>               }
>> -             ret = send_truncate(sctx, sctx->cur_ino, sctx->cur_inode_gen,
>> -                             sctx->cur_inode_size);
>> -             if (ret < 0)
>> -                     goto out;
>>       }
>>
>>       if (need_chown) {
>> @@ -6044,6 +6040,15 @@ static int changed_inode(struct send_ctx *sctx,
>>                                       sctx->left_path->nodes[0], left_ii);
>>               }
>>       }
>> +     if (result == BTRFS_COMPARE_TREE_NEW ||
>> +         result == BTRFS_COMPARE_TREE_CHANGED) {
>> +             if (S_ISREG(sctx->cur_inode_mode)) {
>> +                     ret = send_truncate(sctx, sctx->cur_ino, sctx->cur_inode_gen,
>> +                                             sctx->cur_inode_size);
>> +                     if (ret < 0)
>> +                             goto out;
>> +             }
>> +     }
>>
>>  out:
>>       return ret;
>>
>



-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”