[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] shrink struct vectorlike_header
From: |
Stefan Monnier |
Subject: |
Re: [PATCH] shrink struct vectorlike_header |
Date: |
Thu, 11 Oct 2012 08:57:09 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2.50 (gnu/linux) |
> 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