comparison src/share/vm/gc_implementation/g1/concurrentMark.cpp @ 1358:72f725c5a7be

6940310: G1: MT-unsafe calls to CM::region_stack_push() / CM::region_stack_pop() Summary: Calling the methods region_stack_push() and region_stack_pop() concurrent is not MT-safe. The assumption is that we will only call region_stack_push() during a GC pause and region_stack_pop() during marking. Unfortunately, we also call region_stack_push() during marking which seems to be introducing subtle marking failures. This change introduces lock-based methods for pushing / popping to be called during marking. Reviewed-by: iveresov, johnc
author tonyp
date Mon, 05 Apr 2010 12:19:22 -0400
parents d4197f8d516a
children 23b1b27ac76c
comparison
equal deleted inserted replaced
1357:781e29eb8e08 1358:72f725c5a7be
295 } 295 }
296 // Otherwise, we need to try again. 296 // Otherwise, we need to try again.
297 } 297 }
298 } 298 }
299 299
300 // Currently we do not call this at all. Normally we would call it
301 // during the concurrent marking / remark phases but we now call
302 // the lock-based version instead. But we might want to resurrect this
303 // code in the future. So, we'll leave it here commented out.
304 #if 0
300 MemRegion CMRegionStack::pop() { 305 MemRegion CMRegionStack::pop() {
301 while (true) { 306 while (true) {
302 // Otherwise... 307 // Otherwise...
303 jint index = _index; 308 jint index = _index;
304 309
317 // that entry was invalidated... let's skip it 322 // that entry was invalidated... let's skip it
318 assert(mr.end() == NULL, "invariant"); 323 assert(mr.end() == NULL, "invariant");
319 } 324 }
320 } 325 }
321 // Otherwise, we need to try again. 326 // Otherwise, we need to try again.
327 }
328 }
329 #endif // 0
330
331 void CMRegionStack::push_with_lock(MemRegion mr) {
332 assert(mr.word_size() > 0, "Precondition");
333 MutexLockerEx x(CMRegionStack_lock, Mutex::_no_safepoint_check_flag);
334
335 if (isFull()) {
336 _overflow = true;
337 return;
338 }
339
340 _base[_index] = mr;
341 _index += 1;
342 }
343
344 MemRegion CMRegionStack::pop_with_lock() {
345 MutexLockerEx x(CMRegionStack_lock, Mutex::_no_safepoint_check_flag);
346
347 while (true) {
348 if (_index == 0) {
349 return MemRegion();
350 }
351 _index -= 1;
352
353 MemRegion mr = _base[_index];
354 if (mr.start() != NULL) {
355 assert(mr.end() != NULL, "invariant");
356 assert(mr.word_size() > 0, "invariant");
357 return mr;
358 } else {
359 // that entry was invalidated... let's skip it
360 assert(mr.end() == NULL, "invariant");
361 }
322 } 362 }
323 } 363 }
324 364
325 bool CMRegionStack::invalidate_entries_into_cset() { 365 bool CMRegionStack::invalidate_entries_into_cset() {
326 bool result = false; 366 bool result = false;
3361 if (!_cm->region_stack_empty()) { 3401 if (!_cm->region_stack_empty()) {
3362 if (_cm->verbose_low()) 3402 if (_cm->verbose_low())
3363 gclog_or_tty->print_cr("[%d] draining region stack, size = %d", 3403 gclog_or_tty->print_cr("[%d] draining region stack, size = %d",
3364 _task_id, _cm->region_stack_size()); 3404 _task_id, _cm->region_stack_size());
3365 3405
3366 MemRegion mr = _cm->region_stack_pop(); 3406 MemRegion mr = _cm->region_stack_pop_with_lock();
3367 // it returns MemRegion() if the pop fails 3407 // it returns MemRegion() if the pop fails
3368 statsOnly(if (mr.start() != NULL) ++_region_stack_pops ); 3408 statsOnly(if (mr.start() != NULL) ++_region_stack_pops );
3369 3409
3370 while (mr.start() != NULL) { 3410 while (mr.start() != NULL) {
3371 if (_cm->verbose_medium()) 3411 if (_cm->verbose_medium())
3382 // We finished iterating over the region without aborting. 3422 // We finished iterating over the region without aborting.
3383 regular_clock_call(); 3423 regular_clock_call();
3384 if (has_aborted()) 3424 if (has_aborted())
3385 mr = MemRegion(); 3425 mr = MemRegion();
3386 else { 3426 else {
3387 mr = _cm->region_stack_pop(); 3427 mr = _cm->region_stack_pop_with_lock();
3388 // it returns MemRegion() if the pop fails 3428 // it returns MemRegion() if the pop fails
3389 statsOnly(if (mr.start() != NULL) ++_region_stack_pops ); 3429 statsOnly(if (mr.start() != NULL) ++_region_stack_pops );
3390 } 3430 }
3391 } else { 3431 } else {
3392 assert(has_aborted(), "currently the only way to do so"); 3432 assert(has_aborted(), "currently the only way to do so");
3415 _task_id, 3455 _task_id,
3416 newRegion.start(), newRegion.end()); 3456 newRegion.start(), newRegion.end());
3417 } 3457 }
3418 // Now push the part of the region we didn't scan on the 3458 // Now push the part of the region we didn't scan on the
3419 // region stack to make sure a task scans it later. 3459 // region stack to make sure a task scans it later.
3420 _cm->region_stack_push(newRegion); 3460 _cm->region_stack_push_with_lock(newRegion);
3421 } 3461 }
3422 // break from while 3462 // break from while
3423 mr = MemRegion(); 3463 mr = MemRegion();
3424 } 3464 }
3425 _region_finger = NULL; 3465 _region_finger = NULL;