United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6884552 remove some unnecessary #ifdef's introduced in the fix for 4957990
JDK-6884552 : remove some unnecessary #ifdef's introduced in the fix for 4957990

Details
Type:
Bug
Submit Date:
2009-09-22
Status:
Resolved
Updated Date:
2010-04-02
Project Name:
JDK
Resolved Date:
2009-09-30
Component:
hotspot
OS:
generic
Sub-Component:
gc
CPU:
generic
Priority:
P4
Resolution:
Fixed
Affected Versions:
hs17
Fixed Versions:
hs17 (b02)

Related Reports
Backport:
Backport:
Relates:

Sub Tasks

Description
On 09/22/09 08:17, Andrew John Hughes wrote:
> 2009/9/21  <###@###.###>:
>> Hi Andrew --
>>
>> I _think_ you should be able to just remove the
>> #ifdef's entirely, and it should work. (I don't know
>> what i was trying to achieve w/the #ifdef, but the
>> rationale was probably obsoleted during the long (several years!)
>> gestation period of that changeset before it was, ummm,
>> pushed.)
>>
>> Let me know if there are any issues.
>> -- ramki
>>
>> On 09/21/09 15:39, Andrew John Hughes wrote:
>>> This changeset:
>>>
>>> http://hg.openjdk.java.net/jdk7/jdk7/hotspot/rev/8b46c4d82093
>>>
>>> was promoted as part of b72 and fixes
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4957990.  It's nice
>>> to see a long-running bug being resolved, but unfortunately I've run
>>> into an issue with the fix.
>>>
>>> In markSweep.hpp and psParallelCompact.hpp, two new functions are
>>> introduced:
>>>
>>> + static void follow_mdo_weak_refs();
>>> + static void revisit_mdo(ParCompactionManager* cm, DataLayout* p);
>>>
>>> However, in markSweep.cpp and psParallelCompact.cpp, these are only
>>> implemented conditionally:
>>>
>>> +#if ( defined(COMPILER1) || defined(COMPILER2) )
>>> +void MarkSweep::revisit_mdo(DataLayout* p) {
>>> + _revisit_mdo_stack->push(p);
>>> +}
>>> +
>>> etc...
>>>
>>> If neither COMPILER1 nor COMPILER2 are defined (as is the case with
>>> building Gary Benson's zero port:
>>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2009-August/001957.html
>>> for example), then the build now fails:
>>>
>>> Linking launcher...
>>> /mnt/builder/icedtea/hotspot/outputdir/linux_zero_core/product/libjvm.so:
>>> undefined reference to
>>> `PSParallelCompact::revisit_mdo(ParCompactionManager*, DataLayout*)'
>>> /mnt/builder/icedtea/hotspot/outputdir/linux_zero_core/product/libjvm.so:
>>> undefined reference to `MarkSweep::revisit_mdo(DataLayout*)'
>>> /mnt/builder/icedtea/hotspot/outputdir/linux_zero_core/product/libjvm.so:
>>> undefined reference to `MarkSweep::follow_mdo_weak_refs()'
>>> /mnt/builder/icedtea/hotspot/outputdir/linux_zero_core/product/libjvm.so:
>>> undefined reference to `PSParallelCompact::follow_mdo_weak_refs()'
>>> collect2: ld returned 1 exit status
>>>
>>> The obvious fix that springs to mind is to add an #else block that
>>> implements these with ShouldNotCallThis() as in this webrev:
>>>
>>> http://cr.openjdk.java.net/~andrew/hotspot/webrev.01/
>>>
>>> but I'm not sure if that's the correct fix.
>>>
>>> Any ideas?
>>>
>>> Thanks,
>>
>
> Ah, that's good, I was worried about just having no implementation of
> part of the GC.
>
> With:
>
> http://cr.openjdk.java.net/~andrew/hotspot/webrev.02/hotspot.patch
>
> and turning off compressed oops, I've been able to build a working
> Zero-based HotSpot build with b72.
>
> Can you assign this a bug ID and put the webrev through jprt to get it applied?
>
> Thanks,

                                    

Comments
SUGGESTED FIX

--- old/src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.cpp	2009-09-22 15:56:47.322978476 +0100
+++ new/src/share/vm/gc_implementation/parallelScavenge/psParallelCompact.cpp	2009-09-22 15:56:47.204734235 +0100
@@ -2752,7 +2752,6 @@
   cm->revisit_klass_stack()->push(k);
 }
 
-#if ( defined(COMPILER1) || defined(COMPILER2) )
 void PSParallelCompact::revisit_mdo(ParCompactionManager* cm, DataLayout* p) {
   cm->revisit_mdo_stack()->push(p);
 }
@@ -2778,8 +2777,6 @@
     follow_stack(cm);
   }
 }
-#endif //  ( COMPILER1 || COMPILER2 )
-
 
 #ifdef VALIDATE_MARK_SWEEP
 
--- old/src/share/vm/gc_implementation/shared/markSweep.cpp	2009-09-22 15:56:48.341323100 +0100
+++ new/src/share/vm/gc_implementation/shared/markSweep.cpp	2009-09-22 15:56:48.245500132 +0100
@@ -73,7 +73,6 @@
   follow_stack();
 }
 
-#if ( defined(COMPILER1) || defined(COMPILER2) )
 void MarkSweep::revisit_mdo(DataLayout* p) {
   _revisit_mdo_stack->push(p);
 }
@@ -92,7 +91,6 @@
   }
   follow_stack();
 }
-#endif //  ( COMPILER1 || COMPILER2 )
 
 MarkSweep::FollowRootClosure  MarkSweep::follow_root_closure;
                                     
2009-09-22
EVALUATION

Should be fixed.
                                     
2009-09-22
EVALUATION

http://hg.openjdk.java.net/jdk7/hotspot-gc/hotspot/rev/c3c4a1d3801a
                                     
2009-09-23



Hardware and Software, Engineered to Work Together