Add jemalloc_stats() and jemalloc.h

RESOLVED FIXED

Status

()

enhancement
RESOLVED FIXED
12 years ago
9 years ago

People

(Reporter: jasone, Assigned: jasone)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

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).
Attachment #309453 - Flags: review?(pavlov)
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 .
Attachment #309453 - Attachment is obsolete: true
Attachment #309571 - Flags: review?(pavlov)
Attachment #309453 - Flags: review?(pavlov)
Product: Firefox → Core
QA Contact: general → general
Attachment #309571 - Flags: review?(pavlov) → review+
Assignee: jasone → nobody
Status: ASSIGNED → NEW
Component: General → jemalloc
QA Contact: general → jemalloc
Assignee: nobody → jasone
This is an updated patch (tested on Linux only).
Attachment #309571 - Attachment is obsolete: true
Blocks: 431735
The previous patch was missing jemalloc.h.
Attachment #319437 - Attachment is obsolete: true
Update patch to apply to mozilla-central.
Attachment #319986 - Attachment is obsolete: true
changeset:   15458:663c51189e98
user:        Jason Evans <jasone@canonware.com>
date:        Fri Jun 20 10:34:42 2008 -0700
summary:     Bug 422960: Add jemalloc_stats() and jemalloc.h, r=benjamin
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
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 ()'
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Posted patch possible fix for comment #6. (obsolete) — Splinter Review
This may fix comment #6.
But I don't know why the same error occurs in compiling jemalloc.c.
(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.
(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.
I suppose this patch is better.

Unfortunately, I can test this only on Linux.
Attachment #327055 - Attachment is obsolete: true
Attachment #353655 - Flags: review?(jasone)
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.
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.
(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
Ubuntu 8.10: glibc 2.8, gcc 4.3.2
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.
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.
Blocks: 586962
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.
Status: REOPENED → RESOLVED
Closed: 11 years ago9 years ago
Resolution: --- → FIXED
Attachment #353655 - Flags: review?(jasone)
You need to log in before you can comment on or make changes to this bug.