Review request: White box testing API for HotSpot

Wed Feb 22 00:44:36 PST 2012

Forgot to reply-all..

On 2012-02-20 16:56, Paul Hohensee wrote:
> 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. :)
>
> make/jprt.properties still needs a copyright year of 2012 instead of 2011.

Ok,
new webrev at
http://cr.openjdk.java.net/~mgerdin/wbapi/webrev.8/

Thanks
/Mikael

>
> 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
>>>>>>>>>>
>>>>>