Request for review (XXS): 7067973: test/java/lang/management/MemoryMXBean/CollectionUsageThreshold.java hanging intermittently

[John Cuthbertson]

Hi Bengt,

The change looks OK to me. I do agree with Jon that a more explanatory 
comment is needed. Perhaps something along the lines:

// We must call G1MonitoringSupport::update_sizes() in the same scoping 
level
// as an active TraceMemoryManagerStats object (i.e. before the 
destructor for the
// TraceMemoryManagerStats is called) so that the G1 memory pools are 
updated
// before any GC notifications are raised.

However, I think that 
G1CollectedHeap::do_collection_pause_at_safepoint() may have the same issue.

The scope in which the TraceMemoryManagerStats object is created is at 
line 3612:

  // Inner scope for scope based logging, timers, and stats collection
  {
    ...
    TraceCollectorStats tcs(g1mm()->incremental_collection_counters());
    TraceMemoryManagerStats tms(false /* fullGC */, gc_cause());

The TraceMemoryManagerStats object is created at line 3629 (above).

The scope is closed at line 3933:

      gc_epilogue(false);
    }
  }

  // The closing of the inner scope, immediately above, will complete
  // logging at the "fine" level. The record_collection_pause_end() call
  // above will complete logging at the "finer" level.

The update_sizes() call is at line 3951:

  print_heap_after_gc();
  g1mm()->update_sizes();

It should be safe to move these two lines into the scope.

A possible way to test this could be to run the test with 
-XX:+ExplicitGCInvokesConcurrent.

Regards,

JohnC

On 05/31/12 13:04, Bengt Rutisson wrote:
>
> Hi all,
>
> Can I have a couple of reviews for this really small change?
> http://cr.openjdk.java.net/~brutisso/7067973/webrev.00/
>
> Background:
> The CollectionUsageThreshold test fails with G1. The test lowers the 
> notification threshold for the G1 old gen memory pool and expects to 
> get a notification after a full GC.
>
> The problem in G1 is that the decision to send the notification is 
> done in TraceMemoryManagerStats::~TraceMemoryManagerStats(). This 
> eventually does pool->get_memory_usage() to get the memory usage after 
> a collection. The problem is that we update this information in 
> G1MonitoringSupport::update_sizes() which is called in 
> G1CollectedHeap::do_collection() _after_ the TraceMemoryManagerStats 
> scope had been exited.
>
> Extending the scope to cover the call to 
> G1MonitoringSupport::update_sizes() solves the issue.
>
> Testing:
> Before this change the CollectionUsageThreshold failed every time I 
> ran it. After this change it passes every time I ran it.
>
> Thanks,
> Bengt