On 04/30/2015 04:11 AM, Alberto Garcia wrote: > The qcow2 L2/refcount cache contains one separate table for each cache > entry. Doing one allocation per table adds unnecessary overhead and it > also requires us to store the address of each table separately. > > Since the size of the cache is constant during its lifetime, it's > better to have an array that contains all the tables using one single > allocation. > > In my tests measuring freshly created caches with sizes 128MB (L2) and > 32MB (refcount) this uses around 10MB of RAM less. > > Signed-off-by: Alberto Garcia <be...@igalia.com> > --- > block/qcow2-cache.c | 48 +++++++++++++++++++++--------------------------- > 1 file changed, 21 insertions(+), 27 deletions(-) >
>
> typedef struct Qcow2CachedTable {
> - void* table;
> int64_t offset;
> bool dirty;
> int cache_hits;
> @@ -40,39 +39,34 @@ struct Qcow2Cache {
> struct Qcow2Cache* depends;
> int size;
> bool depends_on_flush;
> + void *table_array;
> + int table_size;
Should this be size_t? [1]
> };
>
> +static inline void *table_addr(Qcow2Cache *c, int table)
> +{
> + return c->table_array + table * c->table_size;
Addition on void* is not portable.
> +}
> +
> Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables)
> {
> BDRVQcowState *s = bs->opaque;
> Qcow2Cache *c;
> - int i;
>
> c = g_new0(Qcow2Cache, 1);
> c->size = num_tables;
> + c->table_size = s->cluster_size;
[1] Oh, maybe not, since BDRVQcowState declares cluster_size as int.
> c->entries = g_try_new0(Qcow2CachedTable, num_tables);
> - if (!c->entries) {
> - goto fail;
> - }
> + c->table_array = qemu_try_blockalign(bs->file, num_tables *
> c->table_size);
Are we sure this won't overflow?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
