code review round 1 for Full Debug Symbols on MacOS X hotspot (7165611)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Oct 15 14:38:51 UTC 2013
On 10/15/13 12:07 AM, David Holmes wrote:
> On 15/10/2013 1:18 AM, Daniel D. Daugherty wrote:
>> Thanks for the re-review!
>>
>>
>> On 10/13/13 7:57 PM, David Holmes wrote:
>>> Hi Dan,
>>>
>>> Only further comment I have, and it may well be deferred for future
>>> work, is that we should be able to abstract away the actual
>>> "extension" used for the "debuginfo" file so that we don't need macosx
>>> conditionals as much eg one:
>>>
>>> if macosx
>>> DEBUGINFO_EXTENSION=dSYM
>>> else
>>> DEBUGINFO_EXTENSION=debuginfo
>>> endif
>>>
>>> then eg:
>>>
>>> JSIG_DEBUGINFO := $(strip $(wildcard
>>> $(HOTSPOT_DIST)/jre/lib$(OPENJDK_TARGET_CPU_LIBDIR)/libjsig.$DEBUGINFO_EXTENSION)
>>>
>>> \
>>> $(wildcard
>>> $(HOTSPOT_DIST)/jre/lib$(OPENJDK_TARGET_CPU_LIBDIR)/libjsig.diz) )
>>
>> The biggest worry that I have about something like DEBUGINFO_EXTENSION
>> is that on MacOS X the '.dSYM' objects are directories instead of files.
>> This requires that different cmd options be used in many/most/all of the
>> cases. More thought would be needed.
>
> I agree that the main commands wouldn't benefit from this, but things
> like the JSIG_DEBUGINFO definition would.
Possibly. It would be good if I got some cycles to look at FDS again
from a higher level with an eye toward overall simplification and
clarification. Instead, we're again trying to get some changes done
in time for another release deadline.
> I must admit that I'm somewhat surprised that .dSYM being a directory
> hasn't caused more problems.
Hopefully, I've caught all the right places. JPRT jobs and manual
builds (mine and Staffan's) indicate that things work. The proof
in the pudding will be when a promoted build actually contains a
proper "debuginfo" bundle from RE...
> Anyway no blockers here.
Thanks! And thanks for all of your help with and reviews of the
dreaded FDS stuff... Most folks run screaming in the other direction
from FDS changes...
Dan
>
> Cheers,
> David
>
>>
>>> Hmm second comment - I don't see a .m4 file change that corresponds to
>>> the DSYMUTIL configure change ??
>>
>> Yikes! I'll check into that shortly. Not sure what happened here.
>>
>> Dan
>>
>>
>>>
>>> Thanks,
>>> David
>>>
>>> On 12/10/2013 6:27 AM, Daniel D. Daugherty wrote:
>>>> Greetings,
>>>>
>>>> I'm ready for code review round 1 of the FDS on MacOS X hotspot
>>>> changes.
>>>> Below is the original code review round 0 invite (slightly edited for
>>>> clarity). Working on FDS is like pulling a thread on a sweater... so
>>>> there are four additional changed files along with more changes to
>>>> many of the files from round 0.
>>>>
>>>> Here is the code review round 1 JDK8/HSX-25 webrev URL:
>>>>
>>>> OpenJDK:
>>>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/1-jdk8/
>>>> Internal:
>>>> http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/1-jdk8/
>>>>
>>>>
>>>> Changes from code review round 0 to round 1:
>>>>
>>>> root repo:
>>>> - add configure support for DSYMUTIL macro
>>>>
>>>> hotspot repo:
>>>> - drop Minimal1 config support for FDS on MacOS X
>>>> - use DSYMUTIL macro instead of literal 'dsymutil'
>>>> - fix FASTDEBUG_CFLAGS typo and add USE_CLANG support
>>>> - STRIP is not used on MacOS X
>>>> - fix $(UNIVERSAL_COPY_LIST) rule
>>>> - refine install-dir function
>>>>
>>>> jdk repo:
>>>> - add prep-target-dir, install-import-dir, and
>>>> install-import-debuginfo
>>>> functions
>>>> - support importing .dSYM directories from hotspot
>>>> - create libjsig.dSYM symlink from VM specific directory to
>>>> the real location
>>>>
>>>> Files changed from code review round 0 to round 1:
>>>>
>>>> common/autoconf/generated-configure.sh
>>>> hotspot/make/Makefile
>>>> hotspot/make/bsd/makefiles/defs.make
>>>> hotspot/make/bsd/makefiles/dtrace.make
>>>> hotspot/make/bsd/makefiles/gcc.make
>>>> hotspot/make/bsd/makefiles/jsig.make
>>>> hotspot/make/bsd/makefiles/saproc.make
>>>> hotspot/make/bsd/makefiles/universal.gmk
>>>> hotspot/make/bsd/makefiles/vm.make
>>>> hotspot/make/defs.make
>>>>
>>>> New files changed in code review round 1:
>>>>
>>>> hotspot/make/bsd/makefiles/product.make
>>>> jdk/make/common/Defs.gmk
>>>> jdk/make/java/redist/Makefile
>>>> jdk/makefiles/Import.gmk
>>>>
>>>> Additional bugs filed based on code review comments, testing and
>>>> just plain looking at the Makefiles:
>>>>
>>>> 8026280 implement Full Debug Symbols on MacOS X jdk
>>>> 8026281 hotspot 'jdk6_or_earlier logic' for FDS needs to be
>>>> removed
>>>> 8026282 the '64' subdir logic in Makefiles might be removable
>>>> 8026283 literal 'lipo' cmd uses in HotSpot makefiles
>>>> 8026284 Minimal1 references/support should be removed from
>>>> BSD/MacOS X
>>>> 8026285 JPRT -buildenv ZIP_DEBUGINFO_FILES=0 no longer works
>>>>
>>>> As always, comments, questions and suggestions are welcome.
>>>>
>>>> Dan
>>>>
>>>> > Greetings,
>>>> >
>>>> > I have the initial support for Full Debug Symbols (FDS) on MacOS X
>>>> done
>>>> > and ready for review:
>>>> >
>>>> > 7165611 implement Full Debug Symbols on MacOS X hotspot
>>>> > https://bugs.openjdk.java.net/browse/JDK-7165611
>>>> >
>>>> > Here is the JDK8/HSX-25 webrev URL:
>>>> >
>>>> > OpenJDK:
>>>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/0-jdk8/
>>>> > Internal:
>>>> http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/0-jdk8/
>>>>
>>>> >
>>>> > This webrev includes changes for the follow repos:
>>>> >
>>>> > jdk8
>>>> > jdk8/hotspot
>>>> > jdk8/jdk
>>>> >
>>>> > Once these changes are approved, I'm planning to push them to
>>>> > RT_Baseline. From there, they can follow the normal path to
>>>> > Main_Baseline and eventually JDK8.
>>>> >
>>>> > This work enables FDS on MacOS X for the 'hotspot' repo; the
>>>> changes in
>>>> > the other repos are necessary to support importing the .diz files
>>>> from
>>>> > the MacOS X 'hotspot' build into the forest build. I also fixed
>>>> a few
>>>> > FDS related errors in the magic incantations for the new build.
>>>> This is
>>>> > mostly a port from Linux -> MacOS X/BSD with the dtrace changes
>>>> ported
>>>> > from Solaris. In other words, this is Frankenstein's monster...
>>>> >
>>>> > Thanks to Staffan Larsen for providing an initial set of changes
>>>> > which I morphed into what you see here.
>>>> >
>>>> > Testing:
>>>> > - JPRT HSX build and test on all platforms; verification of .diz
>>>> > files in the MacOS X JPRT bundles
>>>> > - JPRT JDK8 forest build and test on all platforms; verification of
>>>> > .diz files in the MacOS X JPRT bundles
>>>> > Note: In previous FDS changesets, I also did a standalone 'jdk'
>>>> > repo build and test, but that no longer seems to work.
>>>> >
>>>> > As always, comments, questions and suggestions are welcome.
>>>> >
>>>> > Dan
>>
More information about the build-dev
mailing list