Review request: White box testing API for HotSpot

Mon Feb 20 07:56:16 PST 2012

Not quite there yet.

The copyright year in the new files is now "2011, 2012" when it should
just be "2012".  The new code didn't get pushed in 2011. :)

Paul

On 2/20/12 10:43 AM, Mikael Gerdin wrote:
> Hi Paul,
>
> On 2012-02-17 23:54, Paul Hohensee wrote:
>> Thanks for doing this, it's something I've been wanting to see in 
>> Hotspot
>> for quite awhile. :)
>>
>> Looks good. You'll need a GC reviewer for the whitebox.cpp method 
>> content.
>
> Right. Some of the GC functions were more or less examples of what 
> could be done and were not very useful besides that. I removed the 
> most obscure ones at least but I'll keep some core functions.
>
>>
>> A few small things:
>>
>> In globals.hpp, add a blank line ending in '\' before the declaration of
>> WhiteBoxAPI.
>
> Done
>
>>
>> The copyright year in the all the new files should be 2012. Same with
>> all the old files except make/Makefile, nativeLookup.cpp,
>> make/bsd/makefiles/defs.make,
>> arguments.cpp, globals.hpp and vmError.cpp, which already have 2012.
>
> I think I updated all of them.
>
>>
>> What's the rationale for the directory path ('
>> ||src/share/tools/whitebox/src/classes/
>> sun/hotspot') used to get to WhiteBox.java? Seems a bit long.
>
> It made more sense when there was a native library part living in 
> src/share/tools/whitebox/src/native
> I changed it to
> src/share/tools/whitebox
>
>
>>
>> In whitebox.hpp, use SHARE_VM_PRIMS_WHITEBOX_HPP instead of
>> SHARE_VM_PRIMS_WHITEBOX_H.
>
> Done
>
> New webrev at: http://cr.openjdk.java.net/~mgerdin/wbapi/webrev.7/
>
> Thanks for looking at this.
>
> /Mikael
>
>>
>> Thanks,
>>
>> Paul
>>
>> On 2/17/12 10:12 AM, Mikael Gerdin wrote:
>>> I gave up on getting this into HS23 and was busy doing other stuff for
>>> HS23.
>>> Anyway, there were some issues with the last patch so here's a rebased
>>> webrev that actually compiles :)
>>>
>>> http://cr.openjdk.java.net/~mgerdin/wbapi/webrev.5/
>>>
>>> /Mikael
>>>
>>> On 2012-02-17 12:21, Nils Loodin wrote:
>>>> Sooo, what happened with this?
>>>>
>>>> I think that testing this framework would be an excellent fit for
>>>> using in testing our diagnostic framework.
>>>> But it kind of depends on this being actually checked in :)
>>>>
>>>> /Nisse
>>>>
>>>> On Jan 17, 2012, at 16:34 , Mikael Gerdin wrote:
>>>>
>>>>> David,
>>>>>
>>>>> On 2012-01-13 00:00, David Holmes wrote:
>>>>>> Hi Mikael,
>>>>>>
>>>>>> On 13/01/2012 2:20 AM, Mikael Gerdin wrote:
>>>>>>>> wbapi.java: normal Java naming style is to use camel-case for 
>>>>>>>> class
>>>>>>>> names. Though as WB is itself an acronym I'd be okay with WBApi.
>>>>>>>> In fact
>>>>>>>> I'd be happy with anything other than initial lower-case :)
>>>>>>>
>>>>>>> Many of our existing tests have lower-case names so I guess I 
>>>>>>> thought
>>>>>>> that was some sort of convention, it does not really matter to me.
>>>>>>
>>>>>> I think those tests must have been written by C programers ;-)
>>>>>>
>>>>>>> WBApi it is then.
>>>>>>
>>>>>> Thanks.There is a slight typo in that the file is WBapi.java not
>>>>>> WBApi.java
>>>>>
>>>>> Fixed.
>>>>>
>>>>> I also re-ran JPRT to verify that it still builds on all platforms
>>>>> and found that the size of a region in G1 had changed to size_t, so
>>>>> I added a cast to jint (region sizes of>2G seems to be unreasonable).
>>>>>
>>>>> I also tried with Jim Melvin's patch to run OS X and verified that
>>>>> "wbapitest" works on OS X as well.
>>>>>
>>>>> /Mikael
>>>>>
>>>>>>
>>>>>> David
>>>>>> -----
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> test/Makefile: does wbapitest need to be added to the phoney list?
>>>>>>>
>>>>>>> Yes, fixed.
>>>>>>>
>>>>>>> New webrev at:
>>>>>>> http://cr.openjdk.java.net/~mgerdin/wbapi/webrev.3/
>>>>>>> Incremental at:
>>>>>>> http://cr.openjdk.java.net/~mgerdin/wbapi/webrev.2-to-3/webrev/
>>>>>>>
>>>>>>> /Mikael
>>>>>>>
>>>>>>>>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> David
>>>>>>>> -----
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/01/2012 5:27 AM, Mikael Gerdin wrote:
>>>>>>>>> Hi all
>>>>>>>>>
>>>>>>>>> Back from vacations now with an updated version of the webrev
>>>>>>>>> based on
>>>>>>>>> the feedback received in this thread.
>>>>>>>>> Changes include:
>>>>>>>>> * removed install target from makefiles
>>>>>>>>> * renamed flag form EnableWhiteBoxAPI to remove redundant Enable
>>>>>>>>> * installs wb.jar into jre/lib and made -XX:+WhiteBoxAPI add
>>>>>>>>> wb.jar to
>>>>>>>>> the boot class path from inside the VM.
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~mgerdin/wbapi/webrev.2/
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> Mikael Gerdin
>>>>>>>>>
>>>>>>>>> On 2011-11-29 17:04, Mikael Gerdin wrote:
>>>>>>>>>> Hi
>>>>>>>>>>
>>>>>>>>>> I've been working on a white box testing API for HotSpot in
>>>>>>>>>> order to
>>>>>>>>>> allow for improved precision in vm testing.
>>>>>>>>>>
>>>>>>>>>> The basic idea is to open up the possibility for tests written
>>>>>>>>>> in Java
>>>>>>>>>> to call native methods which query or poke the vm in some way.
>>>>>>>>>>
>>>>>>>>>> The API is accessible by using the class sun/hotspot/WhiteBox
>>>>>>>>>> which is
>>>>>>>>>> not intended to be available in public builds.
>>>>>>>>>> In order to allow the WhiteBox class access to the VM the
>>>>>>>>>> registerNatives function is linked to 
>>>>>>>>>> JVM_RegisterWhiteBoxMethods.
>>>>>>>>>> That
>>>>>>>>>> function then links all the implementation functions using
>>>>>>>>>> normal JNI
>>>>>>>>>> RegisterNatives.
>>>>>>>>>>
>>>>>>>>>> The API is not meant to be used by end users for any intent or
>>>>>>>>>> purpose
>>>>>>>>>> and as such it is both guarded by 
>>>>>>>>>> "-XX:+UnlockDiagnosticVMOptions
>>>>>>>>>> -XX:+EnableWhiteboxAPI" and the fact that the class files will
>>>>>>>>>> not be
>>>>>>>>>> present in an end user build of a JDK.
>>>>>>>>>> If the VM crashes after this API has been accessed a note 
>>>>>>>>>> will be
>>>>>>>>>> written in the hs_err file to signal that the API has been used.
>>>>>>>>>>
>>>>>>>>>> Webrev:
>>>>>>>>>> http://cr.openjdk.java.net/~stefank/mgerdin/wbapi.0/webrev/
>>>>>>>>>> (thanks to stefank for hosting my webrev :)
>>>>>>>>>>
>>>>>>>>>> CR:
>>>>>>>>>> I'll file a CR tomorrow.
>>>>>>>>>>
>>>>>>>>>> Change comments:
>>>>>>>>>>
>>>>>>>>>> make/jprt.properties
>>>>>>>>>>
>>>>>>>>>> Add a test target to make sure that the API is available on all
>>>>>>>>>> supported platforms
>>>>>>>>>>
>>>>>>>>>> make/**
>>>>>>>>>>
>>>>>>>>>> Makefile changes to build the class sun/hotspot/WhiteBox, put
>>>>>>>>>> it in a
>>>>>>>>>> JAR file and copy it to the jre/lib/endorsed directory in the
>>>>>>>>>> export
>>>>>>>>>> targets.
>>>>>>>>>> The BSD makefile changes are not tested since I don't have
>>>>>>>>>> access to
>>>>>>>>>> any
>>>>>>>>>> BSD/OSX machine to test them on.
>>>>>>>>>>
>>>>>>>>>> src/share/vm/prims/nativeLookup.cpp
>>>>>>>>>>
>>>>>>>>>> Special-case the method sun/hotspot/WhiteBox/registerNatives and
>>>>>>>>>> link it
>>>>>>>>>> to JVM_RegisterWhiteBoxMethods
>>>>>>>>>>
>>>>>>>>>> src/share/vm/prims/whitebox.*
>>>>>>>>>>
>>>>>>>>>> The implementation of the white box API. The actual API
>>>>>>>>>> functions are
>>>>>>>>>> only examples of what we want to be able to do using the API.
>>>>>>>>>>
>>>>>>>>>> src/share/vm/runtime/globals.hpp
>>>>>>>>>>
>>>>>>>>>> Add the command line flag
>>>>>>>>>>
>>>>>>>>>> src/share/vm/utilities/vmError.cpp
>>>>>>>>>>
>>>>>>>>>> Print a message in hs_err files when white box API has been 
>>>>>>>>>> used.
>>>>>>>>>>
>>>>>>>>>> test/Makefile
>>>>>>>>>>
>>>>>>>>>> Add a makefile test target for the white box API test
>>>>>>>>>>
>>>>>>>>>> test/sanity/wbapi.java
>>>>>>>>>>
>>>>>>>>>> JTreg test to ensure that the API works.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks
>>>>>>>>>> /Mikael Gerdin
>>>>>>>>>
>>>>