On 3/10/2013 1:32 AM, Daniel D. Daugherty wrote:
David,
Thanks for the thorough review (as always)!
It will take me a while to process all the comments, but another
reply will be forthcoming.
Don't bother with the minimal VM build attempt, you won't even get
started. The files don't exist for BSD.
David
Dan
On 10/1/13 8:52 PM, David Holmes wrote:
Hi Dan,
Overall thumbs up. A couple of minor issues that need fixing. A few
meta-comments (I hate seeing all this stuff duplicated again and again.
David
-----
- common/autoconf/hotspot-spec.gmk.in
Seems a good simplification.
----
- common/autoconf/jdk-options.m4
No comment.
---
- common/makefiles/NativeCompilation.gmk
So JDK .diz support is phase 2? :)
The Windows changes here seem okay given that on windows a .debuginfo
file should never be in the target list.
---
- hotspot/make/Makefile
+ $(EXPORT_CLIENT_DIR)/%.dSYM: $(MINIMAL1_BUILD_DIR)/%.dSYM
EXPORT_CLIENT_DIR should be EXPORT_MINIMAL_DIR.
For fun you can try building minimal on OSX, but I don't know how far
you will get:
--with-jvm-variants=client,server,minimal1
I'll point out obvious issues with minimal VM support anyway.
---
- hotspot/make/bsd/Makefile
No comment.
- hotspot/make/bsd/makefiles/buildtree.make
No comment.
- make/bsd/makefiles/defs.make
Note that the whole jdk6_or_earlier logic is defunct as this will
never be used for jdk6 or earlier. But best to clean all that up at
the one time.
Sadly I never found a satisfactory solution to the multiplicity and
verbosity of the FDS messages, so OSX builds will now be afflicted by
them too.
328 else
329 EXPORT_LIST += $(EXPORT_MINIMAL_DIR)/libjvm.debuginfo
330 endif
This pre-existing minimal VM code needs to be modified to reference
the dSYM directory on OSX as per the client/server cases.
---
- hotspot/make/bsd/makefiles/dtrace.make
Note related to your changes but I don't think any of the "64"
directory stuff has any meaning outside of Solaris.
102 dsymutil $@
I think dsymutil should be assigned to a variable in the platform
defs.make as with other tools.
Would be nice if objcopy/dsymutil could be hidden behind a single
SYM_TOOL variables as well so there wasn't so much duplication of the
process. You could have a single set of instructions to invoke
SYM_TOOL, STRIP and ZIP (strip would be skipped of course because
STRIP_POLICY would never be set on osx). Just wishful thinking ...
---
- hotspot/make/bsd/makefiles/gcc.make
+ FASTDEBUG_CFLAGS += $(DEBUG_CFLAGS/$(BUILDARCH))
should be
+ FASTDEBUG_CFLAGS += $(FASTDEBUG_CFLAGS/$(BUILDARCH))
Don't we need the USE_CLANG variations here as for linux?
---
- hotspot/make/bsd/makefiles/jsig.make
- hotspot/make/bsd/makefiles/saproc.make
Similar comments to dtrace.make
---
- make/bsd/makefiles/universal.gmk
I did not understand the additional logic here:
63 # Copy built non-universal binaries in place
64 $(UNIVERSAL_COPY_LIST):
65 BUILT_COPY_FILES="`find
$(EXPORT_JRE_LIB_DIR)/{i386,amd64}/$(subst $(EXPORT_JRE_LIB_DIR)/,,$@)
2>/dev/null`"; \
66 if [ -n "$${BUILT_COPY_FILES}" ]; then \
67 for i in $${BUILT_COPY_FILES}; do \
68 if [ -f $${i} ]; then \
69 $(MKDIR) -p $(shell dirname $@); \
70 $(CP) -R $${i} $@; \
71 fi; \
72 if [ -d $${i} ]; then \
73 $(MKDIR) -p $@; \
74 fi; \
75 done; \
76 fi
until I realized that foo.dSYM is a directory not a file! Even so
don't you want to copy the contents of the directory across ?
BTW: UNIVERSAL_COPY_LIST doesn't handle minimal VM.
---
- make/bsd/makefiles/vm.make
Similar comments to dtrace.make ref dsymutil.
---
- hotspot/make/defs.make
No comment.
---
- jdk/make/common/Defs-macosx.gmk
No comment
- jdk/makefiles/Tools.gmk
Not sure about this one. Logically is seems correct but up to now this
has been defined for everything without it seeming to cause a problem.
So why do we need to change it and what impact will it have?
David
-----
On 21/09/2013 1:36 PM, Daniel D. Daugherty wrote:
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
jdk8/jdk/make/closed
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
