diff mbox series

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

Message ID xp66c7g532kqzu6tai7enm3ribhhvftsr2o7uqfv3qmodcxy5l@tarta.nabijaczleweli.xyz
State Accepted
Commit 7c665e151246cf8b5072ca4f1916f8ed0fa8565c
Delegated to: Tom Rini
Headers show
Series [v4] tools: copyfile: use 64k instead of 512 buffer | expand

Commit Message

Ahelenia Ziemiańska April 9, 2024, 12:14 p.m. UTC
This is a trivial but significant 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 extract the buffer size to a macro.

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 tools/fit_common.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Dragan Simic April 9, 2024, 1:46 p.m. UTC | #1
On 2024-04-09 14:14, Ahelenia Ziemiańska wrote:
> This is a trivial but significant 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 extract the buffer size to a macro.
> 
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

Looking good to me, thanks for the v4.

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

> ---
>  tools/fit_common.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/fit_common.c b/tools/fit_common.c
> index 2d417d47..d1cde16c 100644
> --- a/tools/fit_common.c
> +++ b/tools/fit_common.c
> @@ -23,6 +23,8 @@
>  #include <image.h>
>  #include <u-boot/crc.h>
> 
> +#define COPYFILE_BUFSIZE (64 * 1024)
> +
>  void fit_print_header(const void *fit, struct image_tool_params 
> *params)
>  {
>  	fit_print_contents(fit);
> @@ -145,14 +147,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;
Tom Rini April 18, 2024, 3:40 a.m. UTC | #2
On Tue, 09 Apr 2024 14:14:34 +0200, Ahelenia Ziemiańska wrote:

> This is a trivial but significant 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
> 
> [...]

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/tools/fit_common.c b/tools/fit_common.c
index 2d417d47..d1cde16c 100644
--- a/tools/fit_common.c
+++ b/tools/fit_common.c
@@ -23,6 +23,8 @@ 
 #include <image.h>
 #include <u-boot/crc.h>
 
+#define COPYFILE_BUFSIZE (64 * 1024)
+
 void fit_print_header(const void *fit, struct image_tool_params *params)
 {
 	fit_print_contents(fit);
@@ -145,14 +147,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;