Re: [Gnustep-cvs] r33146 - in /libs/gui/trunk: Source/ TextConverters/RTF/ Tools/

[Richard Frith-Macdonald]

On 27 May 2011, at 13:42, David Chisnall wrote:

> Author: theraven
> Date: Fri May 27 14:42:37 2011
> New Revision: 33146
> 
> URL: http://svn.gna.org/viewcvs/gnustep?rev=33146&view=rev
> Log:
> Turn release messages sent to autorelease pools into drain messages.  No 
> functionality change in non-GC mode, in GC mode it invokes a quick GC pass to
> try to delete the short-lived objects.
> 
> Also deleted some [pool release] lines just before exit() or return-from-main
> statements.  These cause objects to be swapped in and destructors to be run to
> no benefit (the OS will reclaim this memory without requiring stuff to be
> swapped in when the process exits).
> 
> 
> Modified:
>    libs/gui/trunk/Source/Functions.m
>    libs/gui/trunk/Source/NSAnimation.m
>    libs/gui/trunk/Source/NSApplication.m
>    libs/gui/trunk/Source/NSAttributedString.m
>    libs/gui/trunk/Source/NSBitmapImageRep.m
>    libs/gui/trunk/Source/NSComboBoxCell.m
>    libs/gui/trunk/Source/NSFileWrapper.m
>    libs/gui/trunk/Source/NSInputManager.m
>    libs/gui/trunk/Source/NSPrinter.m
>    libs/gui/trunk/Source/NSProgressIndicator.m
>    libs/gui/trunk/Source/NSWindow.m
>    libs/gui/trunk/TextConverters/RTF/RTFProducer.m
>    libs/gui/trunk/Tools/GSspell.m
>    libs/gui/trunk/Tools/make_services.m
>    libs/gui/trunk/Tools/set_show_service.m

This might need reverting ... the change from RELEASE(X) to [X drain] is 
generally wrong as it will cause a compile to fail with a complaint about the 
missing variable X when you build in GC mode, since  the use of the RELEASE() 
macro implies that the pool was created using CREATE_AUTORELEASE_POOL(X), and 
in GC mode that evaluates to an empty string and no pool is created.

Alternatively, perhaps we should deprecate the use of the 
CREATE_AUTORELEASE_POOL macro and change its meaning so that it's always 
equivalent to 'NSAutoreleasePool X = [NSAutoreleasePool new]' ?
This would make sense to support mixed GC and non-GC code (I mean code intended 
to work when linked with other code in either world) as we'd then always have 
the pool in existence.
One of the purposes of the macros was to avoid the overheads of 
creating/destroying and sending messages to autorelease pools when running in 
GC mode, but that's a consideration from many years ago when processors were 
slower ...

On another issue ... you say:
> Also deleted some [pool release] lines just before exit() or return-from-main
> statements.  These cause objects to be swapped in and destructors to be run to
> no benefit (the OS will reclaim this memory without requiring stuff to be
> swapped in when the process exits).

I seem to remember this philosophical point being raised before on the mailing 
lists, and someone saying that this was a bad thing to do.
Yes, skipping 'cleanup' on program exit is more efficient, but are machines 
really so slow that this is an issue?  I think it was pointed out that doing 
the release helped with tracking down memory leaks and exposing bugs in the 
code which would show up when the objects are released.  Was some consensus 
reached on this?  I don't recall anyone particularly agreeing with skipping the 
releases (though I wasn't involved in the discussion and may well have missed 
reading some of it).