Re: Dead code (was: Re: Good numbers from Ritter's new string allocator)

[Loren James Rittle <rittle at latour dot rsch dot comm dot mot dot com>]

>> Cool. What I'd like to see is an updated ChangeLog, and then we'll wait
>> to see what Loren says. I'd like to check in what you've posted as a
>> starting point: fine tuning can happen in subsequent patches.

> Well, I have a Linux and a pagesize of 4096 is perfect for me, so I
> can only agree with you ;-)

>        * include/bits/basic_string.h (basic_string::_Rep::__default_excess,
>_S_excess_slop):
>        Unused, remove.
>        * include/bits/basic_string.tcc (basic_string::_Rep::_S_create):
>Do not grow
>anymore
>        the capacity one char at a time, align to page boundaries.
>Remove dead code.

For my part, I prefer something like:

* include/bits/basic_string.tcc (basic_string::_Rep::_S_create):
Enforce allocation size blocking policy to reduce fragmentation and
enhance performance with common malloc implementations.

>        * include/bits/c++config: Remove comment.

Only under the observation that no one has taken an interest in doing
the whole job, I could support using pagesize as the primary blocking
factor and checking in my old patch.  However, as Ulrich observed,
malloc implementations vary in a way that the test is subtly more
complex than just taking pagesize.  To my shock, most of the modern
malloc implementations I studied appeared to have no minimal block
size related to VM pagesize nor do they, in general, align "large"
requests to that pagesize.

And, from a study of various malloc implementations, ``4 *
sizeof(void*)'' was seen to be the worst case malloc header size thus
I suppose we could use it without a test.  Given that many platform's
malloc did no alignment whatsoever to VM pagesize (at least when the
allocation request was exactly one or two pages in size), I see little
point in using one default over another.  If anyone with configure
magic wishes to revist this issue, see:

http://gcc.gnu.org/ml/libstdc++/2001-07/msg00083.html

as the basis for a better test.  Anyone that wants to improve the
exponential shaping of progression of allocation sizes, should
consider this policy: first apply exponential growth, then round up to
the nearest size contained by this blocking algorithm.  Of course,
this is only my best guess for general use without tuning it anywhere.

Paolo, I only got your proposed ChangeLog entry for
include/bits/c++config not what comment you propose to remove.

Also, this line needs to be updated before check-in:

>       const size_t __pagesize = 4096; // This magic constant, from OS.

As you observed, there appears to be an autoconfig test for
getpagesize() already.  I am not sure that we like the idea of a
function call on that code path.

Since it has been 4-5 months since this issue was last raised, I would
like to sleep on it until Paolo is available again (since he is
willing to be a co-author of the patch perhaps he can do some more
tuning before we add it to the mainline ;-).

Regards,
Loren