Re: [PATCH 1/6] Add memory pool library
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Jun 4, 2010, 18:29 UTC
- Message-ID
- <20100604182928.GB20909@progeny.tock>
- In-Reply-To
- <1275658871-1473-2-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Add a memory pool library implemented using cpp macros. The library > provides macros that can be used to create a type-specific memory pool > API.
More details:
The memory pool library is distinguished from the existing specialized allocators in alloc.c by using a contiguous block for all allocations. This means that on one hand, long-lived pointers have to be written as offsets, since the base address changes as the pool grows, but on the other hand, the entire pool can be easily written to the file system. This allows the memory pool to persist between runs of an application. For svn-fe, such a facility is useful because each svn revision can copy trees and files from any previous revision. Therefore the relevant information for all revisions has to persist somehow to support incremental runs. The current implementation is backed by the file system using mmap().
Thanks for the explanations, David.
Show 6 quoted lines
> +#define obj_pool_gen(pre, obj_t, initial_capacity) \
> +static struct { \
> + uint32_t size; \
> + uint32_t capacity; \
> + obj_t *base; \
> + FILE *file; \Whitespace damaged.
Show 5 quoted lines
> +} pre##_pool = { 0, 0, NULL, NULL}; \
> +static void pre##_init(void) \
> +{ \
> + struct stat st; \
> + size_t ps = sysconf (_SC_PAGESIZE); \Probably an ignorant question, but why? I do not think the win32 mmap emulation in git currently supports sysconf().
Show 5 quoted lines
> + /* Touch binary file before opening read/write */ \ > + pre##_pool.file = fopen(#pre ".bin", "a"); \ > + fclose(pre##_pool.file); \ > + /* Open, check size, compute capacity */ \ > + pre##_pool.file = fopen(#pre ".bin", "r+"); \
Maybe
..._pool.file = fopen(..., "a+"); rewind(..._pool.file);
would be simpler. But given that there is no buffered I/O going on for this file, why not just
..._pool.file = open(..., O_RDWR);
?
> + /* Truncate to calculated capacity and map to VM */ \ > + ftruncate(fileno(pre##_pool.file), pre##_pool.capacity * sizeof(obj_t)); \
Reasonable.
> + pre##_pool.base = mmap(0, pre##_pool.capacity * sizeof(obj_t), \ > + PROT_READ | PROT_WRITE, MAP_SHARED, \ > + fileno(pre##_pool.file), 0); \
This is the first use of MAP_SHARED in git. I wouldn’t be surprised if the win32 mmap emulation does not support it.
Show 8 quoted lines
> +} \
> +static uint32_t pre##_alloc(uint32_t count) \
> +{ \
> + uint32_t offset; \
> + if (pre##_pool.size + count > pre##_pool.capacity) { \
> + if (NULL == pre##_pool.base) \
> + pre##_init(); \
> + fsync(fileno(pre##_pool.file)); \Necessary? (Maybe yes: we are about to truncate the file, so the fsync() may be intended to force the data to be committed before the metadata.) Portable? (I suspect the fsync() should go after the munmap().)
[...]
> +static void pre##_reset(void) \
> +{ \
> + if (pre##_pool.base) { \
> + fsync(fileno(pre##_pool.file)); \Same question.
Show 5 quoted lines
> + munmap(pre##_pool.base, \ > + pre##_pool.capacity * sizeof(obj_t)); \ > + ftruncate(fileno(pre##_pool.file), \ > + pre##_pool.size * sizeof(obj_t)); \ > + fclose(pre##_pool.file); \
Because of these portability concerns, I’d rather use the old version until the incremental parser is ready.
Hope that helps, Jonathan