Re: [RFC] Enhanced Garbage Collection Probe Points

[Lukas Berk <lberk at redhat dot com>]

Hey list,

I've updated and attached my patch to reflect the comments made in 
the thread above, primarily:

- split begin and end probe points where possible

- correct probe point names

- include and autoconf version of the tapset for immediate inclusion

I've also run the DaCapo benchmarking suite[1] and have attached the
results below.

In order to properly judge the performance effects (if any) my probe
points have had on hotspot, I ran three different sets of tests
(using the eclipse benchmark).  First, with icedtea configured using
--disable-systemtap, then configured with systemtap and no probes
run, and finally with the gc probes in use with a basic log(probestr)
systemtap script. (ie $stap -ve 'probe hotspot.gc_* {log(probestr)}'
-c "java -jar dacapo-9.12-bach.jar eclise -n 10". I ran each test 4
times to get a better sample size.

--disable-systemtap:
run 1: 44119 msec
run 2: 43255 msec
run 3: 44517 msec
run 4: 44017 msec
--------------------
avg: 43977 msec

--enable-systemtap but not used:
run 1: 44987 msec
run 2: 47154 msec
run 3: 48099 msec
run 4: 41069 msec
--------------------
avg: 45327 msec

--enable-systemtap and used:
run 1: 47538 msec
run 2: 43264 msec
run 3: 40396 msec
run 4: 44422 msec
--------------------
avg: 43905 msec

Considering that the probe points seem to have relatively little impact
on the benchmarking results and the interest in my patch, what else
needs to be done to get this patch applied to icedtea?

Cheers,

Lukas

[1] http://dacapobench.org


* Lukas Berk <lberk@redhat.com> [2012-08-02 09:11]:
> Hey,
> 
> I've been working on adding improved probe point within the garbage
> collection system.  This will allow system administrators using various
> tools to better analyze which garbage collection algorithms are
> effective and java developers to better understand how (often) their
> objects are being collected.
> 
> Specific probe points that I've aimed to include are:
> 
> - G1, concurrent mark sweep, parallel mark sweep, and tenured
>   collections
> 
> - new generation definitions
> 
> - parallel scavenges
> 
> - parallel compaction
> 
> - object 'moves/resizes' between memory addresses
> 
> Please note that the attached patch should be appended to the
> patch/systemtap.patch file.  Any feedback or suggestions would be
> greatly appreciated.
> 
> Cheers,
> 
> Lukas

> diff -Nru openjdk.orig/hotspot/src/share/vm/memory/generation.cpp openjdk/hotspot/src/share/vm/memory/generation.cpp
> --- openjdk.orig/hotspot/src/share/vm/memory/generation.cpp	2012-06-15 11:36:43.022837742 -0400
> +++ openjdk/hotspot/src/share/vm/memory/generation.cpp	2012-06-15 13:35:39.714838057 -0400
> @@ -40,6 +40,11 @@
>  #include "runtime/java.hpp"
>  #include "utilities/copy.hpp"
>  #include "utilities/events.hpp"
> +#include "utilities/dtrace.hpp"
> +
> +#ifndef USDT2
> +  HS_DTRACE_PROBE_DECL4(provider, gc__collection__contig, bool, bool, size_t, bool);
> +#endif /* !USDT2 */
>  
>  Generation::Generation(ReservedSpace rs, size_t initial_size, int level) :
>    _level(level),
> @@ -471,6 +476,9 @@
>    ReferenceProcessorSpanMutator
>      x(ref_processor(), GenCollectedHeap::heap()->reserved_region());
>    GenMarkSweep::invoke_at_safepoint(_level, ref_processor(), clear_all_soft_refs);
> +#ifndef USDT2
> +  HS_DTRACE_PROBE4(hotspot, gc__collection__contig, full, clear_all_soft_refs, size, is_tlab);
> +#endif  /* !USDT2 */
>    SpecializationStats::print();
>  }
>  
> diff -Nru openjdk.orig/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp openjdk/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp
> --- openjdk.orig/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp	2012-06-15 11:36:42.164837741 -0400
> +++ openjdk/hotspot/src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp	2012-06-15 13:35:45.224838227 -0400
> @@ -55,6 +55,11 @@
>  #include "runtime/vmThread.hpp"
>  #include "services/memoryService.hpp"
>  #include "services/runtimeService.hpp"
> +#include "utilities/dtrace.hpp"
> +
> +#ifndef USDT2
> +  HS_DTRACE_PROBE_DECL4(provider, gc__collection__contig, bool, bool, size_t, bool);
> +#endif /* !USDT2 */
>  
>  // statics
>  CMSCollector* ConcurrentMarkSweepGeneration::_collector = NULL;
> @@ -1655,6 +1660,10 @@
>                             size_t size,
>                             bool   tlab)
>  {
> +
> +#ifndef USDT2
> +  HS_DTRACE_PROBE4(hotspot, gc__collection__contig, full, clear_all_soft_refs, size, tlab);
> +#endif /* !USDT2 */
>    if (!UseCMSCollectionPassing && _collectorState > Idling) {
>      // For debugging purposes skip the collection if the state
>      // is not currently idle
> diff -Nru openjdk.orig/hotspot/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp openjdk/hotspot/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp
> --- openjdk.orig/hotspot/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp	2012-06-15 11:36:41.816837741 -0400
> +++ openjdk/hotspot/src/share/vm/gc_implementation/parNew/parNewGeneration.cpp	2012-06-15 13:35:56.298821181 -0400
> @@ -49,6 +49,11 @@
>  #include "utilities/copy.hpp"
>  #include "utilities/globalDefinitions.hpp"
>  #include "utilities/workgroup.hpp"
> +#include "utilities/dtrace.hpp"
> +
> +#ifndef USDT2
> +  HS_DTRACE_PROBE_DECL4(provider, gc__collection__parnew, bool, bool, size_t, bool);
> +#endif /* !USDT2 */
>  
>  #ifdef _MSC_VER
>  #pragma warning( push )
> @@ -878,6 +883,9 @@
>                                 bool   clear_all_soft_refs,
>                                 size_t size,
>                                 bool   is_tlab) {
> +#ifndef USDT2
> +  HS_DTRACE_PROBE4(hotspot, gc__collection__parnew, full, clear_all_soft_refs, size, is_tlab);
> +#endif  /* !USDT2 */
>    assert(full || size > 0, "otherwise we don't want to collect");
>    GenCollectedHeap* gch = GenCollectedHeap::heap();
>    assert(gch->kind() == CollectedHeap::GenCollectedHeap,
> diff -Nru openjdk.orig/hotspot/src/share/vm/memory/defNewGeneration.cpp openjdk/hotspot/src/share/vm/memory/defNewGeneration.cpp
> --- openjdk.orig/hotspot/src/share/vm/memory/defNewGeneration.cpp	2012-06-15 11:36:42.970837742 -0400
> +++ openjdk/hotspot/src/share/vm/memory/defNewGeneration.cpp	2012-06-15 13:36:05.328838805 -0400
> @@ -39,6 +39,11 @@
>  #include "runtime/java.hpp"
>  #include "utilities/copy.hpp"
>  #include "utilities/stack.inline.hpp"
> +#include "utilities/dtrace.hpp"
> +
> +#ifndef USDT2
> +  HS_DTRACE_PROBE_DECL4(provider, gc__collection__defnew, bool, bool, size_t, bool);
> +#endif /* !USDT2 */
>  #ifdef TARGET_OS_FAMILY_linux
>  # include "thread_linux.inline.hpp"
>  #endif
> @@ -528,6 +533,9 @@
>                                 bool   clear_all_soft_refs,
>                                 size_t size,
>                                 bool   is_tlab) {
> +#ifndef USDT2
> +  HS_DTRACE_PROBE4(hotspot, gc__collection__defnew, full, clear_all_soft_refs, size, is_tlab);
> +#endif  /* !USDT2 */
>    assert(full || size > 0, "otherwise we don't want to collect");
>    GenCollectedHeap* gch = GenCollectedHeap::heap();
>    _next_gen = gch->next_gen(this);
> diff -Nru openjdk.orig/hotspot/src/share/vm/memory/tenuredGeneration.cpp openjdk/hotspot/src/share/vm/memory/tenuredGeneration.cpp
> --- openjdk.orig/hotspot/src/share/vm/memory/tenuredGeneration.cpp	2012-06-15 11:36:43.016837742 -0400
> +++ openjdk/hotspot/src/share/vm/memory/tenuredGeneration.cpp	2012-06-15 13:36:08.848839364 -0400
> @@ -33,6 +33,11 @@
>  #include "memory/tenuredGeneration.hpp"
>  #include "oops/oop.inline.hpp"
>  #include "runtime/java.hpp"
> +#include "utilities/dtrace.hpp"
> +
> +#ifndef USDT2
> +  HS_DTRACE_PROBE_DECL4(provider, gc__collection__tenured, bool, bool, size_t, bool);
> +#endif /* !USDT2 */
>  
>  TenuredGeneration::TenuredGeneration(ReservedSpace rs,
>                                       size_t initial_byte_size, int level,
> @@ -307,6 +312,9 @@
>                                  size_t size,
>                                  bool   is_tlab) {
>    retire_alloc_buffers_before_full_gc();
> +#ifndef USDT2
> +  HS_DTRACE_PROBE4(hotspot, gc__collection__tenured, full, clear_all_soft_refs, size, is_tlab);
> +#endif  /* !USDT2 */
>    OneContigSpaceCardGeneration::collect(full, clear_all_soft_refs,
>                                          size, is_tlab);
>  }
> diff -Nru openjdk.orig/hotspot/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp openjdk/hotspot/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp
> --- openjdk.orig/hotspot/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp
> +++ openjdk/hotspot/src/share/vm/gc_implementation/parallelScavenge/parallelScavengeHeap.cpp
> @@ -40,8 +40,13 @@
>  #include "runtime/handles.inline.hpp"
>  #include "runtime/java.hpp"
>  #include "runtime/vmThread.hpp"
> +#include "utilities/dtrace.hpp"
>  #include "utilities/vmError.hpp"
>  
> +#ifndef USDT2
> +  HS_DTRACE_PROBE_DECL2(provider, gc__collection__parscavenge, *uintptr_t, *uintptr_t);
> +#endif /* !USDT2 */
> +
>  PSYoungGen*  ParallelScavengeHeap::_young_gen = NULL;
>  PSOldGen*    ParallelScavengeHeap::_old_gen = NULL;
>  PSPermGen*   ParallelScavengeHeap::_perm_gen = NULL;
> @@ -806,6 +811,9 @@
>    }
>  
>    VM_ParallelGCSystemGC op(gc_count, full_gc_count, cause);
> +#ifndef USDT2
> +  HS_DTRACE_PROBE2(hotspot, gc__collection__parscavenge, &op, cause);
> +#endif /* !USDT2 */
>    VMThread::execute(&op);
>  }
> 
> diff -Nru openjdk.orig/hotspot/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp openjdk/hotspot/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp
> --- openjdk.orig/hotspot/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp	2012-06-26 09:24:22.472325287 -0400
> +++ openjdk/hotspot/src/share/vm/gc_implementation/g1/g1MarkSweep.cpp	2012-07-05 10:43:08.273800575 -0400
> @@ -45,8 +45,13 @@
>  #include "runtime/thread.hpp"
>  #include "runtime/vmThread.hpp"
>  #include "utilities/copy.hpp"
> +#include "utilities/dtrace.hpp"
>  #include "utilities/events.hpp"
>  
> +#ifndef USDT2
> +  HS_DTRACE_PROBE_DECL2(provider, gc__collection__G1, *uintptr_t, *uintptr_t);
> +#endif /* !USDT2 */
> +
>  class HeapRegion;
>  
>  void G1MarkSweep::invoke_at_safepoint(ReferenceProcessor* rp,
> @@ -83,7 +88,9 @@
>    // We should save the marks of the currently locked biased monitors.
>    // The marking doesn't preserve the marks of biased objects.
>    BiasedLocking::preserve_marks();
> -
> +#ifndef USDT2
> +  HS_DTRACE_PROBE2(hotspot, gc__collection__G1, &sh, sh->gc_cause());
> +#endif /* !USDT2 */
>    mark_sweep_phase1(marked_for_unloading, clear_all_softrefs);
>  
>    mark_sweep_phase2();
> diff -Nru openjdk.orig/hotspot/src/share/vm/compiler/oopMap.cpp openjdk/hotspot/src/share/vm/compiler/oopMap.cpp
> --- openjdk.orig/hotspot/src/share/vm/compiler/oopMap.cpp	2012-06-26 09:24:22.390325184 -0400
> +++ openjdk/hotspot/src/share/vm/compiler/oopMap.cpp	2012-07-06 10:12:44.981413003 -0400
> @@ -33,9 +33,13 @@
>  #include "memory/resourceArea.hpp"
>  #include "runtime/frame.inline.hpp"
>  #include "runtime/signature.hpp"
> +#include "utilities/dtrace.hpp"
>  #ifdef COMPILER1
>  #include "c1/c1_Defs.hpp"
>  #endif
> +#ifndef USDT2
> +  HS_DTRACE_PROBE_DECL1(provider, gc__collection__delete, *uintptr_t);
> +#endif /* !USDT2 */
>  
>  // OopMapStream
>  
> @@ -677,6 +681,9 @@
>                      " - Derived: " INTPTR_FORMAT "  Base: " INTPTR_FORMAT " (Offset: %d)",
>            derived_loc, (address)*derived_loc, (address)base, offset);
>      }
> +#ifndef USDT2
> +  HS_DTRACE_PROBE1(hotspot, gc__collection__delete, entry);
> +#endif /* !USDT2 */
>  
>      // Delete entry
>      delete entry;
> --- openjdk.orig/hotspot/src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.cpp	2012-07-12 09:48:40.349999515 -0400
> +++ openjdk/hotspot/src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.cpp	2012-07-19 18:38:07.560757426 -0400
> @@ -53,11 +53,18 @@
>  #include "runtime/vmThread.hpp"
>  #include "services/management.hpp"
>  #include "services/memoryService.hpp"
> +#include "utilities/dtrace.hpp"
>  #include "utilities/events.hpp"
>  #include "utilities/stack.inline.hpp"
>  
>  #include <math.h>
>  
> +#ifndef USDT2
> +  HS_DTRACE_PROBE_DECL2(provider, gc__collection__ParallelCompact__clear, *uintptr_t, *uintptr_t);
> +  HS_DTRACE_PROBE_DECL2(provider, gc__collection__partest1, *uintptr_t, *uintptr_t);
> +  HS_DTRACE_PROBE_DECL4(provider, gc__collection__move, *uintptr_t, *uintptr_t, *uintptr_t, *uintptr_t);
> +#endif /* !USDT2 */
> +
>  // All sizes are in HeapWords.
>  const size_t ParallelCompactData::Log2RegionSize  = 9; // 512 words
>  const size_t ParallelCompactData::RegionSize      = (size_t)1 << Log2RegionSize;
> @@ -433,6 +439,9 @@
>  
>  void ParallelCompactData::clear()
>  {
> +#ifndef USDT2
> +  HS_DTRACE_PROBE2(hotspot, gc__collection__ParallelCompact__clear, &_region_data, _region_data->data_location());
> +#endif /* !USDT2 */
>    memset(_region_data, 0, _region_vspace->committed_size());
>  }
>  
> @@ -1970,6 +1979,9 @@
>           "should be in vm thread");
>  
>    ParallelScavengeHeap* heap = gc_heap();
> +#ifndef USDT2
> +  HS_DTRACE_PROBE2(hotspot, gc__collection__partest1, heap, heap->gc_cause());
> +#endif /* !USDT2 */
>    GCCause::Cause gc_cause = heap->gc_cause();
>    assert(!heap->is_gc_active(), "not reentrant");
>  
> @@ -3376,6 +3388,9 @@
>    // past the end of the partial object entering the region (if any).
>    HeapWord* const dest_addr = sd.partial_obj_end(dp_region);
>    HeapWord* const new_top = _space_info[space_id].new_top();
> +#ifndef USDT2
> +  HS_DTRACE_PROBE4(hotspot, gc__collection__move, &beg_addr, &end_addr, &dest_addr, &new_top);
> +#endif /* !USDT2 */
>    assert(new_top >= dest_addr, "bad new_top value");
>    const size_t words = pointer_delta(new_top, dest_addr);
>  
> --- openjdk.orig/hotspot/src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp	2012-07-12 09:48:40.401000822 -0400
> +++ openjdk/hotspot/src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp	2012-07-19 18:36:43.787767399 -0400
> @@ -51,8 +51,12 @@
>  #include "runtime/vmThread.hpp"
>  #include "runtime/vm_operations.hpp"
>  #include "services/memoryService.hpp"
> +#include "utilities/dtrace.hpp"
>  #include "utilities/stack.inline.hpp"
>  
> +#ifndef USDT2
> +  HS_DTRACE_PROBE_DECL2(provider, name, *uintptr_t, *uintptr_t);
> +#endif /* !USDT2 */
>  
>  HeapWord*                  PSScavenge::_to_space_top_before_gc = NULL;
>  int                        PSScavenge::_consecutive_skipped_scavenges = 0;
> @@ -226,7 +230,13 @@
>    PSAdaptiveSizePolicy* policy = heap->size_policy();
>    IsGCActiveMark mark;
>  
> +#ifndef USDT2
> +  HS_DTRACE_PROBE2(hotspot, gc__collection__PSScavenge__begin, *heap, heap->gc_cause());
> +#endif /* !USDT2 */
>    const bool scavenge_done = PSScavenge::invoke_no_policy();
> +#ifndef USDT2
> +  HS_DTRACE_PROBE2(hotspot, gc__collection__PSScavenge__end, *heap, heap->gc_cause());
> +#endif /* !USDT2 */
>    const bool need_full_gc = !scavenge_done ||
>      policy->should_full_GC(heap->old_gen()->free_in_bytes());
>    bool full_gc_done = false;
> @@ -243,9 +253,27 @@
>      const bool clear_all_softrefs = cp->should_clear_all_soft_refs();
>  
>      if (UseParallelOldGC) {
> +#ifndef USDT2
> +  HS_DTRACE_PROBE2(hotspot, gc__collection__PSParallelCompact__begin, *heap, heap->gc_cause()); 
> +#endif /* !USDT2 */
> +
>        full_gc_done = PSParallelCompact::invoke_no_policy(clear_all_softrefs);
> +
> +#ifndef USDT2
> +  HS_DTRACE_PROBE2(hotspot, gc__collection__PSParallelCompact__end, *heap, heap->gc_cause());
> +#endif /* !USDT2 */
> +
>      } else {
> +
> +#ifndef USDT2
> +  HS_DTRACE_PROBE2(hotspot, gc__collection__PSMarkSweep__begin, *heap, heap->gc_cause());
> +#endif /* !USDT2 */
> +
>        full_gc_done = PSMarkSweep::invoke_no_policy(clear_all_softrefs);
> +
> +#ifndef USDT2
> +  HS_DTRACE_PROBE2(hotspot, gc__collection__PSMarkSweep__end, *heap, heap->gc_cause());
> +#endif /* !USDT2 */
>      }
>    }
>

Attachment: icedtea.patch
Description: Text document

Attachment: pgp00000.pgp
Description: PGP signature