[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-hotspot-dev
Subject: RE: RFR: 8237192: Generate stripped/public pdbs on Windows for jdk images
From: "Langer, Christoph" <christoph.langer () sap ! com>
Date: 2020-02-19 15:42:22
Message-ID: AM0PR02MB5714537E20703B9A9270B47B8A100 () AM0PR02MB5714 ! eurprd02 ! prod ! outlook ! com
[Download RAW message or body]
Hi Erik,
thanks for the review. Seems like it's looking good now in our CI. I'll run it \
through jdk-submit before pushing.
Best regards
Christoph
From: Erik Joelsson <erik.joelsson@oracle.com>
Sent: Dienstag, 18. Februar 2020 18:09
To: Langer, Christoph <christoph.langer@sap.com>; Magnus Ihse Bursie \
<magnus.ihse.bursie@oracle.com>; 'build-dev@openjdk.java.net' \
<build-dev@openjdk.java.net>
Cc: 'hotspot-dev@openjdk.java.net' <hotspot-dev@openjdk.java.net>; Baesken, Matthias \
<matthias.baesken@sap.com>
Subject: Re: RFR: 8237192: Generate stripped/public pdbs on Windows for jdk images
Hello Christoph,
Thanks for hanging in there, this is now looking good to me. Nice to see a more \
general solution to the java.pdb problem.
/Erik
On 2020-02-18 06:47, Langer, Christoph wrote:
Hi,
I had to update my change a little bit – I forgot to exclude jimage.map, java.map \
and jpackage.map when copying the cmd debug symbols in Images.gmk.
Furthermore, I needed to modify tests jdk/tools/launcher/TestHelper.java and \
jdk/tools/launcher/VersionCheck.java to cope with debug symbol files in \
images/jdk/bin on non Windows platforms.
http://cr.openjdk.java.net/~clanger/webrevs/8237192.2/
Cheers
Christoph
From: Langer, Christoph
Sent: Montag, 17. Februar 2020 10:17
To: Erik Joelsson <erik.joelsson@oracle.com><mailto:erik.joelsson@oracle.com>; Magnus \
Ihse Bursie <magnus.ihse.bursie@oracle.com><mailto:magnus.ihse.bursie@oracle.com>; \
'build-dev@openjdk.java.net<mailto:build-dev@openjdk.java.net>' \
<build-dev@openjdk.java.net><mailto:build-dev@openjdk.java.net>
Cc: 'hotspot-dev@openjdk.java.net<mailto:hotspot-dev@openjdk.java.net>' \
<hotspot-dev@openjdk.java.net><mailto:hotspot-dev@openjdk.java.net>; Baesken, \
Matthias <matthias.baesken@sap.com><mailto:matthias.baesken@sap.com>
Subject: RE: RFR: 8237192: Generate stripped/public pdbs on Windows for jdk images
Hi Erik,
thanks for your review. While I've addressed all your points (thanks for the great \
hints regarding usage of SetupCopyFiles), I also enhanced the configure option a \
little bit.
What you can now pass is --with-external-symbols-in-bundles=<none|public|full>. \
Default setting is none. It'll require --with-native-debug-symbols=external obviously \
and is currently only implemented for Windows. This is checked in configure. \
Depending on the value used, the bundles and jmods will contain either no pdbs, \
public pdbs or the full pdbs. This configure option could easily be enhanced to work \
for Linux/Unix/Mac as well – but I didn't want to go too far with my change now.
What also comes with my change is debug files for executables (launchers, cmds) in \
images/<jdk|jre> which weren't copied so far in Images.gmk. It's however needed \
because bundles are created from these images and with the current bundle logic, \
stripped pdb files need to exist in images/… to get bundled for option ‘public'.
I also removed the special handling of the pdb file for java.exe in \
Launcher-java.base.gmk, as the filtering to make sure it's not overwriting the pdb \
for java.dll has to be done in CreateJmods.gmk and Images.gmk anyway.
Eventually, when this is all refactored, one should probably generate the pdb files \
to ship into support/modules_libs and support/modules_cmds and only in the case of \
public pdbs redirect generation of the full pdbs to something like \
support/modules_libs_full_debug_info or something like that. Then, from \
support/modules_libs and support/modules_cmds, one should generate the base image via \
jlink which can simply be packed into the shipment bundle. And the other image with \
all debug/demos can be created by copying the base image and then applying the stuff \
from support/ modules_libs_full_debug_info etc.
Here's the new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8237192.1/
Best regards
Christoph
From: Erik Joelsson <erik.joelsson@oracle.com<mailto:erik.joelsson@oracle.com>>
Sent: Mittwoch, 12. Februar 2020 23:17
To: Langer, Christoph <christoph.langer@sap.com<mailto:christoph.langer@sap.com>>; \
Magnus Ihse Bursie <magnus.ihse.bursie@oracle.com<mailto:magnus.ihse.bursie@oracle.com>>; \
'build-dev@openjdk.java.net<mailto:build-dev@openjdk.java.net>' \
<build-dev@openjdk.java.net<mailto:build-dev@openjdk.java.net>>
Cc: 'hotspot-dev@openjdk.java.net<mailto:hotspot-dev@openjdk.java.net>' \
<hotspot-dev@openjdk.java.net<mailto:hotspot-dev@openjdk.java.net>>; Baesken, \
Matthias <matthias.baesken@sap.com<mailto:matthias.baesken@sap.com>>
Subject: Re: RFR: 8237192: Generate stripped/public pdbs on Windows for jdk images
Hello Christoph,
This patch certainly looks better to me, though I agree it's a bit hackish to have to \
filter and rename the stripped.pdb files twice, once for jmods and again for bundles. \
I think I'm ok with it for now though. The future improvement I would like to make \
would be to create two sets of jdk images, one that contains debug symbols and demos, \
which we continue to use for testing, and another which only contains exactly what we \
bundle up, including the correctly named top dir. The latter would be created first \
and used as input for the former. I think lots of things would be cleaner then, \
especially Bundles.gmk.
But, that can wait for later. With your current proposal, my proposed future change \
will apply seamlessly in regard to the stripped pdb files and our distribution \
bundles.
The clash between launchers and libs is annoying, and we also have it for java.exe \
and java.dll already, though the build is explicitly handling that one somewhere \
else.
Now to review, there are some details I will nitpick about.
CreateJmods.gmk:
174, 185: Rule declarations should be indented like any other line inside \
conditionals. But, I have a problem with these rules as the target is the directory. \
This will not work well with incremental builds. I would recommend doing a \
SetupCopyFiles construct instead so you get individual rules for each file. It would \
look something like this: rename-stripped-pdb = \
$(patsubst %.stripped.pdb,%.pdb,$1)
$(eval $(call SetupCopyFiles, COPY_STRIPPED_LIBS, \
SRC := $(LIBS_DIR), \
DEST := $(LIBS_DIR_STRIPPED), \
FILES := $(call FindFiles, $(LIBS_DIR)), \
NAME_MACRO := rename-stripped-pdb, \
))
DEPS += $(COPY_STRIPPED_LIBS)
For the corresponding CMD_DIR, the removal of jimage and friends can be done with \
$(filter ) around The FindFiles call.
GenerateLinkOptData.gmk:
Please indent inside ifeq block. I would prefer having the TARGETS += inside the \
conditional block. Seems you also left a commented out endif there.
NativeCompilation.gmk
919: You changed the continuation indent from 4 to 2, please revert.
/Erik
On 2020-02-12 05:26, Langer, Christoph wrote:
Hi Magnus, Erik,
I started off with Matthias' patch and tried to address your concerns. Especially I \
explored adding the stripped pdbs to the jmods as well. Here is what I came up with: \
http://cr.openjdk.java.net/~clanger/webrevs/8237192.0/
It's a bit hacky in that it'll copy support/modules_libs to \
support/modules_libs_stripped and fix the pdbs to ship in there. The same goes for \
modules_cmds. The problem with that approach is that probably not all dependencies \
are placed correctly and also there's a bit more of duplication of binaries in the \
build directories. I think, it could be repaired eventually by refactoring, e.g. have \
a support/modules_dbgsymbols folder where the real debug symbol files get placed and \
used from there.
There's also two additional caveats, one is that jimage.pdb and jpackage.pdb exist \
twice. Once for the libs and once for the launchers with the same name. This will \
cause failures when jlinking. I decided to keep the pdbs for the libs. And I also had \
to take care of the classlist generation, to have the resulting classlist placed in \
support/modules_libs_stripped as well.
I furthermore updated the naming of options and variables, hopefully to your like. \
And I made the debug output logInfo.
I tested (on Windows), both, with --enable-public-debug-symbols and without. Without \
the option, everything seems as before. With the option enabled, the stripped debug \
symbols will be installed in the bundles and also in the jmods. 😊
Please let me know what you think.
Thanks & Best regards
Christoph
From: Magnus Ihse Bursie \
<magnus.ihse.bursie@oracle.com><mailto:magnus.ihse.bursie@oracle.com>
Sent: Freitag, 7. Februar 2020 14:09
To: Baesken, Matthias <matthias.baesken@sap.com><mailto:matthias.baesken@sap.com>; \
Erik Joelsson <erik.joelsson@oracle.com><mailto:erik.joelsson@oracle.com>; Langer, \
Christoph <christoph.langer@sap.com><mailto:christoph.langer@sap.com>; David Holmes \
<david.holmes@oracle.com><mailto:david.holmes@oracle.com>; \
'build-dev@openjdk.java.net<mailto:build-dev@openjdk.java.net>' \
<build-dev@openjdk.java.net><mailto:build-dev@openjdk.java.net>; \
'hotspot-dev@openjdk.java.net<mailto:hotspot-dev@openjdk.java.net>' \
<hotspot-dev@openjdk.java.net><mailto:hotspot-dev@openjdk.java.net>
Subject: Re: RFR: 8237192: Generate stripped/public pdbs on Windows for jdk images
On 2020-02-07 09:50, Baesken, Matthias wrote:
Hello, here is a slightly changed new webrev :
http://cr.openjdk.java.net/~mbaesken/webrevs/8237192.4/
In Bundles.gmk, this line:
$(ECHO) found stripped pdb file $$$${f}, we rename it to: $$$${f%stripped.pdb}pdb; \
It looks almost like left-over debug output. If you want to keep it, please rephrase \
to something more terse, that fits better with the existing style of build messages. \
Also, it should probably be on the LOG=info level, so add a $(LOG_INFO).
In NativeCompilation.gmk:
Why not just a simple,
ifeq ($(ENABLE_STRIPPED_PDBS), true)
$1_EXTRA_LDFLAGS += \
"-pdbstripped:$$($1_SYMBOLS_DIR)/$$($1_NOSUFFIX).stripped.pdb" endif
?
I see no reason to duplicate code.
In jdk-options.m4:
I'm not 100% sure about the name of the new option. --enable-stripped-pdb does not \
fully convey the fact that we do not strip the *existing* pdb:s, but instead also add \
a new type. Maybe --enable-bundle-stripped-pdb?
/Magnus
(adjusted $(JRE_STRIPPED_PDB_FILES) in Bundles.gmk, that was in the wrong place ) .
I think it needs to be a separate option as it's all orthogonal to the
main debug symbols file creation.
Yes it is a separate option I agree that's better . One has to set \
--enable-stripped-pdbs=yes .
Best regards , Matthias
On 2020-02-06 04:48, Magnus Ihse Bursie wrote:
On 2020-02-06 12:36, Langer, Christoph wrote:
Hi,
let me chime in to this discussion at this point. So, first of all, Matthias,
thanks for your work so far.
Erik, I fully understand your points and I agree that it's probably a good
idea to refactor this whole process of creating these different types of
bundles.
Our idea is to provide functionality in the build system to add "public" or
stripped debug files to the delivery image of the JDK. This would provide
better information in case of crashes and would hence allow for better
supportability. That's something we'd probably want to enable in
SapMachine binary distributions.
I very much support the idea of using these stripped pdb files. It has
been a long standing issue in hotspot on Windows to not have access to
stacktraces.
So, can we get to add a configure option like the one proposed by
Matthias into the current code base?
The option should be turned off by default. If it is switched on, the
images/jdk folder in the build directory will have both, the *stripped.pdb
files and the standard *.pdb files. However, having *stripped.pdb files
around should not matter in terms of functionality (for developers and
testing) as they'd not be used in the presence of the "real" pdb files anyway.
The actual JDK bundle for delivery would then contain the *stripped.pdb
files, renamed to *.pdb. And the debug symbols bundle would have the full
*.pdb files only. Both could then be overlaid as needed.
I think you raised two concerns.
One is that you'd rather like to refactor bundling for several reasons. But I
guess, should you eventually get to your refactoring, it shouldn't be a
problem to take the functionality of this new option along.
The other was regarding JMODs. I believe it's correct, that JMODs have
never carried external debug information. For platforms other than MS
Windows that's actually not a big problem because debug information can be
internalized. And jlink has gotten several additions to set flags for stripping
that data to the right level. I assume if jlinked images on Windows should
ever be enabled to carry debug information, inclusion of external debug files
would have to be added to JMODs and jlink. But that's definitely out of scope
here.
The argument "jmods have never carried external debug information" just
doesn't apply here. Neither has the distribution bundles, for the exact
same reason. You really should not compare these new stripped pdb files
to the existing debug symbol files, they are different files with
different purposes. One is meant to be shipped to customers, the other
is not. You say you want to ship these new stripped pdb files with your
distribution so that customers have them present when they use your
distribution. If you then omit these new files from the jmods, any
customer created jlinked image will not have these new stripped pdb
files, which IMO is a very weird and unexpected behavior from a customer
point of view. Jlinking new images is an integral and promoted way of
using a JDK, so any mismatch between the original JDK distribution and
what you are able to jlink is a serious discrepancy.
So, what do you think? What speaks against adding this option (that is off
by default)?
My main objective is that you introduce further discrepancies between
the original distribution JDK image and what's possible to generate
using jlink from that distribution JDK image. My second objective is
that the already convoluted bundles creation logic becomes even more
convoluted. I believe there is a better possible solution in the build
but it will require a lot more work to figure out.
All that said, if you still wish to continue, I will stop standing in
the way.
While Erik will need to comment on this himself, from my POV this
looks okay. The conditions are:
* This is controlled by a separate option (or perhaps even better as a
new argument to --with-native-debug-symbols), so without this, nothing
will change.
I think it needs to be a separate option as it's all orthogonal to the
main debug symbols file creation.
* The code need to make sure to separate stripped.pdb and normal pdb
files, when enabled.
* And there must not be a heavy penalty in added code complexity.
/Erik
/Magnus
Thanks
Christoph
-----Original Message-----
From: build-dev<build-dev-bounces@openjdk.java.net><mailto:build-dev-bounces@openjdk.java.net> \
On Behalf Of
Erik
Joelsson
Sent: Donnerstag, 23. Januar 2020 18:49
To: Baesken, Matthias<matthias.baesken@sap.com><mailto:matthias.baesken@sap.com>; \
David Holmes
<david.holmes@oracle.com><mailto:david.holmes@oracle.com>; \
'build-dev@openjdk.java.net<mailto:build-dev@openjdk.java.net>' <build-
dev@openjdk.java.net<mailto:dev@openjdk.java.net>>; \
'hotspot-dev@openjdk.java.net<mailto:hotspot-dev@openjdk.java.net>' <hotspot-
dev@openjdk.java.net<mailto:dev@openjdk.java.net>>
Subject: Re: RFR: 8237192: Generate stripped/public pdbs on Windows
for
jdk images
On 2020-01-23 00:03, Baesken, Matthias wrote:
Hi Erik, yes true sorry for answering your comments a bit late .
If a user runs jlink and includes all the jmods we ship with the JDK, the
result
should be essentially equivalent to the original JDK image. The way
the
stripped pdb files are included in the bundles sort of at the last
second of the build here breaks this property.
I think we should address this in a separate bug/CR .
Maybe. I realize that my proposal below is quite a big task. But on the
other hand, I don't think breaking the relationship between the jmods
and the distribution bundles is on the table really.
Looking for example into a Linux build, I see a lot of debuginfo files in
the
jdk image (more or less for every shared lib) .
But when looking into the jmods of that jdk image , no debuginfo files
are
in there ( I checked the java.base jmod).
So putting the files with debug information into the jmods seems to
be
something that was not done so far cross platform (or is there some
build
switch for it that I did not check?) .
Maybe to keep the jmods as small as possible .
No, we do not put the debuginfo files in the jmods nor the bundles
because those are not intended to be shipped to customers. We are
currently overlaying them into images/jdk in the build output to make
them available for local debugging. (This is convoluted and I would very
much like to get away from this practice at some point so that there is
a 1-1 mapping between images/jdk and bundles/jdk*-bin.tar.gz.) The
stripped pdb files you are proposing are on the contrary intended for
shipping to customers (as I understand your proposal) so comparing
them
with the debuginfo files is not relevant.
Now if MS had been kind enough to define a separate file type for the
stripped pdbs, so that they could live alongside the full pdbs, we
wouldn't have this issue. The heart of the problem is that only one set
of files (either stripped or full) can be present and usable in
images/jdk at a time. We have 2 main uses for images/jdk.
1. Developer running and debugging locally
2. Serve as the source for generating the distribution bundles
We currently have one image serving both of these purposes, which is
already creating complicated and convoluted build steps. To properly
solve this I would argue for splitting these two apart into two
different images for the two different purposes. The build procedure
would then be, first build the images for distribution, only containing
what should go into each bundle. Then create the developer jdk image
by
copying files from the distribution images. On Windows, the first image
would contain the stripped pdbs and when building the second, those
would get overwritten with the full pdbs.
Now that I figured out a working model that would solve a bunch of
other
problems as well, I would love to implement it, but I doubt I will have
time in the near future.
/Erik
To properly implement this, care will need to be taken to juggle the
two
sets of pdb files around, making sure each build and test use case has
the correct one in place where and when it's needed. Quite possibly,
we
cannot cover all use cases with one build configuration. Developers
needing the full debug symbols when debugging locally would likely
need
to disable the stripped symbols so they get the full symbols
everywhere.
Possibly this would need to be the default for debug builds and
configurable for release builds.
From my limited experience , the developers do not work with the
bundles (that would contain now after my patch the stripped pds) but
with
a "normal" jdk image that is created my make all.
Best regards, Matthias
This still does not address anything in my objection.
/Erik
On 2020-01-22 07:46, Baesken, Matthias wrote:
Hello, here is an updated version :
http://cr.openjdk.java.net/~mbaesken/webrevs/8237192.3/
this one supports a configure switch "--enable-stripped-pdbs" to
enable
the feature .
Best regards, Matthias
-----Original Message-----
From: Baesken, Matthias
Sent: Dienstag, 21. Januar 2020 11:03
To: 'David Holmes'<david.holmes@oracle.com><mailto:david.holmes@oracle.com>; 'build-
dev@openjdk.java.net<mailto:dev@openjdk.java.net>'<build-dev@openjdk.java.net><mailto:build-dev@openjdk.java.net>; \
'hotspot-
dev@openjdk.java.net<mailto:dev@openjdk.java.net>'<hotspot-dev@openjdk.java.net><mailto:hotspot-dev@openjdk.java.net>
Subject: RE: RFR: 8237192: Generate stripped/public pdbs on
Windows
for
jdk images
Hi David , yes I think it makes sense to have a configure option for
this .
Not everyone would like to have a larger JDK (even it is only a bit
larger).
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic