diff mbox series

[3/6] htm: Use splice() to copy dump

Message ID 20180703065810.14917-3-mikey@neuling.org
State Accepted
Headers show
Series [1/6] htm: Fix compiling on ARM | expand

Commit Message

Michael Neuling July 3, 2018, 6:58 a.m. UTC
Gives 10% speedup for no more complexity.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 libpdbg/htm.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

Comments

Amitay Isaacs July 4, 2018, 4:18 a.m. UTC | #1
On Tue, 2018-07-03 at 16:58 +1000, Michael Neuling wrote:
> Gives 10% speedup for no more complexity.

How about copy_file_range()?

> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>
> ---
>  libpdbg/htm.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/libpdbg/htm.c b/libpdbg/htm.c
> index fa15f5f5d3..a494493935 100644
> --- a/libpdbg/htm.c
> +++ b/libpdbg/htm.c
> @@ -952,41 +952,39 @@ static int do_htm_status(struct htm *htm)
>  	return 1;
>  }
>  
> -#define COPY_BUF_SIZE getpagesize()
>  static int copy_file(int output, int input, uint64_t size)
>  {
> -	char *buf;
>  	size_t r;
> +	int pipefd[2];
> +	int rc = -1;
>  
> -	buf = malloc(COPY_BUF_SIZE);
> -	if (!buf) {
> -		PR_ERROR("Can't malloc buffer\n");
> -		return -1;
> +	if (pipe(pipefd)) {
> +		perror("pipe");
> +		exit(1);
>  	}
>  
>  	while (size) {
> -		r = read(input, buf, MIN(COPY_BUF_SIZE, size));
> +		r = splice(input, 0, pipefd[1], 0, size, 0);
>  		if (r == -1) {
>  			PR_ERROR("Failed to read\n");
>  			goto out;
>  		}
>  		if (r == 0) {
> -			PR_ERROR("EOF\n");
> +			PR_ERROR("Unexpect EOF\n");
>  			goto out;
>  		}
>  
> -		if (write(output, buf, r) != r) {
> +		if (splice(pipefd[0], 0, output, 0, r, 0) != r) {
>  			PR_ERROR("Short write!\n");
>  			goto out;
>  		}
>  		size -= r;
>  	}
> -
> -	return 0;
> -
> +	rc = 0;
>  out:
> -	free(buf);
> -	return -1;
> +	close(pipefd[1]);
> +	close(pipefd[0]);
> +	return rc;
>  
>  }
>  
> -- 
> 2.17.1
> 

Amitay.
Michael Neuling July 5, 2018, 7:48 a.m. UTC | #2
On Wed, 2018-07-04 at 14:18 +1000, Amitay Isaacs wrote:
> On Tue, 2018-07-03 at 16:58 +1000, Michael Neuling wrote:
> > Gives 10% speedup for no more complexity.
> 
> How about copy_file_range()?

Nice but it doesn't work since the FDs are on different filesystems (debugfs vs
real disk)

The man page confirms this:

ERRORS:
....
       EXDEV  The files referred to by file_in and file_out  are  not  on  the
  
           same mounted filesystem.

Mikey
Alistair Popple July 12, 2018, 3:52 a.m. UTC | #3
Out of interest I hacked up a test using sendfile. Results on my laptop were
about the same as using the splice method from this patch. Arguably using
sendfile is cleaner as you don't need the pipe but this patch is perfectly
readable so I've merged it for now. Thanks Mikey.

- Alistair

On Thursday, 5 July 2018 5:48:12 PM AEST Michael Neuling wrote:
> On Wed, 2018-07-04 at 14:18 +1000, Amitay Isaacs wrote:
> > On Tue, 2018-07-03 at 16:58 +1000, Michael Neuling wrote:
> > > Gives 10% speedup for no more complexity.
> > 
> > How about copy_file_range()?
> 
> Nice but it doesn't work since the FDs are on different filesystems (debugfs vs
> real disk)
> 
> The man page confirms this:
> 
> ERRORS:
> ....
>        EXDEV  The files referred to by file_in and file_out  are  not  on  the
>   
>            same mounted filesystem.
> 
> Mikey
>
Michael Neuling July 12, 2018, 3:59 a.m. UTC | #4
On Thu, 2018-07-12 at 13:52 +1000, Alistair Popple wrote:
> Out of interest I hacked up a test using sendfile. Results on my laptop were
> about the same as using the splice method from this patch. Arguably using
> sendfile is cleaner as you don't need the pipe but this patch is perfectly
> readable so I've merged it for now. Thanks Mikey.

Thanks.

I think in the end we get limited by reading the trace from the CI mapped
memory. So there is only so far we can optimise it.

Mikey
diff mbox series

Patch

diff --git a/libpdbg/htm.c b/libpdbg/htm.c
index fa15f5f5d3..a494493935 100644
--- a/libpdbg/htm.c
+++ b/libpdbg/htm.c
@@ -952,41 +952,39 @@  static int do_htm_status(struct htm *htm)
 	return 1;
 }
 
-#define COPY_BUF_SIZE getpagesize()
 static int copy_file(int output, int input, uint64_t size)
 {
-	char *buf;
 	size_t r;
+	int pipefd[2];
+	int rc = -1;
 
-	buf = malloc(COPY_BUF_SIZE);
-	if (!buf) {
-		PR_ERROR("Can't malloc buffer\n");
-		return -1;
+	if (pipe(pipefd)) {
+		perror("pipe");
+		exit(1);
 	}
 
 	while (size) {
-		r = read(input, buf, MIN(COPY_BUF_SIZE, size));
+		r = splice(input, 0, pipefd[1], 0, size, 0);
 		if (r == -1) {
 			PR_ERROR("Failed to read\n");
 			goto out;
 		}
 		if (r == 0) {
-			PR_ERROR("EOF\n");
+			PR_ERROR("Unexpect EOF\n");
 			goto out;
 		}
 
-		if (write(output, buf, r) != r) {
+		if (splice(pipefd[0], 0, output, 0, r, 0) != r) {
 			PR_ERROR("Short write!\n");
 			goto out;
 		}
 		size -= r;
 	}
-
-	return 0;
-
+	rc = 0;
 out:
-	free(buf);
-	return -1;
+	close(pipefd[1]);
+	close(pipefd[0]);
+	return rc;
 
 }