422960 - Add jemalloc_stats() and jemalloc.h

Status: RESOLVED FIXED

(Core :: Memory Allocator, enhancement)

Description

[Jason Evans]

Attachment: Add jemalloc_stats() and jemalloc.h

This functionality was requested by Stuart Parmenter, with the intention that it be used to enhance the about:memory output.  Consumers of jemalloc_stats() need to include the jemalloc.h header, as well as link to libjemalloc (OS X only).

The patch modifies js/src/jsgc.c to use jemalloc.h rather than manually declare a posix_memalign() prototype.  This is a cleanup for Windows, and a compilation fix for OS X (for which the change to js/src/Makefile.in is also necessary).

Comment 1

[Jason Evans]

Attachment: Add jemalloc_stats() and jemalloc.h

As suggested by Stuart, this revision of the patch does away with the VISIBLE macro in jemalloc.c, and instead clear VISIBILITY_FLAGS in memory/jemalloc/Makefile.in .

Comment 2

[Jason Evans]

Attachment: Add jemalloc_stats() and jemalloc.h

This is an updated patch (tested on Linux only).

Comment 3

[Jason Evans]

Attachment: Add jemalloc_stats() and jemalloc.h

The previous patch was missing jemalloc.h.

Comment 4

[Jason Evans]

Attachment: Add jemalloc_stats() and jemalloc.h

Update patch to apply to mozilla-central.

Comment 5

[Jason Evans]

changeset:   15458:663c51189e98
user:        Jason Evans <[e-mail]>
date:        Fri Jun 20 10:34:42 2008 -0700
summary:     Bug 422960: Add jemalloc_stats() and jemalloc.h, r=benjamin

Comment 6

[Takanori MATSUURA]

Build failed with glibc-2.8 (Fedora 9 x86_64)
In file included from jsgc.cpp:95:
../../memory/jemalloc/jemalloc.h:47: error: declaration of 'void* malloc(size_t)' throws different exceptions
/usr/include/stdlib.h:471: error: from previous declaration 'void* malloc(size_t) throw ()'
../../memory/jemalloc/jemalloc.h:48: error: declaration of 'void* valloc(size_t)' throws different exceptions
/usr/include/stdlib.h:502: error: from previous declaration 'void* valloc(size_t) throw ()'
../../memory/jemalloc/jemalloc.h:49: error: declaration of 'void* calloc(size_t, size_t)' throws different exceptions
/usr/include/stdlib.h:474: error: from previous declaration 'void* calloc(size_t, size_t) throw ()'
../../memory/jemalloc/jemalloc.h:50: error: declaration of 'void* realloc(void*, size_t)' throws different exceptions
/usr/include/stdlib.h:486: error: from previous declaration 'void* realloc(void*, size_t) throw ()'
../../memory/jemalloc/jemalloc.h:51: error: declaration of 'void free(void*)' throws different exceptions
/usr/include/stdlib.h:488: error: from previous declaration 'void free(void*) throw ()'
../../memory/jemalloc/jemalloc.h:54: error: declaration of 'int posix_memalign(void**, size_t, size_t)' throws different exceptions
/usr/include/stdlib.h:508: error: from previous declaration 'int posix_memalign(void**, size_t, size_t) throw ()'

Comment 7

[Jason Evans]

Thanks for the report.  I'll try to figure out a way to fix this, but it may turn out to be a real pain to reliably do so across a wide variety of libc's and compilers.

Comment 8

[Takanori MATSUURA]

As far as I have checked, glibc-2.3.2 (included in CentOS 3.9) already have posix_memalign function in /usr/include/stdlib.h.
So jsgc.cpp doesn't need to include on current Linux system required by firefox.

Comment 9

[Takanori MATSUURA]

Attachment: possible fix for comment #6.

This may fix comment #6.
But I don't know why the same error occurs in compiling jemalloc.c.

Comment 10

[Takanori MATSUURA]

(In reply to comment #8)
> So jsgc.cpp doesn't need to include on current Linux system required by
> firefox.

Oh.

So jsgc.cpp doesn't need to include "jemalloc.h" on current Linux system required by firefox.

Comment 11

[Takanori MATSUURA]

(In reply to comment #9)
> But I don't know why the same error occurs in compiling jemalloc.c.

But I don't know why the same error "does not" occur in compiling jemalloc.c.

Comment 12

[Takanori MATSUURA]

Attachment: updated patch for attachment 327055.

I suppose this patch is better.

Unfortunately, I can test this only on Linux.

Comment 13

[Takanori MATSUURA]

Is there anything wrong with choosing the reviewer for attachment 353655 [details] [diff] [review]?
Or any problem with my patch?

We have to apply this patch when mozilla products are compiled on x86_64 Linux platform.

Comment 14

[Jason Evans]

Matsuura, unfortunately I am very busy with things besides Mozilla right now, so I don't have the time to test this on lots of operating systems, but that's what needs to happen.  (By the way, I developed the code on x86_64 Linux (Ubuntu), so I wonder if maybe there's something unusual about the compilation environment you are using.)

It might be better to completely remove the prototypes for malloc, calloc, ..., free, since it is safe to assume they are defined on all systems Mozilla has any hope of compiling on.

Comment 15

[Takanori MATSUURA]

(In reply to comment #14)
> (By the way, I developed the code on x86_64 Linux
> (Ubuntu), so I wonder if maybe there's something unusual about the compilation
> environment you are using.)

Which version of glibc/GCC do you use?

Fedora 9: glibc-2.8, gcc-4.3.0
Fedora 10: glibc-2.9, gcc-4.3.2

Comment 16

[Jason Evans]

Ubuntu 8.10: glibc 2.8, gcc 4.3.2

Comment 17

[Takanori MATSUURA]

Ah,
Fedora uses "-fstack-protector" option with compilers as the default for building RPM packages.

In glibc, malloc, etc. and posix_memalign are defined in stdlib.c as the following:
extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur;
extern void *valloc (size_t __size) __THROW __attribute_malloc__ __wur;
extern void *calloc (size_t __nmemb, size_t __size) __THROW __attribute_malloc__ __wur;
extern void *realloc (void *__ptr, size_t __size) __THROW __attribute_warn_unused_result__;
extern void free (void *__ptr) __THROW;

extern int posix_memalign (void **__memptr, size_t __alignment, size_t __size)
     __THROW __nonnull ((1)) __wur;


(In reply to comment #14)
> It might be better to completely remove the prototypes for malloc, calloc, ...,
> free, since it is safe to assume they are defined on all systems Mozilla has
> any hope of compiling on.

I agree with you.

These prototypes should be set ONLY for the platforms which is really needed.  In other words, we should choice not ifndef approach but ifdef approach.
But I don't know which platform needs the prototypes for malloc, calloc, etc. here.

Comment 18

[Takanori MATSUURA]

I filled new bug 526389 for discussions for comment #7 and below.

I think issue filled in this bug has been already fixed and suggest to change status to CLOSED/FIXED.

Comment 19

[Takanori MATSUURA]

The issue I reopened this bug seems to be fixed by attachment 410120 [details] [diff] [review] at bug 526389.

So I change status to RESOLVED/FIXED.