CRR: 6888316 and 6888619 (remove guarantees / change guarantees to asserts in concurrent mark) (S)

Andrey Petrusenko Andrey.Petrusenko at Sun.COM
Wed Oct 7 09:26:21 UTC 2009


Hello Tony,
	Looks good, one minor syntactic thing:
concurrentMark.cpp:
1922   if (ParallelGCThreads > 0) {
	 ...
1937     SATBMarkQueueSet& satb_mq_set = JavaThread::satb_mark_queue_set();
1938     guarantee(satb_mq_set.completed_buffers_num() == 0, "invariant");
1939   } else {
         ...
1951     SATBMarkQueueSet& satb_mq_set = JavaThread::satb_mark_queue_set();
1952     guarantee(satb_mq_set.completed_buffers_num() == 0, "invariant");
1953   }
	I would move the guarantee out of the 'if' statement just to reduce the amount of debugging code:
        guarantee(JavaThread::satb_mark_queue_set().completed_buffers_num() == 0, 
                  "invariant");

Andrey

On Wednesday 07 October 2009 01:31:30 Tony Printezis wrote:
> Hi all,
> 
> Could I please get a couple of code reviews for this one?
> 
> http://cr.openjdk.java.net/~tonyp/6888316/webrev.0/
> 
> The important (and very small) change is the removal of a guarantee that 
> is incorrect. Please have a look at the Evaluation notes on CR 6888316 
> for details.
> 
> The less important (but larger) change is the change of many guarantees 
> in the G1 concurrent marking code into asserts (as we should not have 
> that overhead in the product build). I also took this opportunity to 
> split some of the asserts and tidy up formatting.
> 
> There are about 150 lines changed, but the changes are straightforward, 
> even though somewhat tedious to review.
> 
> Thanks,
> 
> Tony
> 
> 



More information about the hotspot-gc-dev mailing list