[Mono-devel-list] StringBuilder patch

[Paolo Molaro]

On 12/12/03 Torstensson, Patrik wrote:
> - Uses a String directly as buffer
> - Reallocates string buffer only when ToString has been called on a
> buffer filled more than 50%
> - Keeps internal length/capacity and sets the string length during call
> to ToString()
[...]

Ok, I gave a quick look at the patch (please attach it as a text file
next time, not as octect-stream, so it's easier to view/reply to).
It looks good. Please provide also the benchmarking programs, so we can
compare the numbers too.

I have a few (random) comments:
*) why do you use an additional _capacity field? Isn't _str.Length the
same value?
*) the pattern:
	if (immutable)
		immutable = false;
I think can be written simply as: 
	immutable = false;
The check and cond branch is going to cost more than the memory store.

*) I would replace the immutable flag with a string, as in the previows
implementation: this gives no object size changes on 32 bit systems
anyway and it can be used to avoid the worst case in your code
(ToString() with a less than 50% full _str will always and repeatedly
allocate new strings all equal: just store the result of Substring in
the new string field and return that. The string is simply set to null
on any changes, and if it's set, it's returned in ToString(), like in
the previous implementation). Any reasons not to follow the previous
pattern?

> Please review it as soon as possible, it would be good to get it into
> CVS.

At the end of next week we have a deadline, so, once you have a revised
patch, please wait for me to test it on ppc, since we don't want to
make changes in the core that could impact it. Thanks.

lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better