static _int_malloc et al

static _int_malloc et al

[Roland McGrath]

Hi.

Is there any special reason _int_malloc et al should be static rather than
just hidden?  I anticipate at some point using these routines from another
part of libc, in the Hurd port at least.

How about this?  (I'm including the malloc.c patch since it's literally
just the cvs diff of the last commit inverted.)


2003-01-06  Roland McGrath  <roland@redhat.com>

	* malloc/malloc.c: Revert last change.
	* malloc/malloc.h (_int_*): Move these decls to ...
	* include/malloc.h: ... here.  Add attribute_hidden.


Index: malloc/malloc.h
===================================================================
RCS file: /cvs/glibc/libc/malloc/malloc.h,v
retrieving revision 1.22
diff -p -u -r1.22 malloc.h
--- malloc/malloc.h	12 Mar 2002 21:58:54 -0000	1.22
+++ malloc/malloc.h	7 Jan 2003 07:47:49 -0000
@@ -224,18 +224,6 @@ extern void (*__after_morecore_hook) __M
 /* Activate a standard set of debugging hooks. */
 extern void __malloc_check_init __MALLOC_P ((void));
 
-/* Internal routines, operating on "arenas".  */
-struct malloc_state;
-typedef struct malloc_state *mstate;
-
-extern mstate         _int_new_arena __MALLOC_P ((size_t __ini_size));
-extern __malloc_ptr_t _int_malloc __MALLOC_P ((mstate __m, size_t __size));
-extern void           _int_free __MALLOC_P ((mstate __m, __malloc_ptr_t __ptr));
-extern __malloc_ptr_t _int_realloc __MALLOC_P ((mstate __m,
-						__malloc_ptr_t __ptr,
-						size_t __size));
-extern __malloc_ptr_t _int_memalign __MALLOC_P ((mstate __m, size_t __alignment,
-						 size_t __size));
 
 #ifdef __cplusplus
 }; /* end of extern "C" */
Index: include/malloc.h
===================================================================
RCS file: /cvs/glibc/libc/include/malloc.h,v
retrieving revision 1.2
diff -p -u -r1.2 malloc.h
--- include/malloc.h	12 Mar 2002 21:58:14 -0000	1.2
+++ include/malloc.h	7 Jan 2003 07:47:49 -0000
@@ -8,4 +8,19 @@
 /* Nonzero if the malloc is already initialized.  */
 extern int __malloc_initialized attribute_hidden;
 
+/* Internal routines, operating on "arenas".  */
+struct malloc_state;
+typedef struct malloc_state *mstate;
+
+extern mstate         _int_new_arena (size_t __ini_size) attribute_hidden;
+extern __malloc_ptr_t _int_malloc (mstate __m, size_t __size) attribute_hidden;
+extern void           _int_free (mstate __m, __malloc_ptr_t __ptr)
+     attribute_hidden;
+extern __malloc_ptr_t _int_realloc (mstate __m,
+				    __malloc_ptr_t __ptr,
+				    size_t __size) attribute_hidden;
+extern __malloc_ptr_t _int_memalign (mstate __m, size_t __alignment,
+				     size_t __size)
+     attribute_hidden;
+
 #endif

Re: static _int_malloc et al

[Ulrich Drepper]

Roland McGrath wrote:

> Is there any special reason _int_malloc et al should be static rather than
> just hidden?  I anticipate at some point using these routines from another
> part of libc, in the Hurd port at least.

Why wouldn you want to tie yourself to a specific implementation of
malloc?  The _int_* functions have no interface which is in any way
universal and should be preserved if the implementation changes.

I've changed the code to use static since the generated code uses tail
calls.  This can perhaps be achieved with hidden as well but for the
above reasons I need more convincing.

-- 
--------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------

Re: static _int_malloc et al

[Roland McGrath]

> Why wouldn you want to tie yourself to a specific implementation of
> malloc?  The _int_* functions have no interface which is in any way
> universal and should be preserved if the implementation changes.

The purpose of those interfaces is to use private arenas segregated from
the common malloc heap.  That's what I expect to want (when I get around to
it--the Hurd port needs this, and already has it now by virtue of having a
separate, very crappy, allocator for some internal uses).  The interface is
trivially based on the canonical malloc et al interfaces, so I am not
worried about whatever slight adaptations might be needed if the
implementation changes.

> I've changed the code to use static since the generated code uses tail
> calls.  This can perhaps be achieved with hidden as well but for the
> above reasons I need more convincing.

I am presuming that attribute_hidden has all the code-generation benefits
of static, since that's what it's for.  That's why I suggest not changing
it now, even though the _int_* have yet to be used, since I don't think
there's a downside.

Re: static _int_malloc et al

[Ulrich Drepper]

Roland McGrath wrote:
> The interface is
> trivially based on the canonical malloc et al interfaces, so I am not
> worried about whatever slight adaptations might be needed if the
> implementation changes.

Slight adoption?  Who can guarantee that every future malloc
implementation we might want to use uses the concept of arenas?  I
honestly think it's a big mistake to add such dependencies.

-- 
--------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------

Re: static _int_malloc et al

[Roland McGrath]

> Slight adoption?  Who can guarantee that every future malloc
> implementation we might want to use uses the concept of arenas?  I
> honestly think it's a big mistake to add such dependencies.

I never said it had to be guaranteed.  I'm talking about internal uses by
other parts of libc.  The ability to use segregated arenas was one of the
reasons to switch to (the earlier version of) the new malloc
implementation.  If good reasons to have a new implementation come up in
the future, then fine.  We'll change the other code as necessary.  In the
worst case it will need to use a separate allocator implementation, which
is exactly how it is now and it's a crappy one.  The current malloc
implementation is a much better allocator that I can use where we need it,
and as long as we have it I can do it without duplicating code, so why not?

Re: static _int_malloc et al

[Wolfram Gloger]

Hi,

> Why wouldn you want to tie yourself to a specific implementation of
> malloc?  The _int_* functions have no interface which is in any way
> universal and should be preserved if the implementation changes.

Yes, they do -- at least it was my intention to preserve the _int_*
interface for the foreseeable future.  (One of the people requesting
it was Roland.)  Some advantages:

- using them from user-defined hooks is possible.  (Setting the hook
  pointer values "back" to the system values within a hook has some
  problems, in particular it cannot be made thread-safe) So,
  well-performing user-defined hooks can be written easily using
  _int_*.

- when the spinlock patch is in, they can even be used safely from
  within a signal handler.  That AFAICS would provide the only
  possibility to allocate memory in a signal handler in non-pagesize
  chunks, something which you would otherwise have to implement
  yourself with considerable effort.

- plus all of Roland's arguments :-)

> I've changed the code to use static since the generated code uses tail
> calls.  This can perhaps be achieved with hidden as well but for the
> above reasons I need more convincing.

Please keep them public.  If there are performance advantages to be
achieved while keeping them public, please guide me and I will work on
it.

Regards,
Wolfram.

Re: static _int_malloc et al

[Wolfram Gloger]

> - when the spinlock patch is in, they can even be used safely from
>   within a signal handler.

Scratch that, it should simply read "they can even be used safely from
within a signal handler."  No spinlock required when you generate the
arena within the signal handler.

Regards,
Wolfram.

Re: static _int_malloc et al

[Ulrich Drepper]

Wolfram Gloger wrote:

> Please keep them public.  If there are performance advantages to be
> achieved while keeping them public, please guide me and I will work on
> it.

The functions have never been public.  They were not exported from
libc.so and not used outside malloc.c.

Now as for providing the interface to users, this is exactly what the
libc should not do.  Once exported, always exported.  It would mean the
interface would have to be maintained even if the malloc implementation
changes dramatically.

I hope we all agree that this isn't going to happen.  I see that such an
interface is useful.  But then it should be in a library on its own.


Now, as for using it internally, inside libc itself, for Hurd.  Even
though I still think it's a bad idea I cannot and don't want to keep
Roland from making changes like that.  For this the functions should be
marked hidden if this allows generating the same code (it should).

-- 
--------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------

Re: static _int_malloc et al

[Wolfram Gloger]

Hello,

> The functions have never been public.  They were not exported from
> libc.so and not used outside malloc.c.

Ok, I see.

> Now as for providing the interface to users, this is exactly what the
> libc should not do.  Once exported, always exported.  It would mean the
> interface would have to be maintained even if the malloc implementation
> changes dramatically.

True, it is a matter of seeing such a change before glibc-3.x,
whenever that might be.  Personally, I don't see it but it's your
call.  Also, we have a (bad?!) precedent already, malloc_stats().
That, too, would have to be maintained.  I think it would in fact be
easier to maintain _int_* with a future implementation than
malloc_stats().

> I hope we all agree that this isn't going to happen.  I see that such an
> interface is useful.  But then it should be in a library on its own.

Ok.  Can it be arranged that the _int_* functions are marked
__libc_hidden within libc.so but are exported in a separate library,
say libmalloc.so?  libmalloc.so would also ideally have access to
other __libc_hidden symbols in malloc.os from libc.so; I guess this is
impossible to achieve?

Otherwise we would need code duplication in libmalloc.so.  Not ideal,
but if it keeps libc.so "clean"...

Regards,
Wolfram.