JDK-7102470 : RFE: USDT1 versus USDT2 ifdefing should be revisited
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: svc
  • Affected Version: hs23
  • Priority: P5
  • Status: Closed
  • Resolution: Not an Issue
  • OS: os_x
  • CPU: generic
  • Submitted: 2011-10-18
  • Updated: 2023-07-27
  • Resolved: 2023-07-27
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdResolved
Related Reports
Relates :  
Relates :  
Description
Comment from Tom R:

> On the issue of the dtrace changes, couldn't we minimize later
> disruption by moving the USDT1/USDT2 distinction into a header file?
> For instance, instead of doing this:
> 
> +#ifndef USDT2
> 
>    HS_DTRACE_PROBE1(hotspot, thread__sleep__end,0);
> 
> +#else /* USDT2 */
> +  HOTSPOT_THREAD_SLEEP_END(
> +                           0);
> +#endif /* USDT2 */
> 
> Couldn't it just have
> 
>   HOTSPOT_THREAD_SLEEP_END(0);
> 
> with this ifdef in a header somewhere?
> 
> #ifndef USDT2
> #define  HOTSPOT_THREAD_SLEEP_END(arg) \
>    HS_DTRACE_PROBE1(hotspot, thread__sleep__end, (arg))
> #endif /* USDT2 */
> 
> It minimizes the ugly until we resolve the real issue.  I guess
> the PROBE_DECL stuff will have to be somewhere else too.  Anyway,
> I'm fine with pushing it as is but I wanted to at least mention this.


My reply to Tom R:

> I'll include this in the new bug as a way to phase in the
> changes from USDT1 -> USDT2. I don't want to make that
> change now since it will make it more difficult to keep
> in sync with the MacOS X port project.


A reply from Paul H on Tom R's thread:

> Do we really need to keep in sync with the old macosx-port project?
> Afaic, we're just using that code as a starting point for this project.
> 
> I second Tom's suggestion to move the USDT1/2 distinction into
> a header file.  Doing so would make this changeset a whole lot smaller. 


My reply to Paul H:

On 10/12/11 4:00 PM, Daniel D. Daugherty wrote:
> On 10/12/11 3:19 PM, Paul Hohensee wrote:
>> Do we really need to keep in sync with the old macosx-port project?
>
> Yes. That's one of the ways that I'm keeping this port project sane.
> As far as I know, the bsd-port/hotspot and macosx-port/hotspot repos
> have not yet been retired. If I have to resync with either of those
> repos again before they are retired, then I would like that to go
> smoothly.
>
>
>> Afaic, we're just using that code as a starting point for this project.
>
> 'Afaic' or 'Afaik'? (care or know?)
>
> Yes, we're using that code as a starting point for this project, but
> I don't yet have testing up and running on MacOS X so I don't want to
> make non-trivial MacOS X changes blind.
>
>
>> I second Tom's suggestion to move the USDT1/2 distinction into
>> a header file.  Doing so would make this changeset a whole lot smaller.
>
> Not going to happen in this changeset. I will file another bug to do
> more work on USDT1/2 stuff, but I don't yet have a way to test making
> a change like that on MacOS X. 


Paul's reply to me:

On 10/12/11 4:07 PM, Paul Hohensee wrote:
> On 10/12/11 6:00 PM, Daniel D. Daugherty wrote:
>> On 10/12/11 3:19 PM, Paul Hohensee wrote:
>>> Do we really need to keep in sync with the old macosx-port project?
>>
>> Yes. That's one of the ways that I'm keeping this port project sane.
>> As far as I know, the bsd-port/hotspot and macosx-port/hotspot repos
>> have not yet been retired. If I have to resync with either of those
>> repos again before they are retired, then I would like that to go
>> smoothly.
>
> Ok.  Maybe someone can tell me when they'll be retired, or if they're
> relevant to a jdk7 port.  I see what I can find out.
>
>>
>>
>>> Afaic, we're just using that code as a starting point for this project.
>>
>> 'Afaic' or 'Afaik'? (care or know?)
>
> "concerned". :)
>
>>
>> Yes, we're using that code as a starting point for this project, but
>> I don't yet have testing up and running on MacOS X so I don't want to
>> make non-trivial MacOS X changes blind.
>>
>>
>>> I second Tom's suggestion to move the USDT1/2 distinction into
>>> a header file.  Doing so would make this changeset a whole lot smaller.
>>
>> Not going to happen in this changeset. I will file another bug to do
>> more work on USDT1/2 stuff, but I don't yet have a way to test making
>> a change like that on MacOS X.
>
> Reluctantly, Ok.
Comment from David H:

> src/share/vm/prims/jni.cpp
> 
> The USDT2 ifdef changes are truly ugly especially in this file.
> Is there no way to hide USDT1 vs USDT2 at a lower level so that
> the top-level probe points are not affected? I know this is
> flagged for "future work" but history shows such cleans ups
> rarely if ever happen - and this current code really is pretty
> awful to look at. 


My reply to David H:

> Yes, the #ifdef'ing makes the current code awful. I don't think
> anyone will dispute that observation.
> 
> The better solution would be for Solaris to migrate to USDT2.
> I'm discussing that with Keith M. However, I don't want
> to take much of his time right now because he is hip deep in
> another alligator swamp at the moment.
> 
> The best I can offer at the moment is filing a new bug to track
> the future work. Yes, I know that clean up type work rarely gets
> done, but this merge with the MacOS X port project needs to get
> into HSX-23 as soon as possible. Broken functionality in the
> other platforms is a stopper, but ugly is not a stopper.
Comment from Vladimir K:

> Why new Dtrace implementation for MacOS X use 2 lines where it is not
> needed (a lot of places)? For example:
> 
> +#else /* USDT2 */
> +  HOTSPOT_GC_END(
> +);
> +#endif /* USDT2 */
> 
> Is this what you called "does not follow HotSpot style guidelines very consistently"?


A reply to Vladimir's comment from Paul H:

> I'm ok with fixing that (the style issue) later, as long as it does
> get fixed.  Use of the macro preprocessor is the devil, unless we
> absolutely, positively have no choice. :) 


And my reply to Paul H's comment:

> I can promise that a bug will get filed to track the whole issue
> of migrating from USDT1 -> USDT2. Included in that bug will be
> a note about the style issues. That's the best I can say at the
> moment. 


Roger H's reply to Vladimir K's comment?

> I did the USDT2 implementation with some mechanical tools and a lot
> of by-hand followup.  My intension was to leave the structure of the
> USDT1 macro calls in place.  Most of the USDT2 changes are a smallish
> delta on the USDT1 macro calls.  If we were to have macro-in-a-macro
> capabilities, we could implement this small difference with another macro.
> 
> Fortunately for sanity's sake, macros-in-macros don't work well in the C
> preprocessor, and I had to duplicate the entire macro call.  If Oracle
> moves the Solaris implementation from USDT1 to USDT2, then by all means
> the USDT2 code should be cleaned up.  On the other hand, if the USDT1
> implementation is to live on, any formatting changes should enhance the
> ability to make consistent modifications to both USDT1 and USDT2.


Closing comment from Vladimir K:

> Thank you, Dan
> 
> I agree for this round we should leave new Dtrace amd Mikefiles code
> as it is. Thank you, Alexander and Roger for explaining these changes.


Closing comment from Paul H:

> Ditto for me.


Reply to Roger H's comment from Keith M:

> FWIW, I believe the build platform for jdk7 is Solaris 10, which does
> support USDT2, as far as I know.  So if we can convert to use USDT2
> all the time and drop the USDT1 code, that would be good.
The MacOS X Port project used the following changeset for the initial
push from macosx-port/hotspot -> HSX-23:

    http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/436b4a3231bf

    7098194: integrate macosx-port changes

and that changeset includes a newer Dtrace implementation along side
the original Dtrace implementation. The code is #ifdef'ed in the
following forms:

        #ifndef USDT2
<older Dtrace implementation is in this block>
<some non-Dtrace code (macros) that call the older Dtrace>
<implementation are also in this block>
        #else /* USDT2 */
<new Dtrace implementation for MacOS X in this block>
        #endif /* USDT2 */

        #ifndef USDT2
<older Dtrace implementation is in this block>
<no #else because the newer Dtrace doesn't need/have the equivalent>
        #endif /* ! USDT2 */

The purpose of this RFE is to request that the #ifdef'ing be
revisited to address comments from the code reviewers of 7098194.

Comments
The code was deleted long time ago. Not an issue anymore
27-07-2023

Code cleanup issue. Doesn't need to be done for 10.
05-04-2017

SUGGESTED FIX Possible ways forward: - switch to USDT2 on all platforms and drop USDT1 (Keith) - move the #ifdef'ing down into a header file (David H, Tom R)
18-10-2011