David,
My feeling on this is that the Hotspot changes should go through
hotspot-runtime-dev, and that since they represent a fix to a recent regression
(in 1.7.0_04) they should probably go now without waiting for the test changes
in the jdk project. I'll start a new thread on hotspot-runtime-dev offering
this patch as a fix for 7172708.
Chris
On Jun 4, 2012, at 7:54 PM, David Holmes wrote:
> Hi Chris,
>
> Hotspot changes need to go via the hotspot-dev list (or hotspot-runtime-dev)
> to get pushed into hs24 (for JDK8) and then separately backported to hs23.2
> (for bulk integration into 7u).
>
> David
>
> On 4/06/2012 11:38 PM, Chris Dennis wrote:
>> Hi All,
>>
>> Like all simple issues this one has ballooned slightly... I've prepared a
>> patch and jtreg test for this bug, but have some questions, some process
>> related and some technical.
>>
>> The patch for the actual bug itself is against the hotspot sources. Does
>> this mean that these changes must be pushed through the hotspot-comp forest
>> first and then make it in to jdk7u-dev (and jdk8) in the next hotspot bulk
>> integration? If so I imagine I should submit this patch independently
>> through the hotspot-dev mailing list?
>>
>> The patch to increase test coverage to cover this issue is against the jdk
>> sources, however the issue here is more complicated. While modifying the
>> LimitDirectMemory.sh jtreg test to cover this issue I've run in to some
>> problems with the test - the code that is intended to test the parsing of
>> 'illegal' values was broken. I've 'fixed' this code in the patch to
>> approximate the current behavior of the tip of the jdk7u-dev forest, but the
>> test doesn't currently pass with these modifications (it fails for
>> -XX:MaxDirectMemorySize=-1). It's not clear to me to what extent the
>> details of the behavior for these illegal values is important: is the text
>> of the message important, is the current behavior with -1 a problem? I'm
>> also not sure whether these test changes should be part of a separate commit
>> under a different bug-id?
>>
>> As a further question (I seem to be full of questions today - sorry!) should
>> the jdk changes go through the jdk8 forest first before being merged in to
>> jdk7u-dev or are we okay to go in the other direction?
>>
>> Thanks,
>>
>> Chris
>>
>> P.S. This bug-id is not yet public so the bug synopsis in the subject may be
>> incorrect.
>>
>> ========================================
>>
>> # HG changeset patch
>> # User Chris Dennis<cden...@terracottatech.com>
>> # Date 1338815867 14400
>> # Node ID d5791a95b56c237de6baefd826bb6cbce01354eb
>> # Parent f08a3a0e60c32cb0e8350e72fdc54849759096a4
>> 7172708 : provide correct PRI?PTR definitions for 64-bit windows
>>
>> diff --git a/src/share/vm/utilities/globalDefinitions_visCPP.hpp
>> b/src/share/vm/utilities/globalDefinitions_visCPP.hpp
>> --- a/src/share/vm/utilities/globalDefinitions_visCPP.hpp
>> +++ b/src/share/vm/utilities/globalDefinitions_visCPP.hpp
>> @@ -220,9 +220,15 @@
>> #define PRIu64 "I64u"
>> #define PRIx64 "I64x"
>>
>> +#ifdef _LP64
>> +#define PRIdPTR "I64d"
>> +#define PRIuPTR "I64u"
>> +#define PRIxPTR "I64x"
>> +#else
>> #define PRIdPTR "d"
>> #define PRIuPTR "u"
>> #define PRIxPTR "x"
>> +#endif
>>
>> #define offset_of(klass,field) offsetof(klass,field)
>>
>> # HG changeset patch
>> # User Chris Dennis<cden...@terracottatech.com>
>> # Date 1338816442 14400
>> # Node ID b5f3b100e8597fa2d217b98dbfc9992080498ccd
>> # Parent 3335ab17d9264bb60aac68e789cf6cf95aedeceb
>> 7172708: Add test coverage to ensure large MaxDirectMemorySize values are
>> parsed correctly.
>> XXXXXXX: Fix LimitDirectMemory.sh test to properly test illegal
>> MaxDirectMemorySize values.
>>
>> diff --git a/test/java/nio/Buffer/LimitDirectMemory.java
>> b/test/java/nio/Buffer/LimitDirectMemory.java
>> --- a/test/java/nio/Buffer/LimitDirectMemory.java
>> +++ b/test/java/nio/Buffer/LimitDirectMemory.java
>> @@ -28,16 +28,25 @@
>> private static int K = 1024;
>>
>> public static void main(String [] args) throws Exception {
>> - if (args.length< 2)
>> + if (args.length< 3)
>> throw new RuntimeException();
>> - boolean throwp = parseThrow(args[0]);
>> - int size = parseSize(args[1]);
>> - int incr = (args.length> 2 ? parseSize(args[2]) : size);
>> + long cmdmax = parseSize(args[0]);
>> + boolean throwp = parseThrow(args[1]);
>> + int size = (int)parseSize(args[2]);
>> + int incr = (args.length> 3 ? (int)parseSize(args[3]) : size);
>>
>> Properties p = System.getProperties();
>> if (p.getProperty("sun.nio.MaxDirectMemorySize") != null)
>> throw new RuntimeException("sun.nio.MaxDirectMemorySize
>> defined");
>>
>> + if (p.getProperty("sun.arch.data.model").equals("32"))
>> + cmdmax = cmdmax& 0xffffffffL;
>> +
>> + long max = sun.misc.VM.maxDirectMemory();
>> + if (max != cmdmax)
>> + throw new RuntimeException("MaxDirectMemorySize does not match
>> jvm="
>> + + max + " cmd=" + cmdmax);
>> +
>> ByteBuffer [] b = new ByteBuffer[K];
>>
>> // Fill up most/all of the direct memory
>> @@ -68,22 +77,22 @@
>> throw new RuntimeException("Unrecognized expectation: " + s);
>> }
>>
>> - private static int parseSize(String size) throws Exception {
>> + private static long parseSize(String size) throws Exception {
>>
>> if (size.equals("DEFAULT"))
>> - return (int)Runtime.getRuntime().maxMemory();
>> + return Runtime.getRuntime().maxMemory();
>> if (size.equals("DEFAULT+1"))
>> - return (int)Runtime.getRuntime().maxMemory() + 1;
>> + return Runtime.getRuntime().maxMemory() + 1;
>> if (size.equals("DEFAULT+1M"))
>> - return (int)Runtime.getRuntime().maxMemory() + (1<< 20);
>> + return Runtime.getRuntime().maxMemory() + (1<< 20);
>> if (size.equals("DEFAULT-1"))
>> - return (int)Runtime.getRuntime().maxMemory() - 1;
>> + return Runtime.getRuntime().maxMemory() - 1;
>> if (size.equals("DEFAULT/2"))
>> - return (int)Runtime.getRuntime().maxMemory() / 2;
>> + return Runtime.getRuntime().maxMemory() / 2;
>>
>> int idx = 0, len = size.length();
>>
>> - int result = 1;
>> + long result = 1;
>> for (int i = 0; i< len; i++) {
>> if (Character.isDigit(size.charAt(i))) idx++;
>> else break;
>> @@ -92,7 +101,7 @@
>> if (idx == 0)
>> throw new RuntimeException("No digits detected: " + size);
>>
>> - result = Integer.parseInt(size.substring(0, idx));
>> + result = Long.parseLong(size.substring(0, idx));
>>
>> if (idx< len) {
>> for (int i = idx; i< len; i++) {
>> diff --git a/test/java/nio/Buffer/LimitDirectMemory.sh
>> b/test/java/nio/Buffer/LimitDirectMemory.sh
>> --- a/test/java/nio/Buffer/LimitDirectMemory.sh
>> +++ b/test/java/nio/Buffer/LimitDirectMemory.sh
>> @@ -47,10 +47,10 @@
>> echo "Testing: -XX:MaxDirectMemorySize=$* -cp ${TESTCLASSES} \
>> LimitDirectMemory true DEFAULT DEFAULT+1M"
>> ${TESTJAVA}/bin/java -XX:MaxDirectMemorySize=$* -cp ${TESTCLASSES} \
>> - LimitDirectMemory true DEFAULT DEFAULT+1M> ${TMP1} 2>&1
>> + LimitDirectMemory DEFAULT true DEFAULT DEFAULT+1M> ${TMP1} 2>&1
>> cat ${TMP1}
>> - cat ${TMP1} | grep -s "Unrecognized VM option: \'MaxDirectMemorySize="
>> - if [ $? -ne 0 ]
>> + cat ${TMP1} | grep -q "Could not create the Java Virtual Machine"
>> + if [ $? -eq 0 ]
>> then echo "--- failed as expected"
>> else
>> echo "--- failed"
>> @@ -58,29 +58,32 @@
>> fi
>> }
>>
>> -# $java LimitDirectMemory throwp fill_direct_memory size_per_buffer
>> +# $java LimitDirectMemory max_direct_memory throwp fill_direct_memory
>> size_per_buffer
>>
>> # Memory is properly limited using multiple buffers.
>> -runTest -XX:MaxDirectMemorySize=10 -cp ${TESTCLASSES} LimitDirectMemory
>> true 10 1
>> -runTest -XX:MaxDirectMemorySize=1k -cp ${TESTCLASSES} LimitDirectMemory
>> true 1k 100
>> -runTest -XX:MaxDirectMemorySize=10m -cp ${TESTCLASSES} LimitDirectMemory
>> true 10m 10m
>> +runTest -XX:MaxDirectMemorySize=10 -cp ${TESTCLASSES} LimitDirectMemory 10
>> true 10 1
>> +runTest -XX:MaxDirectMemorySize=1k -cp ${TESTCLASSES} LimitDirectMemory 1k
>> true 1k 100
>> +runTest -XX:MaxDirectMemorySize=10m -cp ${TESTCLASSES} LimitDirectMemory
>> 10m true 10m 10m
>>
>> # We can increase the amount of available memory.
>> runTest -XX:MaxDirectMemorySize=65M -cp ${TESTCLASSES} \
>> - LimitDirectMemory false 64M 65M
>> + LimitDirectMemory 65M false 64M 65M
>>
>> # Exactly the default amount of memory is available.
>> -runTest -cp ${TESTCLASSES} LimitDirectMemory false 10 1
>> -runTest -Xmx64m -cp ${TESTCLASSES} LimitDirectMemory false 0 DEFAULT
>> -runTest -Xmx64m -cp ${TESTCLASSES} LimitDirectMemory true 0 DEFAULT+1
>> +runTest -cp ${TESTCLASSES} LimitDirectMemory DEFAULT false 10 1
>> +runTest -Xmx64m -cp ${TESTCLASSES} LimitDirectMemory DEFAULT false 0 DEFAULT
>> +runTest -Xmx64m -cp ${TESTCLASSES} LimitDirectMemory DEFAULT true 0
>> DEFAULT+1
>>
>> # We should be able to eliminate direct memory allocation entirely.
>> -runTest -XX:MaxDirectMemorySize=0 -cp ${TESTCLASSES} LimitDirectMemory true
>> 0 1
>> +runTest -XX:MaxDirectMemorySize=0 -cp ${TESTCLASSES} LimitDirectMemory 0
>> true 0 1
>>
>> # Setting the system property should not work so we should be able to
>> allocate
>> # the default amount.
>> runTest -Dsun.nio.MaxDirectMemorySize=1K -Xmx64m -cp ${TESTCLASSES} \
>> - LimitDirectMemory false DEFAULT-1 DEFAULT/2
>> + LimitDirectMemory DEFAULT false DEFAULT-1 DEFAULT/2
>> +
>> +# On 64-bit JVMs we should be able to specify long sizes
>> +runTest -XX:MaxDirectMemorySize=5g -cp ${TESTCLASSES} LimitDirectMemory 5g
>> false 0 1
>>
>> # Various bad values fail to launch the VM.
>> launchFail foo
>>
