Request for Review: Conditionalize source so that functionality can be easily included or excluded
Coleen Phillimore
coleen.phillimore at oracle.com
Tue Oct 9 07:12:27 PDT 2012
>>
>> 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?)
>
> Not that I know of. I'll do that.
Okay, this is fine. Please file a bug and send me the number.
Thanks,
Coleen
>
>>
>>> 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.)
>
> I'll get rid of the unnecessary conditional.
>
>>
>>> 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.
>
> Thanks for the review!
>
> joe
>
>>> 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
>>>>
More information about the hotspot-dev
mailing list