RFC: JamVM - Taste the New Flavour!

Xerxes Rånby xerxes at zafena.se
Mon Feb 21 17:48:38 PST 2011


On 2011-02-21 22:40, Robert Lougher wrote:
> Hi,
>
> On 21 February 2011 21:22, Dr Andrew John Hughes<ahughes at redhat.com>  wrote:
>> On 21:20 Sat 19 Feb     , Xerxes Ranby wrote:
>>> Greetings!
>>>
>>> "JamVM's got a shiny new Git repository, which contains the port to the
>>> OpenJDK class-library."
>>> http://draenog.blogspot.com/2011/02/openjdkjamvm-git-repository.html
>>>
>>> The attached patch adds Robert Lougher's new JamVM flavour to Icedtea6!
>>>
>> See comments below on the --enable-jamvm option.
>> I'll leave comments on addvm to doko.

I will split this patch in two commits, one for each option, to ease 
review, In this mail i have added the patch for the --enable-jamvm option.

I will add --with-additional-vms=jamvm in a follow up patch.

>>
>> I think this should be ok to go in before the 1.10 branch as the changes
>> to the build are mainly ancillary and don't affect the main options.
>>
>
> Xerxes, can you take a new snapshot from the berlios git tree?  A
> couple of people have already tested the port.  One of them (Ludovic
> Orban) reported a crash with an application which stressed the GC
> (failure after compaction).  I tracked this down over the weekend, and
> the port is now running stably.
>
> Rob.
>
> P.S.  Thanks for doing the integration!
>

Superb!
I have grabbed the updated stable snapshot. Thank you for the fix!

>>> Index: icedtea6/Makefile.am
>>> ===================================================================
>>> --- icedtea6.orig/Makefile.am 2011-02-19 02:26:05.000000000 +0100
>>> +++ icedtea6/Makefile.am      2011-02-19 20:58:34.000000000 +0100
>>> @@ -11,6 +11,18 @@
>>>   CACAO_URL = $(CACAO_BASE_URL)/$(CACAO_VERSION).tar.gz
>>>   CACAO_SRC_ZIP = cacao-$(CACAO_VERSION).tar.gz
>>>
>>> +# The jamvm-e9b0f82b9c9cfe5e5550f5e77de53ec2fba603a3.tar.gz got fetched from
>>> +# http://git.berlios.de/cgi-bin/cgit.cgi/jamvm/snapshot/jamvm-e9b0f82b9c9cfe5e5550f5e77de53ec2fba603a3.tar.gz
>>> +# Unfortunally the generated .tar.gz by the berlios cgit snapshot function
>>> +# keeps changing sha256sum. I have hosted a snapshot on labb.zafena.se
>>> +# while we wait for the final JamVM 1.6.0 release.
>>> +JAMVM_VERSION = e9b0f82b9c9cfe5e5550f5e77de53ec2fba603a3
>>> +JAMVM_SHA256SUM = 24cf67795970d7e63d297a13b370ae8c3b141c35a22c3fd96137b5e62815a607
>>> +JAMVM_BASE_URL = http://labb.zafena.se/jamvm
>>> +JAMVM_URL = $(JAMVM_BASE_URL)/jamvm-$(JAMVM_VERSION).tar.gz
>>> +JAMVM_SRC_ZIP = jamvm-$(JAMVM_VERSION).tar.gz
>>> +JAMVM_IMPORT_PATH = \$(abs_top_builddir)/jamvm/install/hotspot
>>> +
>>
>> I'd prefer JAMVM_IMPORT_PATH was with the other paths in the Makefile.am rather than here.

JAMVM_IMPORT_PATH Moved down to the other paths.

>> Is there a reason for the leading backslash?

No reason... removed.

>>>
>>> +stamps/download-jamvm.stamp:
>>> +if BUILD_JAMVM
>>> +     if ! echo "$(JAMVM_SHA256SUM)  $(JAMVM_SRC_ZIP)" \
>>> +      | $(SHA256SUM) --check ; \
>>> +     then \
>>> +      if [ $(JAMVM_SRC_ZIP) ] ; \
>>> +      then \
>>> +        mv $(JAMVM_SRC_ZIP) $(JAMVM_SRC_ZIP).old ; \
>>> +      fi ; \
>>> +      $(WGET) $(JAMVM_URL) -O $(JAMVM_SRC_ZIP); \
>>> +      if ! echo "$(JAMVM_SHA256SUM)  $(JAMVM_SRC_ZIP)" \
>>> +       | $(SHA256SUM) --check ; \
>>> +      then echo "ERROR: Bad download of JamVM zip"; false; \
>>> +      fi; \
>>> +     fi
>>> +endif
>>> +     mkdir -p stamps
>>> +     touch $@
>>
>> We need an option equivalent to --with-cacao-src-zip.  I'd suggest
>> adding this in a follow up patch.  As with the drop tarballs, we
>> should verify the checksum for pre-provided tarballs as well as
>> download ones.  It should be just a matter of symlinking the tarball
>> here if an alternate one is specified.
>>

Noted, I will add --with-cacao-src-zip in another follow up patch.

 >> @@ -1018,6 +1080,7 @@
 >>   	fi
 >>
 >>   	icedtea_version="$(PACKAGE_VERSION)$(ICEDTEA_REV)" ; \
 >> +        if ! test "x$(WITH_JAMVM)" = "xno"; then \
 >
 > I don't think this will work, but it's hard to tell in patch form.
 > It looks like WITH_CACAO is getting nested inside WITH_JAMVM which
 > means WITH_CACAO will only be checked if WITH_JAMVM is set.

Thank you for spotting this. Yes this part was clearly wrong as can be 
verified from the -version output of my first patch.

The goal of this part are to make sure that when using CACAO or JamVM 
as the main VM then the Runtime environment should be named IcedTea6 
Runtime Environment instead of OpenJDK Runtime Environment.

 >
 > Can you include the whole rule in a reply?
 >

New whole new rule now looks like this:

         if test "x$(WITH_CACAO)" = "xyes" || \
            test "x$(ENABLE_JAMVM)" = "xyes"; then \
           echo "JDK_DERIVATIVE_NAME=$${icedtea_version}" \
             >>openjdk/jdk/make/common/shared/Defs.gmk ; \
           echo "PRODUCT_NAME=$(ICEDTEA_NAME)" \
             >>openjdk/jdk/make/common/shared/Defs.gmk ; \
         else \
           echo "JDK_DERIVATIVE_NAME=$(ICEDTEA_NAME) $${icedtea_version}" \
             >>openjdk/jdk/make/common/shared/Defs.gmk ; \
         fi ;


And now java -version work as expected when using --enable-jamvm
xranby at hp:~/icedtea6-jamvm-build$ ./openjdk.build/j2sdk-image/bin/java 
-version
java version "1.6.0_21"
IcedTea6 Runtime Environment (1.10pre+r55566ffe7026) (Ubuntu build 
1.6.0_21-b21)
JamVM (build 1.6.0-devel, inline-threaded interpreter)

>>> @@ -1694,6 +1771,61 @@
>>>        fi
>>>        rm -f stamps/rewrite-rhino.stamp
>>>
>>> +# JAMVM

Renamed to JamVM

>>> +
>>> +stamps/jamvm.stamp: $(OPENJDK_TREE) stamps/rt.stamp
>>> +if BUILD_JAMVM
>>> +     cd jamvm/jamvm&&  \
>>> +     ./autogen.sh --with-java-runtime-library=openjdk \
>>> +       --prefix=$(abs_top_builddir)/jamvm/install ; \
>>> +     $(MAKE) ; \
>>> +     $(MAKE) install
>>> +     mkdir -p $(abs_top_builddir)/jamvm/install/hotspot/jre/lib/$(INSTALL_ARCH_DIR)/server
>>> +     cp $(abs_top_builddir)/jamvm/install/lib/libjvm.so $(abs_top_builddir)/jamvm/install/hotspot/jre/lib/$(INSTALL_ARCH_DIR)/server
>>> +     ln -s server $(abs_top_builddir)/jamvm/install/hotspot/jre/lib/$(INSTALL_ARCH_DIR)/client
>>> +     touch $(abs_top_builddir)/jamvm/install/hotspot/jre/lib/$(INSTALL_ARCH_DIR)/server/Xusage.txt
>>> +     touch $(abs_top_builddir)/jamvm/install/hotspot/jre/lib/$(INSTALL_ARCH_DIR)/libjsig.so
>>> +endif

Added missing
	mkdir -p stamps
	touch $@

>>> Index: icedtea6/configure.ac
>>> ===================================================================
>>> --- icedtea6.orig/configure.ac        2011-02-19 02:26:05.000000000 +0100
>>> +++ icedtea6/configure.ac     2011-02-19 02:26:38.000000000 +0100
>>> @@ -175,6 +175,9 @@
>>>   AC_CHECK_WITH_CACAO_HOME
>>>   AC_CHECK_WITH_CACAO_SRC_ZIP
>>>   AC_CHECK_WITH_CACAO_SRC_DIR
>>> +
>>> +AC_CHECK_ENABLE_JAMVM
>>> +
>>
>> Please use IT as the prefix rather than AC.  We are not the autoconf project :-)

Fixed, now we are the IcedTea project :-)!
Thank you Andrew for renaming the rest whole lot!

>>
>>>   DISABLE_OPTIMIZATIONS
>>>   SET_SHARK_BUILD
>>>   ENABLE_ZERO_BUILD
>>> Index: icedtea6/acinclude.m4
>>> ===================================================================
>>> --- icedtea6.orig/acinclude.m4        2011-02-19 02:26:05.000000000 +0100
>>> +++ icedtea6/acinclude.m4     2011-02-19 03:23:52.000000000 +0100
>>> @@ -746,11 +746,15 @@
>>>           sparc*-*-*) ;;
>>>           x86_64-*-*) ;;
>>>           *)
>>> -          if test "x${WITH_CACAO}" != xno; then
>>> +          if test "x${WITH_JAMVM}" != xno; then
>>>               use_zero=no
>>>             else
>>> -            use_zero=yes
>>> -          fi
>>> +            if test "x${WITH_CACAO}" != xno; then
>>> +              use_zero=no
>>> +            else
>>> +              use_zero=yes
>>> +            fi
>>> +       fi
>>
>> This would be clearer if you just added JAMVM as a new option
>> rather than changing the CACAO one.

I have now changed this part to

@@ -746,7 +746,8 @@
          sparc*-*-*) ;;
          x86_64-*-*) ;;
          *)
-          if test "x${WITH_CACAO}" != xno; then
+          if test "x${WITH_CACAO}" != xno || \
+	     test "x${ENABLE_JAMVM}" = xyes; then
              use_zero=no
            else
              use_zero=yes


hope it makes things clearer.

>>
>>>             ;;
>>>         esac
>>>       fi
>>> @@ -820,6 +824,23 @@
>>>     AM_CONDITIONAL(SHARK_BUILD, test "x${use_shark}" = xyes)
>>>   ])
>>>
>>> +AC_DEFUN([AC_CHECK_ENABLE_JAMVM],
>>> +[
>>> +  AC_MSG_CHECKING(whether to use JamVM as VM)
>>> +  AC_ARG_ENABLE([jamvm],
>>> +           [AS_HELP_STRING(--enable-jamvm,use JamVM as VM [[default=no]])],
>>> +  [
>>> +    WITH_JAMVM="${enableval}"
>>> +  ],
>>> +  [
>>> +    WITH_JAMVM=no
>>> +  ])
>>> +
>>> +  AC_MSG_RESULT(${WITH_JAMVM})
>>> +  AM_CONDITIONAL(WITH_JAMVM, test x"${WITH_JAMVM}" = "xyes")
>>> +  AC_SUBST(WITH_JAMVM)
>>> +])
>>> +
>>
>> Again, should be IT_CHECK_ENABLE_JAMVM.
>> Why are we using 'WITH_JAMVM' rather than 'ENABLE_JAMVM'?
When CACAO got integrated in icedtea 1.3 then the configure option got 
named --with-cacao this is  why the code are littered with WITH_CACAO...
I simply copied the naming to make the code look familiar with its CACAO 
heritage.

Noted, I will consider changing the WITH_CACAO to ENABLE_CACAO in a 
future CACAO naming cleanup patch.

   'with' implies it's possible to
>> take some option but we aren't using AC_ARG_WITH but AC_ARG_ENABLE.

I have switched name to ENABLE_JAMVM in the new patch.

>> Also if it is only yes/no a lot of the above would be simpler if we just tested for 'yes'.
>> It seems to assume some other value is possible.

Updated to only check for yes.


>>
>> --
>> Andrew :)
>>
>> Free Java Software Engineer
>> Red Hat, Inc. (http://www.redhat.com)
>>
>> Support Free Java!
>> Contribute to GNU Classpath and IcedTea
>> http://www.gnu.org/software/classpath
>> http://icedtea.classpath.org
>> PGP Key: F5862A37 (https://keys.indymedia.org/)
>> Fingerprint = EA30 D855 D50F 90CD F54D  0698 0713 C3ED F586 2A37
>>

Thank you Rob and Andrew for the first review round.

New patch attached that implements the --enable-jamvm option

Ok to push?

Cheers
Xerxes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: enable-jamvm.22feb.patch
Type: text/x-patch
Size: 12526 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110222/d637ded0/enable-jamvm.22feb.patch 


More information about the distro-pkg-dev mailing list