Re: Proposal: block-based vector allocator

[Stefan Monnier]

> +#define VECTOR_BLOCK_SIZE 4096

This needs a comment explaining the choice of constant.  E.g. it should
say that the code makes no particular assumption about this constant, so
it does not have to be a power of 2.

> +    /* On a 32-bit system, rounding up vector size (in bytes) up
> +       to mult-of-8 helps to maintain mult-of-8 alignment.  */
> +    roundup_size = 8

IIUC what the comment should say is something more like "Used to ensure
alignment (e.g. LSB_TAG requires this to be a multiple of 8), as well as
reduce the size of the vector_free_lists array".

> +#define VECTORS_PER_BLOCK_MAX \
> +  (VECTOR_BLOCK_BYTES / sizeof (struct vectorlike_header))

Unused.

> +/* We maintain one free list for each possible block-allocated
> +   vector size, and this is now much of the free lists we have.  */

Should "now much of the" be replace by "the number of"?

> +struct vector_block
> +{
> +  unsigned char data[VECTOR_BLOCK_BYTES];
> +  struct vector_block *next;
> +};

Wouldn't it be easier to define VECTOR_BLOCK_BYTES to be an arbitrary
constant, and then VECTOR_BLOCK_SIZE to be "sizeof (struct vector_block)"?

> -  return (p == m->start && m->type == MEM_TYPE_VECTORLIKE);
> +  if (m->type == MEM_TYPE_VECTORLIKE)
> +    {
> +      if (m->end - m->start == VECTOR_BLOCK_BYTES)

Please just use a new MEM_TYPE, so we don't need to worry about the
possibility of having a non-vector-block which just happens to have the
same size as a vector-block.

Other than that (and Paul's comments), this looks good, thank you very
much,


        Stefan