[flashrom] [PATCH] Reduce realloc syscall overhead for ft2232 and bitbang

[Sean Nelson]

On 11/25/2009 7:31 AM, Paul Fox wrote:
> carl-daniel wrote:
>   >  Reduce realloc syscall overhead for FT2232 and bitbang.
>   >
>   >  FT2232 ran realloc() for every executed command. Start with a big enough
>   >  buffer and don't touch buffer size unless it needs to grow.
>   >  Bitbang was slightly better: It only ran realloc() if buffer size
>   >  changed. Still, the solution above improves performance and reliability.
>
> this is fine, but i'm curious -- did you measure a performance
> change, or is this "by inspection"?  it's never been my
> impression that historically realloc would be slow, unless the
> buffer is growing -- in which case there's no choice.  and my perhaps
> mistaken assumption was that realloc() would usually not shrink a
> buffer.
>
> paul
>
>   >
>   >  Signed-off-by: Carl-Daniel Hailfinger<c-d.hailfinger.devel.2006 at gmx.net>
>   >
>   >  Index: flashrom-realloc_overhead/bitbang_spi.c
>   >  ===================================================================
>   >  --- flashrom-realloc_overhead/bitbang_spi.c	(Revision 777)
>   >  +++ flashrom-realloc_overhead/bitbang_spi.c	(Arbeitskopie)
>   >  @@ -87,14 +87,16 @@
>   >   	static unsigned char *bufout = NULL;
>   >   	static unsigned char *bufin = NULL;
>   >   	static int oldbufsize = 0;
>   >  -	int bufsize = max(writecnt + readcnt, 260);
>   >  +	int bufsize;
>   >   	int i;
>   >
>   >   	/* Arbitrary size limitation here. We're only constrained by memory. */
>   >   	if (writecnt>  65536 || readcnt>  65536)
>   >   		return SPI_INVALID_LENGTH;
>   >
>   >  -	if (bufsize != oldbufsize) {
>   >  +	bufsize = max(writecnt + readcnt, 260);
>   >  +	/* Never shrink. realloc() calls are expensive. */
>   >  +	if (bufsize>  oldbufsize) {
>   >   		bufout = realloc(bufout, bufsize);
>   >   		if (!bufout) {
>   >   			fprintf(stderr, "Out of memory!\n");
>   >  @@ -109,6 +111,7 @@
>   >   				free(bufout);
>   >   			exit(1);
>   >   		}
>   >  +		oldbufsize = bufsize;
>   >   	}
>   >   		
>   >   	memcpy(bufout, writearr, writecnt);
>   >  Index: flashrom-realloc_overhead/ft2232_spi.c
>   >  ===================================================================
>   >  --- flashrom-realloc_overhead/ft2232_spi.c	(Revision 777)
>   >  +++ flashrom-realloc_overhead/ft2232_spi.c	(Arbeitskopie)
>   >  @@ -200,14 +200,22 @@
>   >   	static unsigned char *buf = NULL;
>   >   	/* failed is special. We use bitwise ops, but it is essentially bool. */
>   >   	int i = 0, ret = 0, failed = 0;
>   >  +	int bufsize;
>   >  +	static int oldbufsize = 0;
>   >
>   >   	if (writecnt>  65536 || readcnt>  65536)
>   >   		return SPI_INVALID_LENGTH;
>   >
>   >  -	buf = realloc(buf, writecnt + readcnt + 100);
>   >  -	if (!buf) {
>   >  -		fprintf(stderr, "Out of memory!\n");
>   >  -		exit(1); // -1
>   >  +	/* buf is not used for the response from the chip. */
>   >  +	bufsize = max(writecnt + 9, 260);
>   >  +	/* Never shrink. realloc() calls are expensive. */
>   >  +	if (bufsize>  oldbufsize) {
>   >  +		buf = realloc(buf, bufsize);
>   >  +		if (!buf) {
>   >  +			fprintf(stderr, "Out of memory!\n");
>   >  +			exit(1);
>   >  +		}
>   >  +		oldbufsize = bufsize;
>   >   	}
>   >
>   >   	/*
>   >
>   >
>   >  -- 
>   >  Developer quote of the month:
>   >  "We are juggling too many chainsaws and flaming arrows and tigers."
>
> =---------------------
>   paul fox, pgf at foxharp.boston.ma.us (arlington, ma, where it's 47.1 degrees)
>
> _______________________________________________
> flashrom mailing list
> flashrom at flashrom.org
> http://www.flashrom.org/mailman/listinfo/flashrom
>    
I always assume any memory resizing calls are expensive due to the 
content checking; if shrinking check that the space isn't used, if 
expanding check that the adjacent memory segments aren't used, or 
searching for a space that isn't used.

The patch looks fine and I see no problems so:
Acked-by: Sean Nelson <audiohacked at gmail.com>