Request for Review: Conditionalize source so that functionality can be easily included or excluded

[David Holmes]

Hi Coleen,

If I may reply in Joe's stead ...

On 9/10/2012 10:36 AM, Coleen Phillimore wrote:
>
> Hi Joe,
>
> Did you take out the Windows "kernel" build with this change? If so,
> there are still code which refers to kernel in
> http://cr.openjdk.java.net/~jprovino/7189254/webrev.10/make/Makefile.html which
> should be removed.

"kernel" has not been completely eradicated but it is no longer built at 
all. Full eradication is a later clean up. (Joe: do we have a CR for 
that yet?)

> In make/Makefile, the COMMON_VM_*_TARGETS is what caused the kernel to
> be built with jprt. This was pretty useful so that people didn't
> inadvertently break the kernel build. I think you should add minimal1 to
> this list instead now so that the same doesn't happen with minimal vm.

The minimal VM is not intended to always be built, but will be, for 
example, as part of building SE Embedded, hence JPRT will be testing the 
minimal VM build works.

Going forward, in the new build system which VMs get built is determined 
by the JVM_VARIANT_* variables, which in turn are set at configure time. 
So the whole notion of "common" VM targets will be going away soon. 
(There are default settings for these variables for non-configure based 
builds.)

> In
> http://cr.openjdk.java.net/~jprovino/7189254/webrev.10/src/share/vm/code/nmethod.cpp.udiff.html
>
>
> Do you not need the conditional compilation because
> JvmtiExport::has_redefined_a_class() is included and always returns false?

Good catch - the conditional compilation is not needed there. (The same 
function is unguarded elsewhere.)

> If kernel is completely removed, there's some code you can remove in
> systemDictionary.cpp as well. I think that's the last of the kernel code.

See above - kernel not completely eradicated.

Thanks,
David
-----

> Rest looks good.
> Coleen
>
> On 10/5/2012 10:59 AM, Joe Provino wrote:
>> The latest and hopefully final webrev is here:
>>
>> http://cr.openjdk.java.net/~jprovino/7189254/webrev.10
>> <http://cr.openjdk.java.net/%7Ejprovino/7189254/webrev.10/>
>>
>> The changes are to make/{linux,bsd}/gcc.make to ensure that
>> CFLAGS and OPT_CFLAGS behave the way they did before any changes.
>>
>> joe
>>