• Type: Bug
• Component: vm-legacy
• Sub-Component: jvmpi
• Affected Version: 6,6u4

• Priority: P3
• Status: Closed
• Resolution: Fixed
• OS: generic,solaris_10
• CPU: generic,sparc

• Submitted: 2007-01-10
• Updated: 2014-09-18
• Resolved: 2011-03-07

JDK 6	JDK 7	Other
6u7Fixed	7Fixed	hs10Fixed

EVALUATION The assert referred to above has still been left in place, but the is_valid_method() has been moved into a CollectedHeap interface. The need for is_in() or is_in_reserved() from this method is gone, and we just rely on is_permanent(). 13-09-2007 SUGGESTED FIX Event: putback-to Parent workspace: /net/jano2.sfbay/export2/hotspot/ws/main/gc_baseline (jano2.sfbay:/export2/hotspot/ws/main/gc_baseline) Child workspace: /net/prt-web.sfbay/prt-workspaces/20070912141354.ysr.merge/workspace (prt-web:/net/prt-web.sfbay/prt-workspaces/20070912141354.ysr.merge/workspace) User: ysr Comment: --------------------------------------------------------- Job ID: 20070912141354.ysr.merge Original workspace: neeraja:/net/spot/workspaces/ysr/merge Submitter: ysr Archived data: /net/prt-archiver.sfbay/data/archived_workspaces/main/gc_baseline/2007/20070912141354.ysr.merge/ Webrev: http://prt-web.sfbay.sun.com/net/prt-archiver.sfbay/data/archived_workspaces/main/gc_baseline/2007/20070912141354.ysr.merge/workspace/webrevs/webrev-2007.09.12/index.html Fixed 6511756: forte_is_valid_method() should call CollectedHeap::is_in_reserved() rather than is_in() Fixed 6602306: PS, ParNew: reference processor's _is_alive_non_header field should be left NULL webrev: http://analemma.sfbay/net/spot/workspaces/ysr/merge/webrev The first allows the so-called interface collectors to be used with Analyzer/Collector. We took the opportunity to consolidate and standardize the check for a "valid" method oop under a CollectedHeap interface. Some simplification followed because tests with "collect -j" using refworkload indicated that the weaker is_in{_reserved}() filter over the entire heap was not adding any additional value. (In that sense the synopsis is now somewhat obsolete, and might have been better rephrased "Sun Studio Collector asserts in GenCollectedHeap::is_in().") The second fixes an inadvertent change to reference processor construction (in the course of the recent extension of parallel reference processing to all parallel collectors) for the case of ParNew and PS, which led to a redundant test during reference discovery. The measured difference in performance is, however, in the noise. Reviewed by: Dan Daugherty, Steve Goldman, Jon Masamitsu Fix verified: yes Verification testing: collect -j java -XX:+UseSerialGC (for instance) Other testing: collect -j on: with refworkload benchmarks running all garbage collectors PRT Dan's Forte Stress Kit (Failures were seen on x86 related to 6380402/6603919, the existence of which predates and is orthogonal to the changes in the present CR; the Forte Stress Kit detected a few hangs when running with Java2Demo, which also similarly predate and appear orthogonal to this change.) Files: update: src/share/vm/gc_implementation/parNew/parNewGeneration.cpp update: src/share/vm/gc_implementation/parallelScavenge/psScavenge.cpp update: src/share/vm/gc_interface/collectedHeap.hpp update: src/share/vm/gc_interface/collectedHeap.inline.hpp update: src/share/vm/memory/compactingPermGenGen.hpp update: src/share/vm/oops/oop.hpp update: src/share/vm/oops/oop.inline2.hpp update: src/share/vm/prims/forte.cpp update: src/share/vm/runtime/fprofiler.cpp Examined files: 4034 Contents Summary: 9 update 4025 no action (unchanged) 13-09-2007 SUGGESTED FIX preliminary webrev:- http://analemma.sfbay.sun.com/net/spot/workspaces/ysr/merge/webrev/ 06-09-2007 EVALUATION Note: the assert affects only the GenCollectedHeap side of things. Current GenCollectedHeap collectors do not actually have a performance problem (despite the pessimistic assert). The assert really belongs in CollectedHeap or specific sub-classes, for example G1CollectedHeap(?), that have non-convex committed regions for which is_in() may be more expensive to compute in general. 24-07-2007 SUGGESTED FIX Note that the above change is safe, because there's a further check further down which protects our use of the putative methodOop, in that we check method->is_perm_and_alloced() which does the stronger test. The stronger (and potentially more expensive) test further down is fine presumably because it will be hit less often and also because in all current and near-future HotSpot configurations (including G1) the committed perm gen is a convex interval of the heap (hmm, i don't know about repercussions for shared perm gens, but may be we don't care much about that at them moment; in any case, i'll check on that). 24-07-2007 SUGGESTED FIX Simple one-line change: bool forte_is_valid_method(methodOop method) { if (method == NULL || // The methodOop is extracted via an offset from the current // interpreter frame. With AsyncGetCallTrace() the interpreter // frame may still be under construction so we need to make // sure that we got an aligned oop before we try to use it. !Space::is_aligned(method) || !Universe::heap()->is_in((void*)method) || ^^^^^^ change to call is_in_reserved(..) instead. Note that calling is_in() is too expensive and would make the profiling code too heacyweight and distort the application execution profile. 10-01-2007