October 2014 - hotspot-gc-dev

RFR: 8061467: Bad page size passed to setup_large_pages() on Solaris
by Erik Helin 06 Nov '14

06 Nov '14

Hi all, this small patch fixes a call to Solaris::setup_large_pages that does not check if the `alignment_hint` parameter is a valid page size (the only such call left). Webrev: http://cr.openjdk.java.net/~ehelin/8061467/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8061467 Testing: - Verified that the fix solves the issue on a Solaris machine that supported NUMA. - JPRT Thanks, Erik

4 8

RFR (S): 8049341: Parallelize clearing the next mark bitmap
by Marcus Larsson 03 Nov '14

03 Nov '14

Hi, I would like reviews for the following patch to parallelize the clearing of the next mark bitmap in G1. Short summary: The heap is divided into as many parts as there are workers and each worker will clear its corresponding part of the bitmap. Workers will join the suspendible thread set instead of the concurrent mark thread to properly allow them to yield during clearing work. Added support for applying heap region closures to a specific part of the heap. SPECjbb2013 shows a slight performance gain with this change (4%, using 4 concurrent threads). Webrev: http://cr.openjdk.java.net/~brutisso/webrev-8049341/ Bug: https://bugs.openjdk.java.net/browse/JDK-8049341 Testing: jprt, SPECjbb2013, SPECjbb2005, SPECjvm2008 Thanks, Marcus

3 9

RFR (XS): 8054491: Remove wrong assert and refactor code in G1CollectorPolicy::record_concurrent_mark_end
by Marcus Larsson 03 Nov '14

03 Nov '14

Hi, Can I have reviews for this small change cleaning up and refactoring record_concurrent_mark_end()? CR: http://cr.openjdk.java.net/~mgerdin/mlarsson/webrev-8054491/ Bug: https://bugs.openjdk.java.net/browse/JDK-8054491 Testing: jprt Thanks, Marcus

3 4

[8u40] Request for review (xs) : backport of 8034056 - assert(_heap_alignment >= _space_alignment) failed: heap_alignment less than space_alignment
by Jon Masamitsu 02 Nov '14

02 Nov '14

This is a backport of 8034056. https://bugs.openjdk.java.net/browse/JDK-8034056 This is the same fix that went into jdk9 but the patch did not apply cleanly so I redid it. http://cr.openjdk.java.net/~jmasa/8034056/webrev.00/

3 4

RFR (S): 8043243: convert SCAN_AND_FORWARD, SCAN_AND_ADJUST_POINTERS, SCAN_AND_COMPACT macros to methods
by Marcus Larsson 30 Oct '14

30 Oct '14

Hi, I would like reviews for the following patch converting the SCAN_AND_* macros into (inline) methods. Short summary: The change is based on the demacroify.patch (bug attachment), using a proxy class to enable different functions for obj_size and similar to be used for different types of Spaces. Each type of space has a proxy defining which methods should be used in the scan_and_* functions. Made no changes to the actual code in the macros, except for replacing a multi-line debug_only(...) with #ifdef ASSERT ... #endif. Webrev: http://cr.openjdk.java.net/~brutisso/webrev-8043243/ Bug: https://bugs.openjdk.java.net/browse/JDK-8043243 Testing: jprt without problems SPECjbb2013, SPECjbb2005, SPECjvm2008 - no significant changes Thanks, Marcus

6 23

RFR: 8061234: ResourceContext.requestAccurateUpdate() is unreliable
by Stefan Johansson 27 Oct '14

27 Oct '14

Hi, Please review this small change: http://cr.openjdk.java.net/~sjohanss/8061234/hotspot.00/ Summary: To reliably be able to provide data for all stats copy_allocation_context_stats needs to return if there are more stats available. Testing: * JPRT * jtreg tests Thanks, Stefan

3 2

RFR: 8061748: Remove check_ct_logs_at_safepoint()
by Kim Barrett 24 Oct '14

24 Oct '14

I would like reviews of this patch to remove the unused function G1CollectedHeap::check_ct_logs_at_safepoint(). It appears to have never had any callers since it was added as part of the initial G1 integration: https://bugs.openjdk.java.net/browse/JDK-6711316 http://hg.openjdk.java.net/jdk9/hs-gc/hotspot/rev/37f87013dfd8 The purpose of this function is to perform a consistency check between the card table and the dirty card logs. If something along those lines is needed for some future debugging task, it's easy enough to write a bespoke test when that time comes. Removal of this function also renders the helper class ClearLoggedCardTableEntryClosure unused, allowing it to be removed as well. The underlying purpose of the removal is to eliminate one of the two callers of CardTableModRefBS::non_clean_card_iterate_serial(), with the intent of later eliminating the other call and allow that function to be removed, thereby eliminating one of several different implementations of iteration over card tables. Webrev: http://cr.openjdk.java.net/~kbarrett/8061748/webrev/ Bug: https://bugs.openjdk.java.net/browse/JDK-8061748 Testing: local jtreg for hotspot/test/{gc,runtime} hotspot/test/closed{gc,runtime} jprt -testset hotspot

3 4

RFR: 8061630: G1 iterates over JNIHandles two times
by Erik Helin 23 Oct '14

23 Oct '14

Hi all, this patch removes a redundant call to JNIHandles::weak_oops_do. The call to JNIHandles::weak_oops_do can be removed because is it also called by ReferenceProcessor::process_discovered_references via G1CollectedHeap::process_discovered_references, see ReferenceProcessor::process_phaseJNI. Both calls to JNIHandles::weak_oops_do uses the same kind of closure for checking if an oop alive, G1STWIsAliveClosure. However, JNIHandles::weak_oops_do in g1CollectedHeap.cpp gets passed an G1KeepAliveClosure, whereas the call to JNIHandles::weak_oops_do in referenceProcessor.cpp gets passed an G1CopyingKeepAliveClosure. Looking at JNIHandleBlock::weak_oops_do, one can see that the `keep_alive` closure will only be called on the root if the `is_alive` closure returns true for the value of the handle. If the G1STWIsAliveClosure returns true, that means the value is either outside the collection set or the value has been forwarded. Since the root (JNIHandle) is outside the G1 heap, G1CopyingKeepAliveClosure will use a G1ParCopyClosure (see _copy_non_heap_obj_cl in G1CopyingKeepAliveClosure). Therefore, if we want to remove the G1KeepAliveClosure phase, we must ensure ourselves that G1ParCopyClosure and G1KeepAliveClosure do the same thing for a JNIHandle root with a value that is either outside the collection set or already forwarded. If the value is outside the collection set and not humongous, then both G1ParCopyClosure and G1STWIsAliveClosure do nothing. If the value is humongous, then both closures calls G1CollectedHeap::set_humongous_is_live. If the value is in the collection set, then G1STWIsAliveClosure assumes it has been forwarded and updates the root. G1ParCopyClosure will also update the root, since the value already has been forwarded. Due to the above, G1ParCopyClosure already does everything G1KeepAliveClosure does for JNIHandles, so there is no need to iterate over them again using G1KeepAliveClosure. Webrev: http://cr.openjdk.java.net/~ehelin/8061630/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8061630 Testing: JPRT Thanks, Erik

3 2

RFR (L): JDK-8061308: Remove iCMS
by Bengt Rutisson 23 Oct '14

23 Oct '14

Hi everyone, Can I have a couple of reviews for this change? HotSpot changes: http://cr.openjdk.java.net/~brutisso/8061308/hotspot/ JDK changes: http://cr.openjdk.java.net/~brutisso/8061308/jdk/ (Just a small update to a test library class.) JBS bug: https://bugs.openjdk.java.net/browse/JDK-8061308 The change removes the incremental mode of CMS. This work is part of JEP 214 (http://openjdk.java.net/jeps/214) There are quite a few lines changed. Mostly it is just removal of code and it should hopefully be fairly straight forward to review. One interesting side effect of removing iCMS was that the ConcEdenSpace (a subclass in the Space hierarchy) could be removed. It implemented the "soft-end" feature that only iCMS was taking advantage of. When I had removed ConcEdenSpace it turned out that its superclass, EdenSpace, was actually an unnecessary class that did not add anything so I removed that too. When I then went to update the comment in space.hpp that describes the Space hierarchy I noticed that the description of the hierarchy was completely out-dated. Rather than bringing it up-to-date I removed the comment. I think that type of information will only get out-dated quickly again and it is much easier to let your IDE create the correct type hierarchy view dynamically for you. Thanks, Bengt

4 8

RFR: BACKOUT - Remove the generations array
by Jesper Wilhelmsson 22 Oct '14

22 Oct '14

Hi, The change to remove the generations array caused non-trivial test failures in the nightly testing that is unlikely to be fixed before the next round of nightly testing starts. According to the new gatekeeper policy this means that the change should be backed out. Webrev for backout: http://cr.openjdk.java.net/~jwilhelm/8061805/webrev.00/ Original webrev: http://cr.openjdk.java.net/~jwilhelm/8055702/webrev.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8061805 Since the webrev is automatically generated by "hg backout" I will review it myself and will therefore only require one additional reviewer. Thanks! /Jesper

3 2