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

Ahelenia Ziemiańska nabijaczleweli at nabijaczleweli.xyz
Thu Mar 21 21:37:28 CET 2024


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 constant.

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli at nabijaczleweli.xyz>
Reviewed-by: Dragan Simic <dsimic at manjaro.org>
---
On Thu, Mar 21, 2024 at 08:49:52PM +0100, Dragan Simic wrote:
> On 2024-03-21 19:29, Ahelenia Ziemiańska wrote:
> > This is a trivial but significant optimisation:
> s/optimisation/optimization/
This seems to run counter to precedent of not doing americans'
imperialism for them for free
(I see -ise/-ize in free variation even in my 100-deep checkout).

> > +#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.
FTR, neither of these points seem to be universal;
I modelled this after tools/mtk_image.c (also cf. scripts/kconfig/expr.c),
but there are at least 25 other function-local macros
(as in git grep -B1 '^#define' | grep -c -A1 -e '-{' returns 27).

Also kinda weird to explicitly request a macro
only to review it back to a constant, but whatever.

 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..37066203 100644
--- a/tools/fit_common.c
+++ b/tools/fit_common.c
@@ -129,6 +129,7 @@ int copyfile(const char *src, const char *dst)
 {
 	int fd_src = -1, fd_dst = -1;
 	void *buf = NULL;
+	const size_t bufsize = 64 * 1024;
 	ssize_t size;
 	size_t count;
 	int ret = -1;
@@ -145,14 +146,14 @@ int copyfile(const char *src, const char *dst)
 		goto out;
 	}
 
-	buf = calloc(1, 512);
+	buf = calloc(1, 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, bufsize);
 		if (size < 0) {
 			printf("Can't read file %s\n", src);
 			goto out;
-- 
2.39.2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240321/992534e8/attachment.sig>


More information about the U-Boot mailing list