RFR: 8054823: Add size_t as a valid VM flag type

Staffan Larsen staffan.larsen at oracle.com
Tue Aug 12 13:11:14 UTC 2014


On 12 aug 2014, at 15:07, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:

> On 12/08/14 14:58, Staffan Larsen wrote:
>> On 12 aug 2014, at 14:42, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>> 
>>> On 12/08/14 14:25, Staffan Larsen wrote:
>>>> About the new AttachSetGetFlag test:
>>>> 
>>>> - To get the pid of the newly launched process use the new Process.getPid() API instead. This is also available cross platform and you can remove the only non-windows restriction.
>>>> 
>>>> - To avoid the annoying and unstable Thread.sleep(5000) let the started program print that it is ready and wait for that string in the output. This also avoids the need for -XX:+PauseAtStartup.
>>> I just copied the code from the other attach test in the same directory test/serviceability/attach/AttachWithStalePidFile.java, but I can update my test if you want to.
>> Yes, please.
> 
> Here's the version with the two suggestions above:
> http://cr.openjdk.java.net/~stefank/8054823/webrev.02.delta
> http://cr.openjdk.java.net/~stefank/8054823/webrev.02

Thanks you! Looks good. I think you can remove -XX:+UnlockDiagnosticVMOptions.

>> 
>>>> - To quit the started program use Process.destroyForcibly() in addition to having the process end by itself after a long timeout.
>>> The target process is destroyed, so the test "only" takes ~18 seconds with a product build, of which 3 x 5 seconds are the sleep you mention above.
>> It should be possible to run it in < 1 sec :-)
> 
> Almost: "elapsed time (seconds): 1.231" =)

Yeah!

> 
> thanks,
> StefanK
> 
>> 
>> /Staffan
>> 
>>> thanks,
>>> StefanK
>>>> Thanks,
>>>> /Staffan
>>>> 
>>>> On 11 aug 2014, at 19:46, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>>>> 
>>>>> On 2014-08-11 18:35, Vladimir Kozlov wrote:
>>>>>> Stefan,
>>>>>> 
>>>>>> Seems fine to me,
>>>>> Thanks for reviewing!
>>>>> 
>>>>>> except new tests don't have correct @bug.
>>>>>> 
>>>>>> 27  * The test helsp verifying that size_t flags can be set/read.
>>>>>>               ^ typo
>>>>>> 
>>>>>> SizeTTest.java - remove @author
>>>>> Too much copy-n-paste. I've updated the patch and did some additional cleanups to the jtreg annotations:
>>>>> http://cr.openjdk.java.net/~stefank/8054823/webrev.01.delta/
>>>>> http://cr.openjdk.java.net/~stefank/8054823/webrev.01/
>>>>> 
>>>>> thanks,
>>>>> StefanK
>>>>> 
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>> 
>>>>>> On 8/11/14 6:48 AM, Stefan Karlsson wrote:
>>>>>>> Hi all,
>>>>>>> 
>>>>>>> I propose that we add 'size_t' as a valid VM flag type. This will help with type safety, since we won't have to cast
>>>>>>> back and forth between the two types.
>>>>>>> 
>>>>>>> This patch adds the framework, but only changes one of the flags. I choose to change one of our experimental flags, so
>>>>>>> that I could write tests for this change.
>>>>>>> 
>>>>>>> Please, review this patch:
>>>>>>>  http://cr.openjdk.java.net/~stefank/8054823/webrev.00/
>>>>>>>  https://bugs.openjdk.java.net/browse/JDK-8054823
>>>>>>> 
>>>>>>> thanks,
>>>>>>> StefanK



More information about the hotspot-dev mailing list