code review request for Full Debug Symbols Revamp (7102323, 7136506)
Daniel D. Daugherty
daniel.daugherty at oracle.com
Fri Mar 23 20:10:47 UTC 2012
On 3/23/12 1:50 PM, Andrew Hughes wrote:
> ----- Original Message -----
>> On 3/23/12 12:08 PM, Andrew Hughes wrote:
>>> ----- Original Message -----
>>>> ----- Original Message -----
>>>>> Greetings,
>>>>>
>>>>> I've backported the FDS changes to JDK7u6. Here's the webrevs:
>>>>>
>>>>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/0-7u6-root/
>>>>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/0-7u6-jdk/
>>>>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7102323-webrev/0-7u6-hotspot/
>>>>>
>>>>> The JDK7u6 and JDK8 root repo patch files:
>>>>> - identical except for timestamps
>>>>> - no content differencebetween JDK7u6 and JDK8
>>>>>
>>>>> The JDK7u6 and JDK8 jdk repo patch files:
>>>>> - identical except for timestamps, some line numbers,
>>>>> and some context diff anchors
>>>>> - no content difference between JDK7u6 and JDK8
>>>>>
>>>>> The JDK7u6 and HSX-24 hotspot repo patch files:
>>>>> - are identical except for timestamps, some line numbers,
>>>>> some context diff anchors, some copyright updates and
>>>>> the indent fix that Serguei reported in the JDK8 review
>>>>> - no content difference between JDK7u6 and JDK8
>>>>> - copyright updates and whitespace updates are not content
>>>>> (IMHO)
>>>>>
>>>>> Thanks, in advance, for any sanity check reviews.
>>>>>
>>>>> Dan
>>>>>
>>>> Doesn't this need to go to jdk7u-dev at openjdk.java.net?
>> Why? These changes were reviewed by the appropriate OpenJDK
>> aliases and that seems to be what is required.
>>
> http://openjdk.java.net/projects/jdk7u/groundrules.html
>
> Rule 3
>
> Changes submitted for a JDK 7 Update forest MUST go through code review, and MUST be approved by the maintainer for that forest. The maintainer of a forest MAY delegate that authority, allowing for approvals to happen in a more finely granular fashion - per repository, for example.
The changes have gone through code review and the maintainer has
approved the changes.
> Rule 4
>
> Maintainer approvals for public JDK 7 Update forests MUST take place on the jdk7u-dev at openjdk.java.net mailing list. Code reviews SHOULD take place on that list - if they take place somewhere else, as part of the approval request a URL for the public code review MUST be provided.
The maintainer approval took place on the right alias. I identified
the aliases, but I didn't provide a URL. Not something that I've ever
done before so I have no idea where to dig up such a URL.
>>> Also, AFAICS, this only just went into 8-tl:
>>> http://hg.openjdk.java.net/jdk8/tl/jdk/rev/e7f813f2ea86
>>> I think it should have some time to soak there before going into
>>> 7u.
>> Again, why? These are build changes. They either work or they don't.
> And if they don't work, putting the same broken fix into multiple repositories creates more work in fixing it and
> breaks the build for more people.
The combined set of changes goes through a minimum of three control
builds before I push any of these changesets. First control build
is the default, second control build is with ENABLE_FULL_DEBUG_SYMBOLS=0
and the third control build is CREATE_DEBUGINFO_BUNDLES=false.
There have also been various test jobs for the HotSpot changes standalone,
the JDK changes standalone and HotSpot and JDK changes together (without
the rest of the repos in a control build).
I have done so many test jobs through various inter JPRT systems that
folks are getting quite annoyed.
> Also, just because something does build doesn't mean a change hasn't subtly changed something.
> We're already having issues with the original addition of this stripping, as GNU/Linux distros have their own methods
> of handling debuginfo and stripping so don't want this being done by the JDK build.
If you launch your builds with ENABLE_FULL_DEBUG_SYMBOLS=0, then you
won't get any of the Full Debug Symbols changes that I've made. However,
the stripping has been done by the JDK long before I made the original
FDS changes back in Sept 2011. I simply made the stripping policy
configurable.
> Also, we've traditionally had a period of allowing changes to soak from when OpenJDK7->OpenJDK6 backports were done, and
> I think this was helpful.
Maybe, but I don't think it makes a difference in this case.
> AFAICS, this change is not even completely in tl at the moment (just the jdk& root parts are).
And you would be right. Because of the nature of this change,
the pushes have to be staged in the right order. The closed
install repos changes are already in for JDK8-B33 and for
JDK7u6-B04. The root and jdk repo changes are in JDK8 T&L and
will be integrated in either JDK8-B33 or JDK8-B34. The root
and jdk repo changes are ready for JDK7u6-B04, but I need to
coordinate with Lana for either JDK7u6-B04 or JDK7u6-B05.
The HotSpot changes are ready for JDK7u6-B04/HSX-23.2-B01 but
I don't think that repo exists yet. I'm almost done resyncing
the HotSpot changes with JDK8/RT_Baseline in preparation for
JDK8-B33/HSX-24-B06 or JDK8-B34/HSX-24-B07. Some of the build
infra changes hit RT_Baseline after I last resync and I need
to adjust to those changes.
This means I'm going to launch another three JPRT control build
jobs and again other folks on my team will "love" me. Especially
since it's a Friday and we're trying to snapshot HSX-24-B05 and
HSX-23-B19 (I think)...
> So I doubt it's had much testing.
And you would be very, very wrong.
Dan
>> Dan
>>
>>
>>
>>
>>>>> On 3/16/12 1:58 PM, Daniel D. Daugherty wrote:
>>>>>> Greetings,
>>>>>>
>>>>>> I need code reviews for some Makefile and packaging changes.
>>>>>> Wait, come back! They're not that scary...
>>>>>>
>>>>>> These are Full Debug Symbols changes... so maybe they are that
>>>>>> scary...
>>>>>>
>>>>>> These changes have gone through two rounds of internal review.
>>>>>>
>>>>>> The following bugs are being used to revamp the OpenJDK side of
>>>>>> the
>>>>>> Full Debug Symbols (FDS) implementation:
>>>>>>
>>>>>> 7102323 4/4 RFE: enable Full Debug Symbols Phase 1 on
>>>>>> Solaris
>>>>>> 7136506 3/4 FDS: rework jdk repo Full Debug Symbols support
>>>>>>
>>>>>> FDS Revamp Summary
>>>>>>
>>>>>> The build infrastructure that supports the Full Debug
>>>>>> Symbols
>>>>>> (FDS)
>>>>>> project is being revamped to reduce the default on-disk
>>>>>> footprint
>>>>>> along with other improvements. FDS info will have to be
>>>>>> unzip'ed
>>>>>> before being usable in the default build config, but the
>>>>>> zip'ed
>>>>>> FDS
>>>>>> info occupies about 25% of the disk space as the original
>>>>>> FDS
>>>>>> info.
>>>>>>
>>>>>> Change summary for the group of fixes:
>>>>>> - ENABLE_FULL_DEBUG_SYMBOLS build flag controls the Full
>>>>>> Debug
>>>>>> Symbols feature; enabled by default
>>>>>> (ENABLE_FULL_DEBUG_SYMBOLS=1)
>>>>>> - ZIP_DEBUGINFO_FILES build flag controls the zip'ing of
>>>>>> "debug
>>>>>> info"
>>>>>> during the build; enabled by default
>>>>>> (ZIP_DEBUGINFO_FILES=1).
>>>>>> - FDS is enabled by default for Linux X86/X64, Solaris
>>>>>> SPARC/SPARC-V9,
>>>>>> Solaris X86, and Windows X86/X64.
>>>>>> - HSX developer builds will put debug info into .diz files
>>>>>> that
>>>>>> are
>>>>>> co-located with the built object, e.g., there will be a
>>>>>> libjvm.diz
>>>>>> file right next to libjvm.so.
>>>>>> - HSX JPRT jobs will also contain .diz files co-located
>>>>>> with
>>>>>> the
>>>>>> built
>>>>>> objects
>>>>>> - RE promoted bits will include new debuginfo.zip bundles
>>>>>> that
>>>>>> contain
>>>>>> all the .debuginfo, .diz, .map and/or .pdb files
>>>>>> generated
>>>>>> by
>>>>>> the
>>>>>> various repos that make up the RE promotion.
>>>>>>
>>>>>> Notes: FDS is not enabled on Solaris X64 due to a bug in
>>>>>> gobjcopy.
>>>>>> FDS has not yet been implemented on MacOS X.
>>>>>>
>>>>>> Just like the original FDS changes, the FDS Revamp changes are
>>>>>> in
>>>>>> multiple repos:
>>>>>>
>>>>>> 'hotspot' repo change summary:
>>>>>>
>>>>>> - add support for exporting .diz (Debug Info Zip) files
>>>>>> - add support for ENABLE_FULL_DEBUG_SYMBOLS build flag
>>>>>> (replaces overloaded uses of OBJCOPY variable)
>>>>>> - add support for ZIP_DEBUGINFO_FILES build flag
>>>>>> - clean up STRIP_POLICY on Linux and Solaris
>>>>>> - On Solaris, also fixes an incorrect 64-bit libjvm_db_g
>>>>>> symlink
>>>>>> and an incorrect 64-bit libjvm_dtrace_g symlink
>>>>>> - The Full Debug Symbols feature is now controllable via
>>>>>> ENABLE_FULL_DEBUG_SYMBOLS and ZIP_DEBUGINFO_FILES on
>>>>>> Windows.
>>>>>> - On Windows, fixed a few hardcoded "sawindbg" uses
>>>>>>
>>>>>> 'hotspot' repo webrev:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7102323-webrev/1-hotspot-full/
>>>>>>
>>>>>> The HotSpot changes are relative to the HSX-24-B03 snapshot
>>>>>> plus
>>>>>> one additional fix and are targeted at JDK8-B33/HSX-24-B06.
>>>>>>
>>>>>>
>>>>>> 'jdk' repo change summary:
>>>>>>
>>>>>> - add support for importing .diz (Debug Info Zip) files
>>>>>> - add support for ENABLE_FULL_DEBUG_SYMBOLS build flag
>>>>>> - add support for ZIP_DEBUGINFO_FILES build flag
>>>>>> - clean up STRIP_POLICY on Linux and Solaris
>>>>>> - LIBRARY_SUPPORTS_FULL_DEBUG_SYMBOLS is only needed in
>>>>>> FDS Phase 2 so just a comment for now
>>>>>> - JPRT needs to use the '-y' option with zip on non-Windows
>>>>>> builds of the jdk repo in order to preserve symbolic
>>>>>> links
>>>>>>
>>>>>> 'jdk' repo webrev:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-jdk-full/
>>>>>>
>>>>>> The JDK changes are relative to the T&L snapshot for
>>>>>> JDK8-B30
>>>>>> and are targeted at JDK8-B33.
>>>>>>
>>>>>>
>>>>>> 'root' repo change summary:
>>>>>>
>>>>>> - JPRT needs to use the '-y' option with zip on non-Windows
>>>>>> control builds in order to preserve symbolic links
>>>>>>
>>>>>> 'root' repo webrev:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-root-full/
>>>>>>
>>>>>> The root changes are relative to the T&L snapshot for
>>>>>> JDK8-B30
>>>>>> and are targeted at JDK8-B33.
>>>>>>
>>>>>> Thanks, in advance, for any review comments.
>>>>>>
>>>>>> Dan
>>>>>>
>>>> --
>>>> Andrew :)
>>>>
>>>> Free Java Software Engineer
>>>> Red Hat, Inc. (http://www.redhat.com)
>>>>
>>>> PGP Key: 248BDC07 (https://keys.indymedia.org/)
>>>> Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07
>>>>
>>>>
More information about the build-dev
mailing list