Request for review 7170638: Use DTRACE_PROBE[N] in JNI Set and SetStatic Field.

[David Holmes]

Coleen,

On 4/10/2012 7:52 AM, Coleen Phillimore wrote:
>
> The logic for setting up OPENJDK is different but there was some code to
> set up HOTSPOT_VM_DISTRO based on the presence or absence of certain
> closed files. Anyway, I think this code is correct now. It sets OPENJDK
> if HOTSPOT_VM_DISTRO is OpenJDK. I can build Hotspot on Linux both ways
> now.

Sorry but that is the wrong fix. If anything OPENJDK should be 
controlling which distro file gets included. The logic for that in 
buildtree.make is:

ifndef HOTSPOT_VM_DISTRO
   ifeq ($(call if-has-altsrc,$(HS_COMMON_SRC)/,true,false),true)
     include $(GAMMADIR)/make/hotspot_distro
   else
     include $(GAMMADIR)/make/openjdk_distro
   endif
endif

So simply overriding HOTSPOT_VM_DISTRO would lose this OPENJDK=true 
setting - and that file is the wrong place to set that variable.

To remedy this in hotspot, for a hotspot-only build, we need to check 
for something that indicates this is not an OPENJDK build. The presence 
of the closed repos is generally what is used for that. But placing such 
a check correctly is extremely awkward (something compounded by the fact 
that present hotspot make/defs.make has a couple of bugs in ordering 
that the minimal VM changes will fix). So right now I can't tell you 
exactly how to fix this in a more suitable way.

David
-----

>
> open webrev at http://cr.openjdk.java.net/~coleenp/7170638_4/
> bug link at http://bugs.sun.com/view_bug.do?bug_id=7170638
>
> I also filed a bug 8000408 for Oracle JVM to support this in the future.
>
> Please review, again, and thank you for your patience!
>
> Coleen
>
> On 10/3/2012 10:38 AM, Mark Wielaard wrote:
>> On Wed, 2012-10-03 at 23:00 +1000, David Holmes wrote:
>>> To be clear, when hotspot is built as part of a full build then it
>>> should be being passed the OPENJDK variable. But I don't think it is,
>>> which is a bug in the top-level make logic. It has been a harmless bug
>>> to-date
>> Yeah, it looks like it might have been the intention to pass it down
>> through COMMON_BUILD_ARGUMENTS?
>>
>>> To allow for the old build system, and for standalone builds within that
>>> system, I think the hotspot/make/defs.make would have to duplicate the
>>> logic that is in jdk/make/common/Defs.gmk.
>> I looked at that, but the build makefile setups are pretty different.
>> The simplest seems to be to at least add it to the
>> COMMON_BUILD_ARGUMENTS at the top-level like so:
>>
>> --- openjdk/make/Defs-internal.gmk.orig 2012-10-03 16:08:00.239767294
>> +0200
>> +++ openjdk/make/Defs-internal.gmk 2012-10-03 16:07:50.508629201 +0200
>> @@ -325,6 +325,10 @@
>> PREVIOUS_MICRO_VERSION=$(PREVIOUS_MICRO_VERSION) \
>> STATIC_CXX=$(STATIC_CXX)
>>
>> +ifdef OPENJDK
>> + COMMON_BUILD_ARGUMENTS += OPENJDK=$(OPENJDK)
>> +endif
>> +
>> ifdef ARCH_DATA_MODEL
>> COMMON_BUILD_ARGUMENTS += ARCH_DATA_MODEL=$(ARCH_DATA_MODEL)
>> endif
>>
>> That works for my setup and should cover most default builds of openjdk
>> starting from the top-level. Does that look OK to you?
>>
>> Next we have to figure out how to get the default for OPENJDK set to
>> true in a plain hotspot dir. But the logic seems to be somewhat
>> different from what the jdk dir does. We might be able to use the
>> openjdk/hostpot/make/openjdk_distro Makefile snippet. Since that is
>> executed when the the hotspot build detects that it doesn't have
>> "closed" components. But I am not entirely sure how to satisfy the
>> restriction listed there:
>> # This file format must remain compatible with both
>> # GNU Makefile and Microsoft nmake formats.
>> Since I don't know anything about nmake.
>>
>> Cheers,
>>
>> Mark