This is the mail archive of the
glibc-bugs@sourceware.org
mailing list for the glibc project.
[Bug libc/13328] New: proposal of small optimization in tr_freehook() for mtrace functionality
- From: "marcukr at op dot pl" <sourceware-bugzilla at sourceware dot org>
- To: glibc-bugs at sources dot redhat dot com
- Date: Thu, 20 Oct 2011 19:18:49 +0000
- Subject: [Bug libc/13328] New: proposal of small optimization in tr_freehook() for mtrace functionality
- Auto-submitted: auto-generated
http://sourceware.org/bugzilla/show_bug.cgi?id=13328
Bug #: 13328
Summary: proposal of small optimization in tr_freehook() for
mtrace functionality
Product: glibc
Version: unspecified
Status: NEW
Severity: minor
Priority: P2
Component: libc
AssignedTo: drepper.fsp@gmail.com
ReportedBy: marcukr@op.pl
Classification: Unclassified
Hi,
I've started to use mtrace functionality to get the summary of
allocations/deallocations done in my system. First I've got into the deadlock
which I found is solved (Bug #6420) in latest libc. Thanks that I had
opportunity to look into libc sources.
Now I would like to propose small optimization in tr_freehook() function.
IMO the hooks should do their job as fast as possible, on the beginning
they block on lock, then call the original malloc/realloc/free...
and unblock the lock when not needed anymore.
I case of tr_freehook() it does the allocation & deallocation twice, but
most of the time they don't need to be called.
The code from latest glibc is like following:
static void tr_freehook (__ptr_t, const __ptr_t) __THROW;
static void
tr_freehook (ptr, caller)
__ptr_t ptr;
const __ptr_t caller;
{
if (ptr == NULL)
return;
Dl_info mem;
Dl_info *info = lock_and_info (caller, &mem);
tr_where (caller, info);
/* Be sure to print it first. */
fprintf (mallstream, "- %p\n", ptr);
__libc_lock_unlock (lock); <==== the unlock
if (ptr == mallwatch) <==== I didn't know about mallwatch till
looking into mtrace.c source, therefore in many cases it is just NULL
tr_break ();
__libc_lock_lock (lock); <==== the lock again
__free_hook = tr_old_free_hook;
if (tr_old_free_hook != NULL)
(*tr_old_free_hook) (ptr, caller);
else
free (ptr);
__free_hook = tr_freehook;
__libc_lock_unlock (lock);
}
My proposal is to do call "internal" __libc_lock_unlock(lock) &
__libc_lock_lock(lock) only inside if (ptr == mallwatch), therefore the code
will look like following:
static void tr_freehook (__ptr_t, const __ptr_t) __THROW;
.....
{
if (ptr == NULL)
return;
Dl_info mem;
Dl_info *info = lock_and_info (caller, &mem);
tr_where (caller, info);
/* Be sure to print it first. */
fprintf (mallstream, "- %p\n", ptr);
if (ptr == mallwatch)
{
__libc_lock_unlock (lock);
tr_break ();
__libc_lock_lock (lock);
}
__free_hook = tr_old_free_hook;
if (tr_old_free_hook != NULL)
(*tr_old_free_hook) (ptr, caller);
.......
The advantages:
if ptr != mallwatch the lock is locked/unlocked only once -> the hook will be
executed faster
if ptr == mallwatch & gdb breakpoint is set on tr_break() debugger will stop
with lock unlocked (as before the potential optimization change)
Regards
Mariusz
--
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.