Please review: 8006498 and 8008474

Bengt Rutisson bengt.rutisson at oracle.com
Thu Feb 21 01:05:12 PST 2013


Hi Joe,

I haven't looked at the make file changes, so don't consider this a full 
review.

I have one request regarding the change in 
vm/gc_implementation/g1/concurrentMark.cpp:

The code guarded by VERIFY_OBJS_PROCESSED actually does not work. This 
looks like some debugging code that was left behind. If someone defines 
VERIFY_OBJS_PROCESSED the code does not compile since _scan_obj_cl and 
ThreadLocalObjQueue do not exist. Do you think you can remove this code 
instead of fixing it?

Thanks,
Bengt

Here is the patch if you want to do it:

diff --git a/src/share/vm/gc_implementation/g1/concurrentMark.cpp 
b/src/share/vm/gc_implementation/g1/concurrentMark.cpp
--- a/src/share/vm/gc_implementation/g1/concurrentMark.cpp
+++ b/src/share/vm/gc_implementation/g1/concurrentMark.cpp
@@ -1310,11 +1310,6 @@
      _markStack.expand();
    }

-#if VERIFY_OBJS_PROCESSED
-  _scan_obj_cl.objs_processed = 0;
-  ThreadLocalObjQueue::objs_enqueued = 0;
-#endif
-
    // Statistics
    double now = os::elapsedTime();
    _remark_mark_times.add((mark_work_end - start) * 1000.0);
@@ -2555,17 +2550,6 @@
    guarantee(satb_mq_set.completed_buffers_num() == 0, "invariant");

    print_stats();
-
-#if VERIFY_OBJS_PROCESSED
-  if (_scan_obj_cl.objs_processed != ThreadLocalObjQueue::objs_enqueued) {
-    gclog_or_tty->print_cr("Processed = %d, enqueued = %d.",
-                           _scan_obj_cl.objs_processed,
-                           ThreadLocalObjQueue::objs_enqueued);
-    guarantee(_scan_obj_cl.objs_processed ==
-              ThreadLocalObjQueue::objs_enqueued,
-              "Different number of objs processed and enqueued.");
-  }
-#endif
  }




On 2/20/13 10:47 PM, JOSEPH PROVINO wrote:
> 8006498: ASSERT and other symbols used incorrectly with #if are 
> supposed to be defined or not
>
> 8008474:  -Wundef should be added to the build to catch #if references 
> to undefined macros.
>
>
> Webrev is here: http://cr.openjdk.java.net/~jprovino/8006498/webrev.00/
>
> Thanks.
>
> joe



More information about the hotspot-dev mailing list