[PATCH v2] tools: copyfile: use 64k instead of 512 buffer

[Dragan Simic]

Hello Ahelenia,

Please see my comments below.

On 2024-03-21 19:29, Ahelenia Ziemiańska wrote:
> This is a trivial but significant optimisation:

s/optimisation/optimization/

> mkimage took >200ms (and 49489 writes (of which 49456 512)),
> now it takes  110ms (and   419 writes (of which   386 64k)).
> 
> sendfile is much more appropriate for this and is done in one syscall,
> but doesn't bring any significant speedups over 64k r/w
> at the 13M size ranges, so there's no need to introduce
> 	#if __linux__
> 	while((size = sendfile(fd_dst, fd_src, NULL, 128 * 1024 * 1024)) > 0)
> 		;
> 	if(size != -1) {
> 		ret = 0;
> 		goto out;
> 	}
> 	#endif
> 
> Also lift up the buffer size to a macro.

s/lift up/extract/
s/macro/constant/

> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli at nabijaczleweli.xyz>
> Reviewed-by: Dragan Simic <dsimic at manjaro.org>
> ---
>  tools/fit_common.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/fit_common.c b/tools/fit_common.c
> index 2d417d47..1df4785c 100644
> --- a/tools/fit_common.c
> +++ b/tools/fit_common.c
> @@ -127,6 +127,7 @@ err:
> 
>  int copyfile(const char *src, const char *dst)
>  {
> +#define copyfile_bufsize (64 * 1024)

This is not the right place for such a preprocessor directive.
Instead, it should be placed at the start of the file.

Also, it should use all uppercase letters.

>  	int fd_src = -1, fd_dst = -1;
>  	void *buf = NULL;
>  	ssize_t size;
> @@ -145,14 +146,14 @@ int copyfile(const char *src, const char *dst)
>  		goto out;
>  	}
> 
> -	buf = calloc(1, 512);
> +	buf = calloc(1, copyfile_bufsize);
>  	if (!buf) {
>  		printf("Can't allocate buffer to copy file\n");
>  		goto out;
>  	}
> 
>  	while (1) {
> -		size = read(fd_src, buf, 512);
> +		size = read(fd_src, buf, copyfile_bufsize);
>  		if (size < 0) {
>  			printf("Can't read file %s\n", src);
>  			goto out;