Re: ALLOC_IN_POOL, Re: lynx-dev lynx2.8.5dev.11

[Thomas E. Dickey]

On Sun, 1 Dec 2002, Bela Lubkin wrote:

> Thomas Dickey wrote:
>
> > 2002-11-11 (2.8.5dev.10)
>
> > * modify GridText.c to store lines, anchors, and forms in the same HText 
> > memory
> >   pool as styles.  This will optimize memory allocation/deallocation by 8Kb
> >   units.  The down side:  lines in TRST mode will be stored twice.  Some
> >   structs are made a bit more compact -LP
>
> This change provokes these warnings:
>
>   "./GridText.c", line 2951: warning: modulus by 0

I saw that with Visual C, but "fixed" it by adding the line
        if (align)

I guess your compiler isn't as easily appeased.

> The warning comes from this portion of it:
>
>   += (HTMainText->pool->used % (sizeof (void *) / sizeof (HTStyleChange)))
>
> sizeof(void *) on this machine is 4; sizeof(HTStyleChange) is 8.

right - LP expects too much from the sizeof(void*).

> I was surprised to find that the resulting binary didn't coredump.  This
> is because the previous line (of the word-wrapped macro expansion) says
> `if ((sizeof (void *) / sizeof (HTStyleChange)))' -- the code that's
> generating the warning can never actually be reached (on this machine).

that's the fix I made - I hadn't seen it coredump without the fix, though.

> ALLOC_IN_POOL() is too big to benefit from being a macro.  It attempts
> to provide some type polymorphism, but that (1) isn't being used
> beneficially, and (2) could be done without a huge macro (put the `prev'
> field at the _beginning_ of each pool type so the compiler doesn't have
> to supply its offset).

ok - I hadn't thought of that.

>
> Meanwhile, the following alternative fix probably saves considerable
> space:
>
> --- GridText.c        Sun Dec  1 18:07:38 2002
> +++ GridText2.c       Sun Dec  1 23:19:03 2002
> @@ -158,7 +158,7 @@
>       unsigned int    direction:2;    /* on or off */
>       unsigned int    horizpos: (sizeof(short)*CHAR_BIT-2);
>           /* horizontal position of this change */
> -     unsigned short  style;          /* which style to change to */
> +     unsigned int    style:16;       /* which style to change to */
>  } HTStyleChange;
>
>  #if defined(USE_COLOR_STYLE)
>
> My compiler follows the rules that (1) like bitfields may be combined
> into a single allocation unit of their base type; (2) the base types
> are fully allocated; (3) int and short are not the same base types.
> Thus the two unsigned int bitfields consume only 16 bits of a (32-bit)
> unsigned int, but the full 32 bits are allocated.  The following
> unsigned short takes another 16 bits, and the structure as a whole
> is 32-bit aligned, bringing the total to 64 bits or 8 bytes.
>
> Making `style' a like bitfield allows it to share the original unsigned
> int, bringing the total down to one unsigned int, 4 bytes.  It also
> incidentally fixes the warnings, since now (sizeof(void *) /
> sizeof(HTStyleChange)) == 1.

That's an area where I thought the portability would be strained -
improvements are welcome, but I'm curious how well it actually does
on some of the LP64 machines.  I got bogged down trying to see how to
debug his change for the HTLine's though, and didn't experiment much
with the struct packing.  It seems to work on Tru64, but not extensively
tested; otherwise I've only been on Linux & Windows with it.

-- 
T.E.Dickey <address@hidden>
http://invisible-island.net
ftp://invisible-island.net


; To UNSUBSCRIBE: Send "unsubscribe lynx-dev" to address@hidden