Project

General

Profile

Actions

Feature #5291

closed

Enabling GC Profiler GC_PROFILE_MORE_DETAIL and CALC_EXACT_MALLOC_SIZE

Added by cfis (Charlie Savage) over 12 years ago. Updated over 12 years ago.

Status:
Closed
Assignee:
-
Target version:
[ruby-core:39326]

Description

I would like to hook ruby-prof into the new GC profiler. However, by default a lot of the profile stats are disabled by these two defines in gcc.c.

#define GC_PROFILE_MORE_DETAIL 0
#define CALC_EXACT_MALLOC_SIZE 0

To turn on these stats, you have to modify the ruby source code! That is a high barrier for most people.

So would it be possible to:

  • Set them to 1 by default. Or would this be too much of a performance hit?
  • or -
  • Remove the lines entirely and then change the logic in the file from this:

#if GC_PROFILE_MORE_DETAIL

To this:

#if defined?(GC_PROFILE_MORE_DETAIL)

Then when buildig Ruby you could tell the compiler to define the symbols (-DGC_PROFILE_MORE_DETAIL).

  • or -
  • Add these two defines to the ./configure script so they end up in config.h. That would make it easy for users to enable them if they wish, and would be also allow ruby-prof to use them too.

Would a patch be accepted to do this?

Updated by normalperson (Eric Wong) over 12 years ago

Charlie Savage wrote:

#define CALC_EXACT_MALLOC_SIZE 0

I tried flipping this a few months back in trunk but didn't have time to
investigate what went wrong when I did. I'm not sure how much people
care for it or if it's even maintained/tested by anyone (I'm not sure
I care).

I have found (and helped fix at least one) 3rd-party extensions that had
an issue with this (due to a x*alloc() paired with a regular free() or
vice-versa). So I suspect this is a common (but currently non-fatal)
bug in other extensions, too.

Anyways, several (most?) malloc implementations support
malloc_usable_size() (or similar) and have various hooks that could be
used to instrument malloc. Unfortunately the various extensions I know
of aren't compatible with each other (dlmalloc/ptmalloc, tcmalloc,
jemalloc).

Updated by cfis (Charlie Savage) over 12 years ago

FYI, pairing a xalloc with a free can be fatal on windows depending on how Ruby is built (mingw is safe, VC++ isn't) and what runtime c libaries are used. See the "Caution" note at the bottom of:

http://msdn.microsoft.com/en-us/library/2kzt1wy3%28v=vs.71%29.aspx

Thanks for fixing them!

Updated by ko1 (Koichi Sasada) over 12 years ago

(2011/09/06 23:04), Charlie Savage wrote:

FYI, pairing a xalloc with a free can be fatal on windows depending on how Ruby is built (mingw is safe, VC++ isn't) and what runtime c libaries are used. See the "Caution" note at the bottom of:

Good point. This measurements can check such errors :P

--
// SASADA Koichi at atdot dot net

Updated by ko1 (Koichi Sasada) over 12 years ago

(2011/09/06 22:05), Charlie Savage wrote:

Issue #5291 has been reported by Charlie Savage.


Feature #5291: Enabling GC Profiler GC_PROFILE_MORE_DETAIL and CALC_EXACT_MALLOC_SIZE
http://redmine.ruby-lang.org/issues/5291

Author: Charlie Savage
Status: Open
Priority: Normal
Assignee:
Category: core
Target version: 1.9.3

I would like to hook ruby-prof into the new GC profiler. However, by default a lot of the profile stats are disabled by these two defines in gcc.c.

#define GC_PROFILE_MORE_DETAIL 0
#define CALC_EXACT_MALLOC_SIZE 0

To turn on these stats, you have to modify the ruby source code! That is a high barrier for most people.

So would it be possible to:

  • Set them to 1 by default. Or would this be too much of a performance hit?
  • or -
  • Remove the lines entirely and then change the logic in the file from this:

#if GC_PROFILE_MORE_DETAIL

To this:

#if defined?(GC_PROFILE_MORE_DETAIL)

Then when buildig Ruby you could tell the compiler to define the symbols (-DGC_PROFILE_MORE_DETAIL).

  • or -
  • Add these two defines to the ./configure script so they end up in config.h. That would make it easy for users to enable them if they wish, and would be also allow ruby-prof to use them too.

Would a patch be accepted to do this?

How about it?

Index: gc.c

--- gc.c (revision 33165)
+++ gc.c (working copy)
@@ -93,7 +93,10 @@
int ruby_gc_debug_indent = 0;

/* for GC profile */
+#ifndef GC_PROFILE_MORE_DETAIL
#define GC_PROFILE_MORE_DETAIL 0
+#endif
+
typedef struct gc_profile_record {
double gc_time;
double gc_mark_time;
@@ -309,7 +312,9 @@
struct gc_list *next;
};

+#ifndef CALC_EXACT_MALLOC_SIZE
#define CALC_EXACT_MALLOC_SIZE 0
+#endif

typedef struct rb_objspace {
struct {

--
// SASADA Koichi at atdot dot net

Updated by cfis (Charlie Savage) over 12 years ago

Yes, that would be much better. Is it too late for this to be part of 1.9.3?

Thank you!

Charlie

Actions #6

Updated by ko1 (Koichi Sasada) over 12 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r33243.
Charlie, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • gc.c (GC_PROFILE_MORE_DETAIL, CALC_EXACT_MALLOC_SIZE):
    define macros only if they are not defined.
    fixes: [Ruby 1.9 - Feature #5291]
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0