Re: [PATCH] shrink struct vectorlike_header

[Stefan Monnier <monnier@iro.umontreal.ca>]

> This is an attempt to shrink struct vectorlike_header to the only
> 'size' field. For pseudovectors, 'size' field is rearranged to have
> one more bitfield to record the size of non-Lisp_Object area, thus
> giving the possibility to calculate the memory footprint of any
> vectorlike object.

Thanks, this is a very welcomed change (which will have to wait for the
freeze to end, of course).

> +/* When V is on the free list, V->contents[0] is used as a pointer to
> +   next vector on the free list.  */
> +
> +#define NEXT_IN_FREE_LIST(v)				\
> +  (*(struct Lisp_Vector **)((char *) v + header_size))

Please make the code match the comment, e.g:

     (*(struct Lisp_Vector **)&(v->contents[0]))

> +/* For the large vector V, previous word is used as a pointer
> +   to next large vector.  */
> +
> +#define NEXT_LARGE_VECTOR(v)					\
> +  (*(struct Lisp_Vector **)((char *) v - sizeof (void *)))

You could also use the simpler:

     (((struct Lisp_Vector **)v)[-1])

> +/* Convert from memory block used for large vector to the vector itself.  */
> +
> +#define LARGE_VECTOR_START(mem)					\
> +  (struct Lisp_Vector *)((char *) mem + sizeof (void *))
> +
> +/* Convert from large vector to it's memory block.  */
> +
> +#define LARGE_VECTOR_MEM(v) (void *)((char *) v - sizeof (void *))

Why not define a struct large_vector, so that NEXT_LARGE_VECTOR
can be (VECTOR_TO_LARGE (v)->next), LARGE_VECTOR_START (renamed to
LARGE_TO_VECTOR) can be (&(mem->vector)), and LARGE_VECTOR_MEM (renamed
to VECTOR_TO_LARGE) is defined maybe in terms of offsetof(vector)?

> +static ptrdiff_t
> +vector_nbytes (struct Lisp_Vector *v)
> +{
> +  ptrdiff_t size = v->header.size & ~ARRAY_MARK_FLAG;
> +
> +  if (size & PSEUDOVECTOR_FLAG)
> +    {
> +      if (PSEUDOVECTOR_TYPEP (&v->header, PVEC_BOOL_VECTOR))
> +	return (bool_header_size
> +		+ (((struct Lisp_Bool_Vector *) v)->size
> +		   + BOOL_VECTOR_BITS_PER_CHAR - 1)
> +		/ BOOL_VECTOR_BITS_PER_CHAR);
> +      return (header_size
> +	      + ((size & PSEUDOVECTOR_SIZE_MASK)
> +		 + ((size & PSEUDOVECTOR_REST_MASK)
> +		    >> PSEUDOVECTOR_SIZE_BITS)) * word_size);
> +    }
> +  return header_size + size * word_size;

I'd put an "else" before this last "return", so that if you forget a
"return" somewhere in the first branch of the "if", GCC will tell you.

> +    PVEC_TYPE_MASK = 0x3f << PSEUDOVECTOR_AREA_BITS,

Please add a note where we define all the PVEC_foo types that the max
number of such types is limited by this 0x3f.

        Stefan