[PATCH 0 of 3] Add support for dtrace compatible sdt probes on GNU/Linux

[serguei.spitsyn at oracle.com]

Keith,

I agree that it'd be nice to follow the rules.
But we also can do it in two stages:
  - first integration to keep close to the original fix that was already 
tested on Linux platform (as Mark tells)
  - separate refactoring to follow the platform separation rules (will 
need to file another CR)

Not sure I'll be able to work on this in the near future (a month) as I 
have to switch to the urgent security issue.
But it is still can be possible to do in a background with a low priority.

Other action items for this integration:
   1. Test that the HS DTrace are not broken on Solaris
   2. Find or setup a Linux machine with the systemTap
   3. Check that the fix is built Ok
   4. Test that the fix works on Linux
       Need a unit test for the fix (Mark, do you have any?)

Thanks,
Serguei


On 5/23/12 9:27 AM, Keith McGuigan wrote:
>
> Hi Mark -
>
> I'd prefer that it's done the "right" way (based on *my* definition of 
> "right", of course :) ), but I won't put up a fuss if whomever 
> shepherds this through agrees with you and wants to keep it in it's 
> current form.  I expect that will be Serguei, or someone else from the 
> serviceability team?  (Serguei?)
>
> -- 
> - Keith
>
> On 5/23/2012 12:19 PM, Mark Wielaard wrote:
>> On Wed, 2012-05-23 at 10:23 -0400, Keith McGuigan wrote:
>>> On 5/23/2012 9:56 AM, Mark Wielaard wrote:
>>>>>     It's (of course) just a style thing, but traditionally in hotspot
>>>>> we've wanted the os or arch specific code in os or arch specific
>>>>> directories, instead of littering the code with #ifdefs.  I know 
>>>>> the OSX
>>>>> stuff started violated this some, but I hope we're going to 
>>>>> resolve that
>>>>> rather than continue down that path.
>>>>
>>>> I wouldn't have mind if the OSX stuff was split out this way, but 
>>>> now I
>>>> don't have any example to follow here. What/How do you suggest it is
>>>> done?
>>>
>>> My first thought would be to create a dtrace_solaris.hpp file in
>>> src/os/solaris/vm and a dtrace_linux.hpp file in src/os/linux/vm (and
>>> maybe the same for bsd and windows?) for any OS-specific stuff, like 
>>> the
>>> macro definitions and maybe even that solaris workaround.  Leave
>>> anything common in the original file (like the default empty 
>>> definitions
>>> and such).  Then you include the specific-file based on the target OS,
>>> similar to what's done here:
>>> http://hg.openjdk.java.net/hsx/hotspot-main/hotspot/file/03d61caacd1e/src/share/vm/runtime/interfaceSupport.hpp 
>>>
>>>
>>> Yeah... it's a bit of a pain, and maybe in this case it's a little
>>> overkill.  But it's definitely nice to have a location for the
>>> OS-specific stuff and it keeps the code rather neat.
>>
>> So, do you require this change or not? I don't particularly like that
>> style (you loose the overview/a central place where all the magic macros
>> are defined), but I can certainly do it if you require it now. But then
>> I want to do it as additional patches on top of the current patches to
>> make sure they are separate. Since I will then have to touch code for at
>> least two architectures to which I don't have access (I also don't have
>> access to windows nor bsd, but I hope I don't have to touch those too).
>> So I want such changes to be testable separately by the architecture
>> maintainers.
>>
>> Thanks,
>>
>> Mark

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20120523/0f51fba2/attachment.html