Re: [PATCH] Enable string mempools to work correctly during initialization phase

On 04/05/2011 07:07 AM, Kinkie wrote:

> Extends string mempools so that they can work before the pools
> subsystem has been fully initialized, as it happens for instance for
> global variables. It does so by explicitly tracking the initialization
> state and by expanding early-allocated strings' sizes in order to mark
> them as non-pooled.

Consider this as a more precise description:

----------
Makes string mempools work before Mem::Init() was called, as may happen
during global variable initialization or early main.cc processing. If
needed, strings allocated before the Mem::Init() call are given an extra
buffer space to make sure the allocated buffer size will not match any
string pool size during deallocation.

Shortcomings: We now waste RAM on buffer increase for early allocated
strings unless they are already bigger than the maximum supported string
pool size. Statistics for early allocations is still broken. Non-string
mempools still do not support early allocations.
----------

* If you can use SizeOfStringBeforePoolsAreInitialized when inializing
the last (biggest) StrPoolsAttrs member, please do so. If you cannot,
add a comment to that member, pointing to the link with
SizeOfStringBeforePoolsAreInitialized that must be kept in sync.

> +// must be at least one byte bigger than the biggest string size

... than the biggest string pool size
(strings themselves may be bigger than that)

> +static const size_t SizeOfStringBeforePoolsAreInitialized = 513;

Not all early-allocated strings will be 513 bytes long. You should also
tie this with MemIsInitialized name. Consider this name instead:
SmallestStringBeforeMemIsInitialized

* Please add spaces around "=" in new assignments unless
SourceFormatting scripts will do that for you.

I have no serious objections to this patch, with or without the above
changes. There is no need to repost unless others need more changes. I
think it would be better to avoid MemIsInitialized and the dependency on
StrPoolsAttrs being a constant POD, but we can do that later.

Thank you,

Alex.
Received on Tue Apr 05 2011 - 20:11:07 MDT