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.