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

[Bela Lubkin]

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
  "./GridText.c", line 4909: warning: modulus by 0
  "./GridText.c", line 9274: warning: modulus by 0
  "./GridText.c", line 9275: warning: modulus by 0
  "./GridText.c", line 11715: warning: modulus by 0
  "./GridText.c", line 11716: warning: modulus by 0
  "./GridText.c", line 11717: warning: modulus by 0
  "./GridText.c", line 12539: warning: modulus by 0
  "./GridText.c", line 12540: warning: modulus by 0
  "./GridText.c", line 12541: warning: modulus by 0

These are all expansions of the macros `POOLallocHTLine' and
`POOLtypecalloc'.

I drilled into the first line, 2951:

    POOLallocHTLine(temp, previous->size);

The macro is:

  #define POOLallocHTLine(ptr, size)  { HTStyleChange* _tmp_;             \
                                        ALLOC_IN_POOL(HTMainText->pool,HTPool,\
                                          _round_(LINE_SIZE(size)),       \
                                          _tmp_,                          \
                                          _align_);                       \
                                        ptr = (HTLine*)_tmp_;             \
                                      }

The expansion is appalling, makes me think something here should be a
function rather than a macro (word-wrapped by me):

  { HTStyleChange * _tmp_ ; if (! HTMainText->pool) { _tmp_ = 0 ; } else {
  if ((sizeof (void *) / sizeof (HTStyleChange))) HTMainText->pool->used
  += (HTMainText->pool->used % (sizeof (void *) / sizeof (HTStyleChange)))
  ; if ((8192 - 4 * sizeof (void *) - sizeof (struct _HTPool *) + sizeof
  (int)) / sizeof (HTStyleChange) - HTMainText->pool->used >= ((sizeof
  (HTLine) + (previous->size)) % sizeof (HTStyleChange) ? (sizeof
  (HTLine) + (previous->size)) / sizeof (HTStyleChange) + 1 : (sizeof
  (HTLine) + (previous->size)) / sizeof (HTStyleChange))) { _tmp_ =
  HTMainText->pool->data + HTMainText->pool->used ; HTMainText->pool->used
  += ((sizeof (HTLine) + (previous->size)) % sizeof (HTStyleChange) ?
  (sizeof (HTLine) + (previous->size)) / sizeof (HTStyleChange) + 1
  : (sizeof (HTLine) + (previous->size)) / sizeof (HTStyleChange)) ;
  } else { HTPool * newpool = (HTPool *) LY_check_calloc (1 , sizeof
  (HTPool)) ; if (! newpool) { _tmp_ = 0 ; } else { newpool->prev =
  HTMainText->pool ; newpool->used = ((sizeof (HTLine) + (previous->size))
  % sizeof (HTStyleChange) ? (sizeof (HTLine) + (previous->size)) / sizeof
  (HTStyleChange) + 1 : (sizeof (HTLine) + (previous->size)) / sizeof
  (HTStyleChange)) ; _tmp_ = newpool->data ; HTMainText->pool = newpool ;
  } } } ; temp = (HTLine *) _tmp_ ; } ;

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.

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

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

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.

>Bela<

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