This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] [ping] Use single shared memory block pool for all pool allocators
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Mikhail Maltsev <maltsevm at gmail dot com>
- Cc: Richard Biener <richard dot guenther at gmail dot com>, Andrew Pinski <pinskia at gmail dot com>, Andi Kleen <andi at firstfloor dot org>, gcc-patches mailing list <gcc-patches at gcc dot gnu dot org>, mliska <mliska at suse dot cz>, Jeff Law <law at redhat dot com>
- Date: Tue, 25 Aug 2015 21:54:02 +0100
- Subject: Re: [PATCH] [ping] Use single shared memory block pool for all pool allocators
- Authentication-results: sourceware.org; auth=none
- References: <55B51674 dot 4080203 at gmail dot com> <87y4i2rbxj dot fsf at tassilo dot jf dot intel dot com> <2378E38A-ACE5-47BB-8A45-BEE3F8486E49 at gmail dot com> <CAFiYyc2vA3tptVYR_JR=wTjYkBF44FbrhB=EM39Aj1zGZxAFUA at mail dot gmail dot com> <55BF291A dot 2010405 at gmail dot com> <55D182DB dot 9030006 at gmail dot com>
Mikhail Maltsev <maltsevm@gmail.com> writes:
> Hi, all.
> I'm pinging this patch: https://gcc.gnu.org/ml/gcc-patches/2015-08/msg00030.html
A week and no reply, so FWIW: I really hope this goes in. malloc always
shows up high in my profiles too.
Just a couple of very minor things to show that I read the patch :-)
> diff --git a/gcc/virtual-memory.cc b/gcc/virtual-memory.cc
> new file mode 100644
> index 0000000..69bda37
> --- /dev/null
> +++ b/gcc/virtual-memory.cc
> @@ -0,0 +1,66 @@
> +/*
> + Copyright (C) 2015 Free Software Foundation, Inc.
Missing a description line. Same for the .h.
> +/* Free all memory allocated by memory_block_pool. */
Strictly speaking I think this is "all unused memory".
> + size_t size = reinterpret_cast<_obstack_chunk *> (chunk)->limit -
> + reinterpret_cast<char *>(chunk);
The emacs formatting rule would make this:
size_t size = (reinterpret_cast<_obstack_chunk *> (chunk)->limit
- reinterpret_cast<char *> (chunk));
Still not sure what the rule is supposed to be wrt space before "<"
or after ">".
> +#ifndef VIRTUAL_MEMORY_H
> +#define VIRTUAL_MEMORY_H
Tab indentation for the #define but not the #ifndef. (Think it should
be a space for both.)
> +/* Allocate single block. Reuse previously returned block, if possible. */
Pedantic, but: "a single block", "a previously returned block".
> +inline void *
> +memory_block_pool::allocate ()
> +{
> + if (instance.m_blocks == NULL)
> + return XNEWVEC (char, block_size);
> +
> + void *result = instance.m_blocks;
> + instance.m_blocks = instance.m_blocks->m_next;
> + return result;
> +}
> +
> +/* Return UNCAST_BLOCK to pool. */
"to the pool"
> +inline void
> +memory_block_pool::remove (void *uncast_block)
> +{
> + block_list *block = reinterpret_cast<block_list *> (uncast_block);
For aliasing purposes, should this instead be a placement new,
to show that a new block_list object is being created?
> +extern void *mempool_obstack_chunk_alloc(size_t) ATTRIBUTE_MALLOC;
> +extern void mempool_obstack_chunk_free(void *);
Space before "(".
Thanks,
Richard