code review round 0 for Full Debug Symbols on MacOS X hotspot (7165611)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Thu Oct 10 13:03:06 UTC 2013
Thanks for confirmation.
Dan
On 10/10/13 3:09 AM, David Holmes wrote:
> I'm fine with not adding untestable minimal VM support.
>
> Thanks,
> David
>
> On 10/10/2013 1:03 AM, Daniel D. Daugherty wrote:
>> Replies also inline...
>>
>>
>> On 10/9/13 6:02 AM, David Holmes wrote:
>>> inline ...
>>>
>>> On 9/10/2013 8:59 AM, Daniel D. Daugherty wrote:
>>>> On 10/1/13 8:52 PM, David Holmes wrote:
>>>>> - 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.
>>>>
>>>> Since you pointed out in a later email that minimal1 isn't
>>>> supported on MacOS X, I'm going to drop those changes. I'm
>>>> going to leave the Client VM support since there are folks
>>>> out in OpenJDK trying to get the Client VM working on MacOS X.
>>>>
>>>> It does look like someone added minimal1 support for other
>>>> non-Linux platforms, but I'm not planning to clean that up.
>>>
>>> Yes I did that when adding minimal support. Seemed better than leaving
>>> someone to have to rediscover what needed to be implemented if/when
>>> minimal support was expanded.
>>
>> Thanks for the history. Are you still OK if I leave out the
>> FDS Minimal1 support?
>>
>>
>>> Someday I hope to remove all the duplicated EXPORT_* stuff and
>>> generate it based on the requested JDK_VARIANT_* values. And do it in
>>> a platform indpendent way - once! :)
>>
>> Well it can be mostly platform independent, but there will be
>> minor differences in what is exported by the different platforms.
>>
>>
>>>
>>>>> 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.
>>>>
>>>> Agreed that we (Oracle) currently don't have plans to drop HSX-25 into
>>>> JDK6 or OpenJDK6, but I don't want to rule out future insanity.
>>>
>>> I do! We need to cut ties with historical baggage.
>>
>> Good sentiment, but not for this changeset. I'll file a bug to track
>> your idea of "best to clean all that up at one time".
>>
>>
>>>
>>>>> - 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 ?
>>>>
>>>> BUILT_COPY_FILES includes both files and directories so everything
>>>> should get copied across. I've added some commments to the rule
>>>> header and reorganized the logic a bit to hopefully be more clear.
>>>
>>> Doesn't this assume that the directory will appear before the files
>>> within it? Is that guaranteed?
>>
>> The way find works is that it lists the directory prior to listing
>> the files within the directory. However, even if find didn't, the
>> containing directory would be created via line 69 above. The one
>> non-obvious feature of lines 72-74 is that an empty directory named
>> in the BUILD_COPY_FILES list would get 'copied' to the destination.
>>
>> Please check out the latest version when I get it out for review.
>>
>>
>>> Which leads me to ask why we have cp -R for copying files?
>>
>> MacOS X strongly discourages use of 'cp -r' and recommends 'cp -R'
>> instead because 'cp -R' properly copies non-directory and non-file
>> file system objects, e.g., symlinks. So if BUILD_COPY_FILES happens
>> to contain a symlink, then the symlink is copied to the destination.
>>
>>
>>>
>>>>> - 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?
>>>>
>>>> It just seemed wrong to define something that should never be used
>>>> on non-Solaris platforms. My control build of the entire forest
>>>> didn't have an issue with this change so I'm planning to keep it.
>>>
>>> Ok.
>>>
>>> Thanks,
>>> David
>>
>> Again, thanks for the thorough reviews.
>>
>> Dan
>>
>>
>>>
>>>> Dan
>>>>
>>>>
>>>>
>>>>>
>>>>> 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
>>>>
>>
More information about the build-dev
mailing list