Re: Gettext: Suggested addition of memory allocation clean-up in 'intl' library [Again]

[Bruno Haible]

Svante Seleborg wrote:
> In gettext 0.14.5, there is no way to explicitly request deallocation of
> allocated memory.

Why is this a problem? You can
  1. reduce the amount of allocated memory by using LC_ALL=C,
  2. distinguish a memory leak from a static allocation by letting your
     program run a long time and then look at the memory amounts.

> I'd like to suggest and discuss a design to fix this issue, and I may be
> willing to make the implementation after the maintainer and other readers of
> the list agree with a design for this change.
> 
> There are three places in the existing code where clean-up is performed,
> apparently at program exit, when _LIBC is #defined:
> 
> dcigettext.c:
> 
> #ifdef _LIBC
> /* If we want to free all resources we have to do some work at
>    program's end.  */
> libc_freeres_fn (free_mem)
> ...
> #endif
> 
> and
> 
> finddomain.c:
> #ifdef _LIBC
> libc_freeres_fn (free_mem)
> {
> ...
> #endif
> 
> Finally
> 
> in
> 
> localealias.c:
> 
> libc_freeres_ptr (static char *string_space); ...
> libc_freeres_ptr (static struct alias_map *map);
> 
> ---
> 
> I have not verified that these calls actually suffice to free all
> allocations, but this might be a good way to find out...
> 
> ---
> 
> I've not been able to find any documentation for the libc_freeres_ptr and
> libc_freeres_fn macros,

They are defined in glibc-2.3.6/include/libc-internal.h and
glibc-2.3.6/include/libc-symbols.h.

> but some source fragements and their usages seems to
> imply that they implement atexit()-style functionality, without an explicit
> call. Just why atexit() is not used is not apparent - that should be
> portable enough, but there may well be a good reason that I don't know
> about.

It's logical when you know what free()ing a memory block does: With most
malloc implementations, there is a small header before each allocated
memory block. free()ing a memory block modifies this header and does some
bookkeeping in libc-internal variables. The bad point about it is that
free()ing a memory block
  1. pulls the memory page where it sits into main memory if it was swapped
     out,
  2. pulls the memory line where it sits into the L2 or L1 cache.

When a program exits, the optimal behaviour - implemented in Linux -
is that the swapped out memory pages are simply dropped from from the
swap space, and that memory cache entries are freed.

Doing a free_all() via atexit() would have the effect to
1. read into main memory most of the swapped out pages of the process,
   thus forcing a number of unnecessary disk read calls - and nearly the
   same amount of disk write calls needed to swap out memory used by other
   processes.
2. clobber cache lines with useless memory contents, thus slowing down other
   processes.

> 1 - Use atexit instead of libc_xxx like above, and change the code to
> reflect this even in the absence of _LIBC.

This is out of question.

> 2 - Introduce a new call, i.e. 'free_all_memory', and have the caller call
> this whenever appropriate. This of course invalidates all the callers
> pointers to texts etc, so it's not trivial to use.

This is acceptable. The function should be called libintl_free_all().

> It does have the
> advantage that memory can be reclaimed without restarting the program, a
> definite advantage in long running server processes and other cases such as
> an interactive language switch.

When you think that clearing a cache is a good idea, you are forgetting that
it takes some CPU cycles and disk reads to retrieve the value the next time
it will be needed.

> 3 - Change the semantics of one of the existing calls to special case a
> situation of for example a domain name == "" to cause a call of
> 'free_all_memory'.

This is a so horrible idea. Never even think about it. Multiplexing multiple
functionalities into a single function is an abomination and _always_ causes
problems later.

Bruno