[ECOS] ELF loader slowness & getc_info help

[ECOS] ELF loader slowness & getc_info help

[Matthew J Fletcher]

Hi,

I have a pretty large C++ application which when uncompressed is about
16mb in size (8.5 when gzip compressed).

Loading this from a flash filesystem to memory takes about 50 seconds, a
 bit of diag_printf()'ing shows that 40 seconds of this time is taken in
the 'ch = (*getc)()' function in load_elf_image(), not surprising given
that its called 16 million times for my application.

A quick hack to read 32k blocks takes the time down to about 15 seconds.
Unfortunately the complexity of interacting with getc inards means the
the resulting memory layout is not quite right.


My idea was to use the getc_info directly to avoid all the extra calls
to getc, which resulted in the following code,..


-- snip --
// handle length to read
if (len >= BUF_SIZE)
  read_size = BUF_SIZE;
else
  read_size = len;

// Copy data into memory
while (len > 0)
{
  getc_info.bufp = getc_info.buf;
  getc_info.len = (*getc_info.fun)(getc_info.bufp, read_size,
&getc_info.err);
  getc_info.avail = getc_info.len;

  for (i =0; i < getc_info.len; i++)
  {
    *addr++ = *getc_info.bufp++;
    offset++;

    // knock off what was read
    len--;
    getc_info.avail--;

    if ((unsigned long)(addr-addr_offset) > highest_address)
    highest_address = (unsigned long)(addr - addr_offset);
  }

  if (getc_info.err)
    break;
}
-- end --

Are there any ELF loader experts out there, or redboot getc internals
experts that could shed some light on the brokenness of my hack/patch ?


Some diagnostics from the original code, extra diagnostics are mine.

-- extra diags --
diag_printf("\r\nAFTER, offset %d, addr %d, highest_address %d,
addr_offset %d",offset,addr,highest_address,addr_offset);
diag_printf("\r\ngetc_info, bufp %d, len %d, err %d, avail
%d",getc_info.bufp,getc_info.len,getc_info.err,getc_info.avail);
-- end --



-- snip --
Start PT_LOAD, 13:48:13

BEFORE, offset 116, addr 4194420, highest_address 0, addr_offset 0
getc_info, bufp 1221020, len 32760, err 0, avail 32644

AFTER, offset 15045312, addr 19239616, highest_address 19239616,
addr_offset 0
getc_info, bufp 1229376, len 32760, err 0, avail 24288



BEFORE, offset 15048704, addr 19267584, highest_address 19239616,
addr_offset 0
getc_info, bufp 1232768, len 32760, err 0, avail 20896

AFTER, offset 16876432, addr 21095312, highest_address 21095312,
addr_offset 0
getc_info, bufp 1225936, len 30548, err 1, avail 25516

End PT_LOAD, 13:49:2

Stats:, decompress calls 516, getc calls 16872924, last len -1
Entry point: 0x004000a6, address range: 0x00400000-0x0141e390
-- end --


and with my hack/patch

-- snip --
Start PT_LOAD, 14:48:11

BEFORE, offset 116, addr 4194420, highest_address 0, addr_offset 0
getc_info, bufp 1221020, len 32760, err 0, avail 32644

AFTER, offset 15045312, addr 19239616, highest_address 19239616,
addr_offset 0
getc_info, bufp 1229260, len 8356, err 0, avail 0

Short data reading ELF file
-- end --




It looks like im not adjusting the getc_info.bufp and getc_info.len
correctly, or just failing to understand how they work.




regards

---
Matthew J Fletcher
amimjf(at)sky.com
www.amimjf.org
---


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] ELF loader slowness & getc_info help

[Gary Thomas]

Matthew J Fletcher wrote:
> Hi,
> 
> I have a pretty large C++ application which when uncompressed is about
> 16mb in size (8.5 when gzip compressed).
> 
> Loading this from a flash filesystem to memory takes about 50 seconds, a
>  bit of diag_printf()'ing shows that 40 seconds of this time is taken in
> the 'ch = (*getc)()' function in load_elf_image(), not surprising given
> that its called 16 million times for my application.
> 
> A quick hack to read 32k blocks takes the time down to about 15 seconds.
> Unfortunately the complexity of interacting with getc inards means the
> the resulting memory layout is not quite right.
> 
> 
> My idea was to use the getc_info directly to avoid all the extra calls
> to getc, which resulted in the following code,..
> 
> 
> -- snip --
> // handle length to read
> if (len >= BUF_SIZE)
>   read_size = BUF_SIZE;
> else
>   read_size = len;
> 
> // Copy data into memory
> while (len > 0)
> {
>   getc_info.bufp = getc_info.buf;
>   getc_info.len = (*getc_info.fun)(getc_info.bufp, read_size,
> &getc_info.err);
>   getc_info.avail = getc_info.len;
> 
>   for (i =0; i < getc_info.len; i++)
>   {
>     *addr++ = *getc_info.bufp++;
>     offset++;
> 
>     // knock off what was read
>     len--;
>     getc_info.avail--;
> 
>     if ((unsigned long)(addr-addr_offset) > highest_address)
>     highest_address = (unsigned long)(addr - addr_offset);
>   }
> 
>   if (getc_info.err)
>     break;
> }
> -- end --
> 
> Are there any ELF loader experts out there, or redboot getc internals
> experts that could shed some light on the brokenness of my hack/patch ?

I see plenty of things to comment on, but without knowing where this
code lives and how it's being called, I hesitate to say much.

A patch against the current tree would be much simpler to work with.

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] ELF loader slowness & getc_info help

[Matthew J Fletcher]

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]

> I see plenty of things to comment on, but without knowing where this
> code lives and how it's being called, I hesitate to say much.
> 
> A patch against the current tree would be much simpler to work with.
> 

Thanks for the reply, i was mostly trying to bounce ideas around to see
if my idea of reducing the getc call overhead was a sensible one.

I already changed both CYGNUM_REDBOOT_GETC_BUFFER and
CYGNUM_REDBOOT_LOAD_ZLIB_BUFFER to 0x2000

All changes in load_elf_image() function, against rev 1.51



regards

---
Matthew J Fletcher
amimjf(at)sky.com
www.amimjf.org
---

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2027 bytes --]

--- load.c.151	2009-04-26 15:51:11.000000000 +0100
+++ load.c	2009-04-26 16:02:35.000000000 +0100
@@ -402,7 +402,7 @@
             }
 
             // Copy data into memory
-            while (len-- > 0) {
+            while (len > 0) {
 #ifdef CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS
                 if (!(valid_address(addr) 
 #ifdef CYGBLD_REDBOOT_LOAD_INTO_FLASH
@@ -414,26 +414,39 @@
                     return 0;
                 }
 #endif
-                if ((ch = (*getc)()) < 0) {
-                    err_printf(SHORT_DATA);
-                    redboot_getc_terminate(true);
-                    return 0;
-                }
-#ifdef CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS
-                if (valid_address(addr)) 
-#endif
-                  *addr++ = ch;
-                
-#ifdef CYGBLD_REDBOOT_LOAD_INTO_FLASH
-                else {
-                  flash_load_write(addr, ch);
-                  addr++;
-                }
-#endif
-                offset++;
-                if ((unsigned long)(addr-addr_offset) > highest_address) {
-                    highest_address = (unsigned long)(addr - addr_offset);
-                }
+
+		getc_info.bufp = getc_info.buf;
+		getc_info.len = (*getc_info.fun)(getc_info.bufp, read_size, &getc_info.err);
+		getc_info.avail = getc_info.len;
+
+		for (i =0; i < getc_info.len; i++)
+		{
+    #ifdef CYGSEM_REDBOOT_VALIDATE_USER_RAM_LOADS
+		    if (valid_address(addr)) 
+    #endif
+		      *addr++ = *getc_info.bufp++;
+		    
+    #ifdef CYGBLD_REDBOOT_LOAD_INTO_FLASH
+		    else {
+		      flash_load_write(addr, *getc_info.bufp++);
+		      addr++;
+		    }
+    #endif
+		      // knock off what was read
+		      len--;
+		      getc_info.avail--;
+
+		    if (getc_info.err < 0) {
+			err_printf(SHORT_DATA);
+			redboot_getc_terminate(true);
+			return 0;
+		    }
+
+		    offset++;
+		    if ((unsigned long)(addr-addr_offset) > highest_address) {
+			highest_address = (unsigned long)(addr - addr_offset);
+		    }
+		}
             }
         }
     }


[-- Attachment #3: Type: text/plain, Size: 148 bytes --]

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss