<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#CCCCCC" text="#000000">
Dan,<br>
<br>
I've reviewed this:<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dcubed/fds_revamp/7102323-webrev/1-hotspot-full/">http://cr.openjdk.java.net/~dcubed/fds_revamp/7102323-webrev/1-hotspot-full/</a><br>
<br>
<br>
<br>
<br>
Wow, you fixed two existing bugs in the make file:<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<b>make/solaris/makefiles/dtrace.make<br>
<br>
</b>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<pre><span class="removed">- [ -f $(XLIBJVM_DB_G_DEBUGINFO) ] || { ln -s $(LIBJVM_DB_DEBUGINFO) $(XLIBJVM_DB_G_DEBUGINFO); }</span>
<span class="new">+ [ -f $(XLIBJVM_DB_G_DEBUGINFO) ] || { ln -s $(XLIBJVM_DB_DEBUGINFO) $(XLIBJVM_DB_G_DEBUGINFO); }</span></pre>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<pre><span class="removed">- [ -f $(XLIBJVM_DTRACE_G_DEBUGINFO) ] || { ln -s $(LIBJVM_DTRACE_DEBUGINFO) $(XLIBJVM_DTRACE_G_DEBUGINFO); }</span>
<span class="new">+ [ -f $(XLIBJVM_DTRACE_G_DEBUGINFO) ] || { ln -s $(XLIBJVM_DTRACE_DEBUGINFO) $(XLIBJVM_DTRACE_G_DEBUGINFO); }</span></pre>
<br>
<br>
Wrong indent:<br>
<br>
<b>make/solaris/makefiles/defs.make</b><br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<pre><span class="changed"> 221 ifeq ($(ZIP_DEBUGINFO_FILES),1)</span>
<span class="changed"> 222 EXPORT_LIST += $(EXPORT_JRE_LIB_ARCH_DIR)/libsaproc.diz</span>
<span class="changed"> 223 else
</span></pre>
Question: Just curious why the <span class="removed">$(ALT_STRIP_POLICY)</span>
is decommissioned?<br>
<br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<b>make/linux/makefiles/defs.make</b><br>
<b> make/solaris/makefiles/defs.make</b><br>
<meta http-equiv="content-type" content="text/html;
charset=ISO-8859-1">
<pre><span class="removed">- DEF_STRIP_POLICY="min_strip"</span>
<span class="removed">- ifeq ($(ALT_STRIP_POLICY),)</span>
<span class="removed">- STRIP_POLICY=$(DEF_STRIP_POLICY)</span>
<span class="removed">- else</span>
<span class="removed">- STRIP_POLICY=$(ALT_STRIP_POLICY)</span>
<span class="removed">- endif</span>
<span class="new">+ # Currently, STRIP_POLICY is only used when Full Debug Symbols is enabled.</span>
<span class="new">+ #</span>
<span class="new">+ STRIP_POLICY ?= min_strip</span></pre>
<br>
The fix is good in general.<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
<br>
On 3/16/12 12:58 PM, Daniel D. Daugherty wrote:
<blockquote cite="mid:4F639B66.8090902@oracle.com" type="cite">Greetings,
<br>
<br>
I need code reviews for some Makefile and packaging changes.
<br>
Wait, come back! They're not that scary...
<br>
<br>
These are Full Debug Symbols changes... so maybe they are that
scary...
<br>
<br>
These changes have gone through two rounds of internal review.
<br>
<br>
The following bugs are being used to revamp the OpenJDK side of
the
<br>
Full Debug Symbols (FDS) implementation:
<br>
<br>
7102323 4/4 RFE: enable Full Debug Symbols Phase 1 on Solaris
<br>
7136506 3/4 FDS: rework jdk repo Full Debug Symbols support
<br>
<br>
FDS Revamp Summary
<br>
<br>
The build infrastructure that supports the Full Debug Symbols
(FDS)
<br>
project is being revamped to reduce the default on-disk
footprint
<br>
along with other improvements. FDS info will have to be
unzip'ed
<br>
before being usable in the default build config, but the
zip'ed FDS
<br>
info occupies about 25% of the disk space as the original FDS
info.
<br>
<br>
Change summary for the group of fixes:
<br>
- ENABLE_FULL_DEBUG_SYMBOLS build flag controls the Full Debug
<br>
Symbols feature; enabled by default
(ENABLE_FULL_DEBUG_SYMBOLS=1)
<br>
- ZIP_DEBUGINFO_FILES build flag controls the zip'ing of
"debug info"
<br>
during the build; enabled by default
(ZIP_DEBUGINFO_FILES=1).
<br>
- FDS is enabled by default for Linux X86/X64, Solaris
SPARC/SPARC-V9,
<br>
Solaris X86, and Windows X86/X64.
<br>
- HSX developer builds will put debug info into .diz files
that are
<br>
co-located with the built object, e.g., there will be a
libjvm.diz
<br>
file right next to libjvm.so.
<br>
- HSX JPRT jobs will also contain .diz files co-located with
the built
<br>
objects
<br>
- RE promoted bits will include new debuginfo.zip bundles that
contain
<br>
all the .debuginfo, .diz, .map and/or .pdb files generated
by the
<br>
various repos that make up the RE promotion.
<br>
<br>
Notes: FDS is not enabled on Solaris X64 due to a bug in
gobjcopy.
<br>
FDS has not yet been implemented on MacOS X.
<br>
<br>
Just like the original FDS changes, the FDS Revamp changes are in
<br>
multiple repos:
<br>
<br>
'hotspot' repo change summary:
<br>
<br>
- add support for exporting .diz (Debug Info Zip) files
<br>
- add support for ENABLE_FULL_DEBUG_SYMBOLS build flag
<br>
(replaces overloaded uses of OBJCOPY variable)
<br>
- add support for ZIP_DEBUGINFO_FILES build flag
<br>
- clean up STRIP_POLICY on Linux and Solaris
<br>
- On Solaris, also fixes an incorrect 64-bit libjvm_db_g
symlink
<br>
and an incorrect 64-bit libjvm_dtrace_g symlink
<br>
- The Full Debug Symbols feature is now controllable via
<br>
ENABLE_FULL_DEBUG_SYMBOLS and ZIP_DEBUGINFO_FILES on
Windows.
<br>
- On Windows, fixed a few hardcoded "sawindbg" uses
<br>
<br>
'hotspot' repo webrev:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dcubed/fds_revamp/7102323-webrev/1-hotspot-full/">http://cr.openjdk.java.net/~dcubed/fds_revamp/7102323-webrev/1-hotspot-full/</a><br>
<br>
The HotSpot changes are relative to the HSX-24-B03 snapshot
plus
<br>
one additional fix and are targeted at JDK8-B33/HSX-24-B06.
<br>
<br>
<br>
'jdk' repo change summary:
<br>
<br>
- add support for importing .diz (Debug Info Zip) files
<br>
- add support for ENABLE_FULL_DEBUG_SYMBOLS build flag
<br>
- add support for ZIP_DEBUGINFO_FILES build flag
<br>
- clean up STRIP_POLICY on Linux and Solaris
<br>
- LIBRARY_SUPPORTS_FULL_DEBUG_SYMBOLS is only needed in
<br>
FDS Phase 2 so just a comment for now
<br>
- JPRT needs to use the '-y' option with zip on non-Windows
<br>
builds of the jdk repo in order to preserve symbolic links
<br>
<br>
'jdk' repo webrev:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-jdk-full/">http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-jdk-full/</a>
<br>
<br>
The JDK changes are relative to the T&L snapshot for
JDK8-B30
<br>
and are targeted at JDK8-B33.
<br>
<br>
<br>
'root' repo change summary:
<br>
<br>
- JPRT needs to use the '-y' option with zip on non-Windows
<br>
control builds in order to preserve symbolic links
<br>
<br>
'root' repo webrev:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-root-full/">http://cr.openjdk.java.net/~dcubed/fds_revamp/7136506-webrev/1-root-full/</a><br>
<br>
The root changes are relative to the T&L snapshot for
JDK8-B30
<br>
and are targeted at JDK8-B33.
<br>
<br>
Thanks, in advance, for any review comments.
<br>
<br>
Dan
<br>
</blockquote>
<br>
</body>
</html>