Re: Modest string handling optimisations (Was: [Gnustep-cvs] gnustep/core/base ChangeLog Source/NSString.m)

[Richard Frith-Macdonald]

On 26 Aug 2004, at 01:01, Alexander Malmberg wrote:

> Richard Frith-Macdonald wrote:
> > Log message:
> >         Modest string handling optimisations.
> >...
> > RCS file:  
> > /cvsroot/gnustep/cvsroot/gnustep/gnustep/core/base/Source/ 
> > NSString.m,v
> > retrieving revision 1.324
> > retrieving revision 1.325
> > ...
> > @@ -2541,21 +2543,32 @@
> >   */
> >  - (NSString*) lowercaseString
> >...
> > +  if (start.length == 0)
> > +    {
> > +      return self;
> > +    }
>
> This should be "return [[self copy] autorelease];". The current code  
> gets the lifetime of the returned string wrong; consider:
>
> {
>   NSString *foo=[[NSString alloc] init...]; /* do some string  
> manipulation */
>   NSString *returnValue=[foo lowercaseString];
>   [foo release];
>   return returnValue;
> }

If you look at the apple guidelines on this subject, you will see than  
(unless they have changed it recently), the lifetime of an object is  
not guaranteed to be that of the enclosing autorelease pool, and you  
are supposed to copy returned values.  Many methods will return the  
receiver or an ivar of the receiver ... so code that expects a returned  
object to be valid after the object which returned it is destroyed is  
wrong.

The above code fragment is therefore incorrect ... it should have  
retained or copied returnValue before releasing foo.
It's not unreasonable to argue that it's more user-friendly/convenient  
to retain/autorelease all values returned by all methods as standard,  
but it's not as efficient, and it's not general behavior.

My change did not add this as a new behavior (lowercaseString always  
returned self for an empty string) so it's unlikely to have exposed  
many bugs in user code.

> It also fails if the string is mutable. There are some other cases of  
> this in NSString that I've been meaning to fix for a while.

Good point .. I consider this as more of a problem (since the memory  
management policy issue is documented and keeps getting discussed on  
mailing lists etc).  I guess, while it's not technically wrong to  
return a mutable object where a method is declared as the immutable  
version (and lots of MacOSX stuff does it), its more confusing.


The issue with string methods is ... performance.  NSStrings are  
already pretty inefficient.  To what extent should we further  
compromise performance by ensuring that method behaviors are not just  
'correct' (ie conform to documentation), but also avoid these oddities  
with the validity/lifetimes of objects in an attempt to make it easier  
for people to write reliable code with a less full understanding of  
lifetime issues?  Perhaps we should just say ease of use is more  
important than performance.

Also, arguably the apple guidelines are wrong ... in a multithreaded  
environment, even if you retain/copy immediately after receiving a  
returned value, another thread could have destroyed it.  So it's not  
enough to say that you should retain/copy objects immediately, and the  
only 100% safe policy is for any method returning an object to return  
it retained and autoreleased.  However, as most classes do not claim to  
be thread-safe, this is perhaps not an issue.