RFR(M) : 7104565 : trim jprt build targets

[David Chase]

David,

The "why" on some of this is that I was starting with an existing mostly-done-but-aged changeset, so I don't always know why, and there could be mistakes.  I'm perfectly happy to put those tests back in (and I will unless someone else objects).

jvmgminimal1_warn needs to go in, I'd call that an oversight.

And as I understand the plan of this patch, the DEBUG and FASTDEBUG definitions are gone; however ASSERT is defined in either case, and I think only in those cases:

find . -name * -exec egrep -e DASSERT /dev/null {} ;
./bsd/makefiles/adlc.make:CXXFLAGS += -DASSERT
./bsd/makefiles/debug.make:SYSDEFS += -DASSERT
./bsd/makefiles/fastdebug.make:SYSDEFS += -DASSERT
./linux/makefiles/adlc.make:CXXFLAGS += -DASSERT
./linux/makefiles/debug.make:SYSDEFS += -DASSERT
./linux/makefiles/fastdebug.make:SYSDEFS += -DASSERT
./solaris/makefiles/adlc.make:CXXFLAGS += -DASSERT
./solaris/makefiles/debug.make:SYSDEFS += -DASSERT
./solaris/makefiles/fastdebug.make:SYSDEFS += -DASSERT -DCHECK_UNHANDLED_OOPS

One of these things is not like the other, how about that?
Should -DCHECK_UNHANDLED_OOPS be removed from solaris.fastdebug, or added to the others (or is Solaris.fastdebug somehow special)?

I don't have much of an opinion on generateJvmOffsets.cpp and stackMapFrame.hpp -- I don't know enough of the history, I don't know how important fastdebug performance is for those two files, and I'm perfectly willing to defer to someone else's judgment here, but I don't see a conclusion yet.  I do see that adding in DEBUG-sensitivity would tweak the makefiles a little again, but that seems like a minor issue compared to the wins of sanitizing both jprt and jvmg-vs--debug and cleaning out old profiled builds.  Do you and Vladimir settle this with rock-paper-scissors, or can we get someone else to weigh in?

On 2013-04-09, at 10:33 PM, David Holmes <david.holmes at oracle.com> wrote:

> Hi David,
> 
> A few comments/concerns:
> 
> jprt.properties:
> 
> So the JPRT GC "_default" tests were redundant? Don't they test that ergonomics based selection works okay versus forced GC selection?
> 
> ---
> 
> makefile related:
> 
> The bug says that the debug targets are unused but a fix for that is not what has been implemented. Please update the CR to reflect that debug lives and jvmg/profiled are what are now gone.
> 
> Why is there no jvmgminimal1_warn target?
> 
> ---
> 
> As discussed in the CR I have concerns about replacing all ifdef DEBUG/FASTDEBUG with ASSERT as they do not mean the same thing. In particular:
> 
> I don't agree with the removal of the FASTDEBUG define as used here:
> 
> ./os/solaris/dtrace/generateJvmOffsets.cpp:#if defined(DEBUG) || defined(FASTDEBUG)
> ./os/solaris/dtrace/generateJvmOffsets.cpp:#endif /* defined(DEBUG) || defined(FASTDEBUG) */
> ./os/bsd/dtrace/generateJvmOffsets.cpp:#if defined(DEBUG) || defined(FASTDEBUG)
> ./os/bsd/dtrace/generateJvmOffsets.cpp:#endif /* defined(DEBUG) || defined(FASTDEBUG) */
> 
> the code this guards couldn't care less about ASSERT, it cares about a symbol issue that is related to doing debug/fastdebug builds. (I wonder if this is still an issue though - what different build settings do we use for a debug build ??)
> 
> Ditto for src/share/vm/classfile/stackMapFrame.hpp - that code is not assert related but debugging related. Further you have now added that code to fastdebug builds - which seems wrong (or at least potentially impacts the "fast" part!). Also as Vladimir has indicated in the bug report this code would now be getting tested for the first time! - so potential bug tail there.
> 
> The changes in src/share/vm/shark/llvmHeaders.hpp need to be confirmed by the shark folk (if they haven't already). (And of course depend on -DDEBUG being eradicated.)
> 
> Thanks,
> David
> -----
> 
> 
> On 10/04/2013 9:01 AM, David Chase wrote:
>> 
>> On 2013-04-09, at 5:26 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>> One note: in frame_sparc.cpp remove 'jvmg' in the line you modified since we will not build jvmg.
>> 
>> done
>> 
>>> And question: did you use mercurial commands to remove and rename jvm.make and debug.make? We need to use hg commands to preserve changes history for jvmg.make (it has more changesets then old debug.make).
>>> 
>>> I just want to confirm because sometimes webrev may not show correctly what was done to sources. I mean you need to do:
>>> 
>>> hg remove debug.make
>>> hg rename jvmg.make debug.make
>>> 
>>> and then edit debug.make
>> 
>> To be sure:
>> 
>> 10009  hg revert make/{solaris,bsd,linux}/makefiles/jvmg.make
>> 10010  zip -9 ../safemakes.zip make/{solaris,bsd,linux}/makefiles/debug.make
>> 10011  hg revert make/{solaris,bsd,linux}/makefiles/debug.make
>> 10012  hg remove make/{solaris,bsd,linux}/makefiles/debug.make
>> 10013  for i in solaris bsd linux ; do hg rename make/$i/makefiles/{jvmg,debug}.make; done
>> 10014  unzip -l ../safemakes.zip
>> 10015  unzip ../safemakes.zip
>> 
>> And the new-and-improved diffs:
>> 
>> http://cr.openjdk.java.net/~drchase/7104565/webrev.06-new/
>> 
>>