Re: RFR: 8287745 jni/nullCaller/NullCallerTest.java fails with "exitValue = 1"

[Daniel D . Daugherty]

On Fri, 3 Jun 2022 07:56:38 GMT, Tim Prinzing  wrote:

> Fixed JtregNativeJdk.gmk to include c++ libs for NullCallerTest

I did a Mach5 Tier1 test run on the v00 version of this patch. There were
no failures and all 6 runs of jni/nullCaller/NullCallerTest.java passed.

-

PR: https://git.openjdk.java.net/jdk/pull/9010

Re: RFR: 8280916: Simplify HotSpot Style Guide editorial changes

[Daniel D . Daugherty]

On Sun, 30 Jan 2022 00:39:20 GMT, Kim Barrett  wrote:

> Please review this change to the HotSpot Style Guide change process.
> 
> The current process involves gathering consensus among the HotSpot Group
> Members.  That's fine for changes of substance.  But it seems overly weighty
> for editorial changes that don't affect the substance of the guide, but only
> it's clarity or accuracy.
> 
> The proposed change would permit the normal PR process to be used for such
> changes, but require the requisite reviewers to additionally be HotSpot Group
> Members.
> 
> Note that there have already been a couple of changes that effectively
> followed the proposed new process.
> https://bugs.openjdk.java.net/browse/JDK-8274169
> https://bugs.openjdk.java.net/browse/JDK-8280182
> 
> This is a modification of the Style Guide, so rough consensus among the
> HotSpot Group members is required to make this change. Only Group members
> should vote for approval (via the github PR), though reasoned objections or
> comments from anyone will be considered. A decision on this proposal will not
> be made before Monday 14-Feb-2022 at 12h00 UTC.
> 
> Since we're piggybacking on github PRs here, please use the PR review process
> to approve (click on Review Changes > Approve), rather than sending a "vote:
> yes" email reply that would be normal for a CFV.

Thumbs up.

-

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7281

Re: RFR: 8278753: Runtime crashes with access violation during JNI_CreateJavaVM call

[Daniel D . Daugherty]

On Tue, 25 Jan 2022 00:20:19 GMT, Yumin Qi  wrote:

> Please review,
>   When jlink with --compress=2, zip is used to compress the files while doing 
> copy. The user case failed to load zip.dll, since zip.dll is not set in PATH. 
> This failure is after we get NULL from GetModuleHandle("zip.dll"), then do 
> LoadLibrary("zip.dll") will have same result.
>   The fix is calling load_zip_library of ClassLoader first --- if zip library 
> already loaded just return the cached handle for following usage, if not, 
> load zip library and cached the handle.
> 
>   Tests: tier1,4,7 in test
>Manually tested user case, and checked output of jimage list  for 
> jlinked files using --compress=2.
> 
> Thanks
> Yumin

Your Tier7 failure is likely this known bug:

JDK-8280601 ClhsdbThreadContext.java test is triggering codecache related 
assert in PointerFinder.find()
https://bugs.openjdk.java.net/browse/JDK-8280601

-

PR: https://git.openjdk.java.net/jdk/pull/7206

Integrated: JDK-8277071 [BACKOUT] JDK-8276743 Make openjdk build Zip Archive generation "reproducible"

[Daniel D . Daugherty]

On Fri, 12 Nov 2021 18:29:01 GMT, Daniel D. Daugherty  
wrote:

> This reverts commit aeba65303479130d9bab74484accad5d7d116a40.

This pull request has now been integrated.

Changeset: 74f3e69d
Author:    Daniel D. Daugherty 
URL:   
https://git.openjdk.java.net/jdk/commit/74f3e69dc888685558408e663df5d32cb906991f
Stats: 261 lines in 4 files changed: 0 ins; 254 del; 7 mod

8277071: [BACKOUT] JDK-8276743 Make openjdk build Zip Archive generation 
"reproducible"

Reviewed-by: erikj

-

PR: https://git.openjdk.java.net/jdk/pull/6370

Re: RFR: JDK-8277071 [BACKOUT] JDK-8276743 Make openjdk build Zip Archive generation "reproducible"

[Daniel D . Daugherty]

On Fri, 12 Nov 2021 18:38:42 GMT, Erik Joelsson  wrote:

>> This reverts commit aeba65303479130d9bab74484accad5d7d116a40.
>
> Marked as reviewed by erikj (Reviewer).

@erikj79 - Thanks for the fast review!

-

PR: https://git.openjdk.java.net/jdk/pull/6370

RFR: JDK-8277071 [BACKOUT] JDK-8276743 Make openjdk build Zip Archive generation "reproducible"

[Daniel D . Daugherty]

This reverts commit aeba65303479130d9bab74484accad5d7d116a40.

-

Commit messages:
 - JDK-8277071 [BACKOUT] JDK-8276743 Make openjdk build Zip Archive generation 
"reproducible"

Changes: https://git.openjdk.java.net/jdk/pull/6370/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6370&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8277071
  Stats: 261 lines in 4 files changed: 0 ins; 254 del; 7 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6370.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6370/head:pull/6370

PR: https://git.openjdk.java.net/jdk/pull/6370

Re: RFR: 8264848: [macos] libjvm.dylib linker warning due to macOS version mismatch [v2]

[Daniel D . Daugherty]

On Wed, 7 Apr 2021 19:03:14 GMT, Daniel D. Daugherty  wrote:

>> Lutz Schmidt has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Updated comment per request from erikj79
>
> Thumbs up.

Side bar: I don't quite understand the commit history part of this PR.
It looks like this PR was used for something else before it was used
for this fix.

-

PR: https://git.openjdk.java.net/jdk/pull/3379

Re: RFR: 8264848: [macos] libjvm.dylib linker warning due to macOS version mismatch [v2]

[Daniel D . Daugherty]

On Wed, 7 Apr 2021 18:48:01 GMT, Lutz Schmidt  wrote:

>> May I please request reviews for this small build fix. It removes a linker 
>> warning by adding a assembly-time parameter which was previously missing. 
>> The same parameter is used at c++ compile time.
>
> Lutz Schmidt has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Updated comment per request from erikj79

Thumbs up.

-

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3379

Re: RFR: 8263465: JDK-8236847 causes tier1 build failure on linux-aarch64

[Daniel D . Daugherty]

On Thu, 11 Mar 2021 18:41:10 GMT, Yumin Qi  wrote:

> Hi, Please review
> 
>   JDK-8236847 changes failed on build linux-aarch64 on xcross build. The 
> reason is we check BUILD_CDS_ARCHIVE which is not correct in such case. We 
> should check ENABLE_CDS instead.
> 
> Thanks
> Yumin

Looks good to me.
This change can be integrated until the trivial fix rules.

-

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2946

Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]

[Daniel D . Daugherty]

On Tue, 2 Feb 2021 11:59:08 GMT, Anton Kozlov  wrote:

>> Please review the implementation of JEP 391: macOS/AArch64 Port.
>> 
>> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and 
>> windows/aarch64. 
>> 
>> Major changes are in:
>> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks 
>> JDK-8253817, JDK-8253818)
>> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with 
>> necessary adjustments (JDK-8253819)
>> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), 
>> required on macOS/AArch64 platform. It's implemented with 
>> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a 
>> thread, so W^X mode change relates to the java thread state change (for java 
>> threads). In most cases, JVM executes in write-only mode, except when 
>> calling a generated stub like SafeFetch, which requires a temporary switch 
>> to execute-only mode. The same execute-only mode is enabled when a java 
>> thread executes in java or native states. This approach of managing W^X mode 
>> turned out to be simple and efficient enough.
>> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941)
>
> Anton Kozlov has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   support macos_aarch64 in hsdis

For platform files that were copied from other ports to this port, if the file 
wasn't
changed I presume the copyright years are left alone. If the file required 
changes
for this port, I expect the year to be updated to 2021. How are you verifying 
that
these copyright years are being properly managed on the new files?

For the new W^X helpers, e.g., WXWriteFromExecSetter, a short comment
explaining why one was landed in a particular place would help reviewers.
Also see my comment about creating a new ThreadToNativeWithWXExecFromVM
helper.

I'm stopping my review with all the src/hotspot files done for now.

make/autoconf/flags.m4 line 140:

> 138: else
> 139:   MACOSX_VERSION_MIN=10.12.0
> 140: fi

Not something that needs to be addressed here, but these changes
illustrate that our collective use of macOSX/MACOSX/MacOSX names
are tied to the fact that the macOS major version number was at 10
for a very long time.

@magicus - Do we have an RFE to rename MACOSX or are we sticking
with it and evolving our interpretation of the 'X' from '10' to */splat/asterik?

make/common/NativeCompilation.gmk line 1178:

> 1176: endif
> 1177: # This only works if the openjdk_codesign identity is 
> present on the system. Let
> 1178: # silently fail otherwise.

Might want to add a comment here:
#  The '-f' option will replace an existing signature if one exists.

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 41:

> 39: #define __ _masm->
> 40: 
> 41: //describe amount of space in bytes occupied by type on native stack

nit - needs a space after `//`

src/hotspot/cpu/aarch64/interpreterRT_aarch64.cpp line 83:

> 81: // On macos/aarch64 native stack is packed, int/float are using only 4 
> bytes
> 82: // on stack. Natural alignment for types are still in place,
> 83: // for example double/long should be 8 bytes alligned

nit typo: s/alligned/aligned/
And sentence should end with a period.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 323:

> 321:   str(zr, Address(rthread, JavaThread::last_Java_pc_offset()));
> 322: 
> 323:   str(zr, Address(rthread, JavaFrameAnchor::saved_fp_address_offset()));

I don't think this switch from `JavaThread::saved_fp_address_offset()`
to `JavaFrameAnchor::saved_fp_address_offset()` is correct since
`rthread` is still used and is a JavaThread*. The new code will give you:

`rthread` + offset of the `saved_fp_address` field in a JavaFrameAnchor

The old code gave you:

`rthread` + offset of the `saved_fp_address` field in the JavaFrameAnchor 
field in the JavaThread

Those are not the same things.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp line 31:

> 29: #include "asm/assembler.inline.hpp"
> 30: #include "oops/compressedOops.hpp"
> 31: #include "runtime/vm_version.hpp"

It's not clear why this include needed to be added.

src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 810:

> 808: #ifdef __APPLE__
> 809:   // Less-than word types are stored one after another.
> 810:   // The code unable to handle this, bailout.

Perhaps:  // The code is unable to handle this so bailout.

src/hotspot/cpu/aarch64/sharedRuntime_aarch64.cpp line 837:

> 835: #ifdef __APPLE__
> 836:   // Less-than word types are stored one after another.
> 837:   // The code unable to handle this, bailout.

Perhaps: // The code is unable to handle this so bailout.

src/hotspot/os/aix/os_aix.cpp line 69:

> 67: #include "runtime/sharedRuntime.hpp"
> 68: #include "runtime/statSampler.hpp"
> 69: #include "runtime/stubRoutines.inline.hpp"

It is not clear wh

Re: RFR: 8258447: Move make/hotspot/hotspot.script to make/scripts

[Daniel D . Daugherty]

On Tue, 15 Dec 2020 23:28:25 GMT, Magnus Ihse Bursie  wrote:

> The hotspot launcher script is misplaced among the hotspot make files. It 
> should move to make/scripts (and get a proper extension).

The rename and makefile changes look good.
I've used the script on occasion, but not recently (not in the last 6 months).

-

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1791

Re: RFR: 8257572: Deprecate the archaic signal-chaining interfaces: sigset and signal

[Daniel D . Daugherty]

On Thu, 3 Dec 2020 04:34:52 GMT, David Holmes  wrote:

> The signal-chaining facility was introduced in JDK 1.4 nearly 20 years ago 
> and supported three different Linux signal API's: sigset, signal and 
> sigaction:
> 
> https://docs.oracle.com/javase/8/docs/technotes/guides/vm/signal-chaining.html
> 
> Only sigaction is a Posix supported API for multi-threaded processes, that we 
> can use cross-platform. Both signal and sigset are obsolete and have 
> undefined behaviour in a multi-threaded process. From the Linux man pages:
> 
> sigset: This API is obsolete: new applications should use the POSIX signal 
> API (sigaction(2), sigprocmask(2), etc.)
> 
> signal: The behavior of signal() varies across UNIX versions, and has also 
> varied historically across different versions of Linux. Avoid its use: use 
> sigaction(2) instead.
> 
> We should deprecate the use of signal and sigset in JDK 16 with a view to 
> their removal in JDK 17.
> 
> A CSR request has been filed.
> 
> Testing: hotspot/jtreg/runtime/signal tests
> 
> Thanks,
> David

Thumbs up on the jsig.c change. No comment on the Lib.gmk change.

-

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1587

Re: RFR: 8256430: add linux-x64-optimized to tier1

[Daniel D . Daugherty]

On Tue, 17 Nov 2020 00:31:24 GMT, Igor Ignatyev  wrote:

> Hi all,
> 
> 
> [8256414](https://bugs.openjdk.java.net/browse/JDK-8256414) /  #1233 added 
> similar profile to submit workflow, this patch defines `linux-x64-optimized` 
> profile in jib-profile so it can be used by mach5 and added to tier1? 
> 
> Thanks
> -- Igor
> 
> cc-ing @dcubed-ojdk

Thumbs up.

-

Marked as reviewed by dcubed (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1244

Re: RFR: 8256414: add optimized build to submit workflow [v2]

[Daniel D . Daugherty]

On Mon, 16 Nov 2020 19:29:59 GMT, Igor Ignatyev  wrote:

>> Marked as reviewed by shade (Reviewer).
>
> Thanks for the reviews, folks. the build looks green, integrating.

@iignatev - did you also change Mach5? I don't have workflow builds enabled
by default since I typically do Mach5 builds...

-

PR: https://git.openjdk.java.net/jdk/pull/1233

Re: RFR: 8212879: Make JVMTI TagMap table concurrent [v4]

[Daniel D . Daugherty]

On Mon, 2 Nov 2020 13:19:08 GMT, Coleen Phillimore  wrote:

>> Coleen Phillimore has updated the pull request with a new target base due to 
>> a merge or a rebase. The pull request now contains five commits:
>> 
>>  - Merge branch 'master' into jvmti-table
>>  - Merge branch 'master' into jvmti-table
>>  - More review comments from Stefan and ErikO
>>  - Code review comments from StefanK.
>>  - 8212879: Make JVMTI TagMap table not hash on oop address
>
> I think I addressed your comments, retesting now.  Thank you!

@coleenp - please make sure you hear from someone on the Serviceability team
for this PR...

-

PR: https://git.openjdk.java.net/jdk/pull/967

Re: RFR: 8212879: Make JVMTI TagMap table not hash on oop address

[Daniel D. Daugherty]

On 11/2/20 8:22 AM, Coleen Phillimore wrote:

On Mon, 2 Nov 2020 08:34:17 GMT, Stefan Karlsson  wrote:


src/hotspot/share/prims/jvmtiTagMap.cpp line 126:


124:   // concurrent GCs. So fix it here once we have a lock or are
125:   // at a safepoint.
126:   // SetTag and GetTag should not post events!

I think it would be good to explain why. Otherwise, this just leaves the 
readers wondering why this is the case.

Maybe even move this comment to the set_tag/get_tag code.

I was trying to explain why there's a boolean there but I can put this comment 
at both get_tag and set_tag.

   // Check if we have to processing for concurrent GCs.


Typo: s/we have to processing/we have to do processing/



   // GetTag should not post events because the JavaThread has to
   // transition to native for the callback and this cannot stop for
   // safepoints with the hashmap lock held.
   check_hashmap(false);

-

PR: https://git.openjdk.java.net/jdk/pull/967

Re: RFR: 8248238: Implementation of JEP: Windows AArch64 Support [v12]

[Daniel D. Daugherty]

So I'm confused... this PR is associated with this bug ID:



  Issue

  * JDK-8248238 :
Implementation of JEP: Windows AArch64 Support




and JDK-8248238 is associated with this JEP:

JDK-8248496  JEP 
388: Windows/AArch64 Port 


Am I missing something here?

Dan


On 10/1/20 5:56 PM, Ludovic Henry wrote:

Hi David,


The JEP is not yet targeted so we have to wait for that formality. But once 
that happens I can sponsor for you.

Perfect, I didn't know about the need for the JEP to be targeted before the 
merge.


Also note that the PR references the wrong JEP so can you please edit the 
description to fix that.

I'll work with @Monica to update the PR's description to point to 
https://openjdk.java.net/jeps/391 instead.

Thank you!
Ludovic
  

Re: RFR: 8243470: [macos] bring back O2 opt level for unsafe.cpp

[Daniel D. Daugherty]

Adding build-dev@... since this is a build system change.

As for the right HotSpot team, I'm not sure who should be making
the call on changing the way unsafe.cpp is built for macOSX. My guess
when I moved the bug to hotspot/runtime was that someone on the Runtime
team would know, but...

Dan



On 6/3/20 1:38 PM, Vladimir Kempik wrote:

Hello

Can somebody please review this simple change ?

Thanks


6 мая 2020 г., в 12:43, Vladimir Kempik  написал(а):

Adding hotspot-runtime-dev


23 апр. 2020 г., в 18:26, Vladimir Kempik  написал(а):


Hello
Please review a fix for JDK-8243470

Long time ago as part of JEP284: New HotSpot Build System this fix was applied 
to jdk9 https://bugs.openjdk.java.net/browse/JDK-8152666
At that time it was decided to lower optimisation level for unsafe.cpp from O2 
to O1 only for clang on Macosx.

I suppose it was done due to issues in Set/Get helper functions where 
too optimistic optimisations were eliminating some null pointer checks. it was 
probably a clang bug.
That issue could be checked with test jdk/test/sun/misc/CopyMemory.java.

I believe that workaround (going from O2 to O1) produced this issue - 
https://bugs.openjdk.java.net/browse/JDK-8234963 (Thread.getStackTrace is slow 
with clang).
JDK-8234963 can only be seen on mac with libjvm compiled by clang.

Here I propose the patch which eliminates that workaround for clang 8+.

I have tested clang versions 8/9/9.1/10, all of them showed good results:
1) CopyMemory test passes fine on 11/14/15.
2) jdk11/jdk14 passed tck. Regression testing were good as well. jdk15: no new 
failures in tck.
3) The testRandom "benchmark" from 8234963 shows great improvements on my 
machine, going down from ~200 seconds to ~150 seconds (the newer clang the better 
result). For comparision, gcc built libjvm for jdk11 shows ~130 seconds on my machine.

The webrev: http://cr.openjdk.java.net/~vkempik/8243470/webrev.00/

getStackTrace benchmark: 
http://cr.openjdk.java.net/~vkempik/8243470/TestRandom.java

The bug: https://bugs.openjdk.java.net/browse/JDK-8243470

Thanks, Vladimir

Re: Mach5 test failure when testing JDK-8237192

[Daniel D. Daugherty]

Greetings,

I filed two relevant bugs when I swept the JDK15 CI this AM:

    JDK-8239566 gtest/GTestWrapper.java fails due to "libstlport.so.1: 
open failed: No such file or directory"

    https://bugs.openjdk.java.net/browse/JDK-8239566

    JDK-8239565 sa/ClhsdbJhisto.java failed due to "assert(false) 
failed: unscheduable graph"

    https://bugs.openjdk.java.net/browse/JDK-8239565

The second bug is similar to a bug I filed earlier this week:

    JDK-8239367 RunThese30M.java failed due to "assert(false) failed: 
graph should be schedulable"

    https://bugs.openjdk.java.net/browse/JDK-8239367

The second bug is also with a different test than is mentioned below:

    serviceability/sa/ClhsdbCDSJstackPrintAll.java

but the assertion and the stack look like they match...

Dan



On 2/20/20 3:51 AM, Langer, Christoph wrote:

Thanks, David for the information.

As I don't see a relation from the crash to my change (I didn't touch any 
hotspot code at least), I guess I'm confident enough to push my patch. If worse 
comes to worse there's still the option to back it out again...

Best regards
Christoph


-Original Message-
From: David Holmes 
Sent: Donnerstag, 20. Februar 2020 09:28
To: Langer, Christoph ; 'build-
d...@openjdk.java.net' ; 'hotspot-
d...@openjdk.java.net' ; Erik Joelsson
; Magnus Ihse Bursie

Subject: Re: Mach5 test failure when testing JDK-8237192

Hi Christoph,

The Solaris failure looks like an infra issue.

The test failure is a crash - info below. I don't see any open, or
recently fixed, bugs for the same crash.

David
-

#
# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error
(/scratch/mesos/slaves/90726e33-be99-4e27-9d68-25dad266ef13-
S3967/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-
0196/executors/9c4085d3-922a-46c2-b44a-9835e74ddb36/runs/ecd258eb-
38e6-4966-b233-
0cdfa582f713/workspace/open/src/hotspot/share/opto/gcm.cpp:276),
pid=34522, tid=43267
#  assert(false) failed: unscheduable graph
#
# JRE version: Java(TM) SE Runtime Environment (15.0) (fastdebug build
15-internal+0-2020-02-19-1905201.christoph.langer.source)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (fastdebug
15-internal+0-2020-02-19-1905201.christoph.langer.source, mixed mode,
sharing, tiered, compressed oops, g1 gc, bsd-amd64)
# Core dump will be written. Default location: core.34522
#
# If you would like to submit a bug report, please visit:
#   https://bugreport.java.com/bugreport/crash.jsp
#

---  S U M M A R Y 

Command Line:
-Denv.class.path=/scratch/mesos/slaves/90726e33-be99-4e27-9d68-
25dad266ef13-S80/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-
0196/executors/fb2f58a7-7e16-4340-8783-4207c1d11742/runs/3ca5d907-
2207-49d1-a673-f3617d83173d/testoutput/test-
support/jtreg_open_test_hotspot_jtreg_tier1_serviceability/classes/1/servi
ceability/sa/ClhsdbCDSJstackPrintAll.d:/scratch/mesos/jib-
master/install/2020-02-19-
1905201.christoph.langer.source/src.full/open/test/hotspot/jtreg/serviceabil
ity/sa:/scratch/mesos/slaves/90726e33-be99-4e27-9d68-25dad266ef13-
S80/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-
0196/executors/fb2f58a7-7e16-4340-8783-4207c1d11742/runs/3ca5d907-
2207-49d1-a673-f3617d83173d/testoutput/test-
support/jtreg_open_test_hotspot_jtreg_tier1_serviceability/classes/1/test/
lib:/scratch/mesos/jib-master/install/2020-02-19-
1905201.christoph.langer.source/src.full/open/test/lib:/scratch/mesos/jib-
master/install/java/re/jtreg/4.2/promoted/all/b16/bundles/jtreg_bin-
4.2.zip/jtreg/lib/javatest.jar:/scratch/mesos/jib-
master/install/java/re/jtreg/4.2/promoted/all/b16/bundles/jtreg_bin-
4.2.zip/jtreg/lib/jtreg.jar
-Dapplication.home=/scratch/mesos/jib-master/install/2020-02-19-
1905201.christoph.langer.source/macosx-x64-debug.jdk/jdk-15/fastdebug
-Xms8m -Djdk.module.main=jdk.hotspot.agent
jdk.hotspot.agent/sun.jvm.hotspot.SALauncher clhsdb --pid=34506

Host: scaaa915.us.oracle.com, MacPro6,1 x86_64 3700 MHz, 8 cores, 16G,
Darwin 17.5.0
Time: Wed Feb 19 19:49:38 2020 GMT elapsed time: 5 seconds (0d 0h 0m 5s)

---  T H R E A D  ---

Current thread (0x7f9158008800):  JavaThread "C2 CompilerThread0"
daemon [_thread_in_native, id=43267,
stack(0x77ca8000,0x77da8000)]


Current CompileTask:
C2:   5392  726   !   4
jdk.internal.reflect.GeneratedConstructorAccessor3::newInstance (53 bytes)

Stack: [0x77ca8000,0x77da8000],  sp=0x77da3b70,
free space=1006k
Native frames: (J=compiled Java code, A=aot compiled Java code,
j=interpreted, Vv=VM code, C=native code)
V  [libjvm.dylib+0xb48133]  VMError::report_and_die(int, char const*,
char const*, __va_list_tag*, Thread*, unsigned char*, void*, void*, char
const*, int, unsigned long)+0x6e5
V  [libjvm.dylib+0xb4884f]  VMError::report_and_die(Thread*, void*, char
const*, int, char const*, char const*, __va_list_tag*)+0x47
V  [libjvm.dylib+0x338454]  report_vm_error(char const*, int, char
const*, char const*, ...)+0x145

Re: RFR(M): 8233787: Break cycle in vm_version* includes

[Daniel D. Daugherty]

Adding build-dev@... since this changeset now touches
make/hotspot/lib/CompileJvm.gmk. The Build team has a standing
request to be included on reviews that touch makefiles...

Dan


On 11/14/19 11:26 AM, Schmidt, Lutz wrote:

Hi Kim,

that wasn't straightforward. Had to adapt make/hotspot/lib/CompileJvm.gmk. 
Build settings like HOTSPOT_VERSION_STRING have to flow into the compile step 
of abstract_vm_version.cpp now. For the details, see my comments below.

Other than that, I hope the new webrev is even closer to your dreams:
http://cr.openjdk.java.net/~lucy/webrevs/8233787.02/

Thanks,
Lutz


On 14.11.19, 00:34, "Kim Barrett"  wrote:

 > On Nov 13, 2019, at 11:42 AM, Schmidt, Lutz  wrote:
 >
 > Hi Kim,
 >
 > there is a new webrev at 
http://cr.openjdk.java.net/~lucy/webrevs/8233787.01/
 >
 > It should be pretty close to what you view as the "right approach". 
There weren't too many changes relative to 8233787.00. Most files already had #include 
runtime/vm_version.hpp.
 
 This looks much better to me, but many (most?) of the changed

 #includes need to be moved into sort order.

R: tried my best to fix the sort order. Sorry for not paying attention in the 
first place.
 
 --

 src/hotspot/share/runtime/vm_version.cpp
 
 Abstract_VM_Version definitions should be moved to abstract_vm_version.cpp.

 Maybe just rename the file; I think the only thing that would be left
 for vm_version.cpp would be VM_Version_init().  But maybe that should
 be left behind in vm_version.cpp?  Though that makes the review messier.

R: Everything moved as you suggested. Doesn't make sense to have 
Abstract_VM_Version:: methods in vm_version.cpp file.
 
 --

 src/hotspot/share/runtime/abstract_vm_version.hpp
 
 Should #include globalDefinitions.hpp.

 - uint64_t features()
 - #define SUPPORTS_NATIVE_CX8

R: Done.
 
 Should forward-declare class outsputStream.

 - print_virtualization_info
 - print_matching_lines_from_file (I wonder why this is *here*, but not 
your problem)

R: Done.
 
 --
 
 
 

Re: RFR: 8233285: Demangling C++ symbols in jhsdb jstack --mixed

[Daniel D. Daugherty]

Since this review contains build changes, I've added build-dev@...

Dan


On 11/1/19 4:56 AM, Yasumasa Suenaga wrote:

(Changed subject to review request)

Hi,

I converted LinuxDebuggerLocal.c to C++ code, and it works fine on 
submit repo.

(mach5-one-ysuenaga-JDK-8233285-1-20191101-0746-6336009)

  http://cr.openjdk.java.net/~ysuenaga/JDK-8233285/webrev.00/

Could you review it?


Thanks,

Yasumasa


On 2019/11/01 8:54, Yasumasa Suenaga wrote:

Hi David,

On 2019/11/01 7:55, David Holmes wrote:

Hi Yasumasa,

New build dependencies cannot be added lightly. This impacts 
everyone who maintains build/test farms.


Ok, thanks for telling it.

We already use the C++ demangling capabilities in the VM. Is there 
some way to export that for use by libsaproc ?


Otherwise using C++ demangle may still be the better choice given we 
already have it as a dependency.


I found abi::__cxa_demangle() is used in ElfDecoder::demangle() at 
decoder_linux.cpp .

It is similar with my original proposal.


http://cr.openjdk.java.net/~ysuenaga/sa-demangle/


I agree with David to use C++ demangle way.
However we need to choice the fix from following:

   A. Convert LinuxDebuggerLocal.c to C++ code
   B. Add C++ code for libsaproc.so to demangle symbols.

I've discussed with Chris about it in [1].
Option A might be large change.


Thanks,

Yasumasa


[1] 
https://mail.openjdk.java.net/pipermail/serviceability-dev/2019-October/029716.html




David

On 1/11/2019 12:58 am, Chris Plummer wrote:

Hi Yasumasa,

Here's the failure during configure:

[2019-10-31T06:07:45,131Z] checking demangle.h usability... no
[2019-10-31T06:07:45,150Z] checking demangle.h presence... no
[2019-10-31T06:07:45,150Z] checking for demangle.h... no
[2019-10-31T06:07:45,151Z] configure: error: Could not find 
demangle.h! You might be able to fix this by running 'sudo yum 
install binutils-devel'.


Chris


On 10/31/19 1:08 AM, Yasumasa Suenaga wrote:

Hi,

I filed this enhancement to JBS:

  https://bugs.openjdk.java.net/browse/JDK-8233285

Also I pushed the changes to submit repo, but it was failed.

  http://hg.openjdk.java.net/jdk/submit/rev/bfbc49233c26
  http://hg.openjdk.java.net/jdk/submit/rev/430e4f65ef25

According to the email from Mach 5, dependency errors were 
occurred in jib.

Can someone share the details?
I'm not familiar in jib, so I want help.

  mach5-one-ysuenaga-JDK-8233285-20191031-0606-6301426


Thanks,

Yasumasa


On 2019/10/31 11:23, Chris Plummer wrote:
You can change the configure script. I don't know if there's any 
concerns with using libiberty.a. That's possibly a legal question 
(GNU GPL). You might want to ask that on jdk-dev and/or build-dev.


Chris

On 10/30/19 7:14 PM, Yasumasa Suenaga wrote:

Hi Chris,

Thanks for quick reply!

If we convert LinuxDebuggerLocal.c to C++ code, we have to 
convert a lot of JNI calls to C++ style.

For example:

  (*env)->FindClass(env, "java/lang/String")
  to
  env->FindClass("java/lang/String")

Can it be accepted?

OTOH I said in my email, to use cplus_demangle(), we need to 
link libiberty.a which is provided by binutils. Thus I think we 
need to check libiberty.a in configure script. Is it ok?



I prefer to use cplus_demangle() if we can change configure script.


Yasumasa


On 2019/10/31 11:03, Chris Plummer wrote:

Hi Yasumasa,

I don't have concerns with adding C++ source to SA, but in 
order to do so you've put the new native code in its own file 
rather than in LinuxDebuggerLocal.c. I'd like to see that 
resolved. So either convert LinuxDebuggerLocal.c to C++, or use 
cplus_demangle().


thanks,

Chris

On 10/30/19 6:54 PM, Yasumasa Suenaga wrote:

Hi all,

I saw C++ frames in `jhsdb jstack --mixed`, and they were 
mangled as below:



0x7ff255a8fa4c 
_ZN9JavaCalls11call_helperEP9JavaValueRK12methodHandleP17JavaCallArgumentsP6Thread 
+ 0x6ac
0x7ff255a8cc1d 
_ZN9JavaCalls12call_virtualEP9JavaValueP5KlassP6SymbolS5_P17JavaCallArgumentsP6Thread 
+ 0x33d



We can demangle them via c++filt, but I think it is more 
convenience if jstack can show demangling symbols.
I think we can demangle in jstack with this patch. If it is 
accepted, I will file it to JBS and send review request.

What do you think?

http://cr.openjdk.java.net/~ysuenaga/sa-demangle/

We can get the stack as below after applying this patch:


0x7ff1aba20a4c JavaCalls::call_helper(JavaValue*, 
methodHandle const&, JavaCallArguments*, Thread*) + 0x6ac
0x7ff1aba1dc1d JavaCalls::call_virtual(JavaValue*, Klass*, 
Symbol*, Symbol*, JavaCallArguments*, Thread*) + 0x33d



I use abi::__cxa_demangle() for demangling, so this patch adds 
C++ source to SA.

If it is not comfortable, we can use cplus_demangle().
But this function is provided by libiberty.a, so we need to 
link it to libsaproc and need to check libiberty.a in 
configure script.



Thanks,

Yasumasa

Re: RFR (XXS) 8224790: Remove Xusage.txt file

[Daniel D. Daugherty]

On 5/25/19 1:49 AM, David Holmes wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8224790
webrev: http://cr.openjdk.java.net/~dholmes/8224790/webrev/


make/hotspot/gensrc/GenerateSources.gmk
    No comments.

src/hotspot/share/Xusage.txt
    Deleted file.

Thumbs up!

When I did a sanity check grep for Xusage.txt I was surprised that
it "showed up" in binary files. For example:

Binary file 
./build/macosx-x86_64-normal-server-release/images/jdk/jmods/java.base.jmod 
matches


Hopefully that's an artifact of the build process?!?!?

Dan




Before Java 7 the Xusage.txt file was used to print the "java -X" help 
information in English. From Java 7 this information was provided in 
localised format by the launcher. The Xusage.txt file was not removed 
at the time because it was also used by the gamma launcher. But the 
gamma launcher was removed in java 8 - JDK-8008772. The information in 
Xusage.txt has become stale and incomplete (despite several edits post 
Java 7). It should just be removed.


Also removed the build logic that copied the file into the JDK image.

Thanks,
David

Re: test/hotspot/jtreg/runtime/CompressedOops/UseCompressedOops.java failing with -XX:+UseShenandoahGC on x86_64

[Daniel D. Daugherty]

It will get pushed to jdk/jdk (for JDK13) on Jesper's next
sync from jdk/jdk12 -> jdk/jdk.

Dan


On 1/28/19 10:20 AM, B. Blaser wrote:

I've seen this has just been pushed to 12 but I think we'd need this
to 13 too, but:

https://bugs.openjdk.java.net/browse/JDK-8217873

has been resolved as duplicate.

I can help reopening it and pushing the fix to 13 if necessary?

Thanks,
Bernard

On Sat, 26 Jan 2019 at 16:47, B. Blaser  wrote:

Hi,

Maybe you're already aware of this failing test, but it seems that
-XX:+UnlockExperimentalVMOptions is necessary with
-XX:+UseShenandoahGC as here under.

Regards,
Bernard

diff --git a/test/hotspot/jtreg/runtime/CompressedOops/UseCompressedOops.java
b/test/hotspot/jtreg/runtime/CompressedOops/UseCompressedOops.java
--- a/test/hotspot/jtreg/runtime/CompressedOops/UseCompressedOops.java
+++ b/test/hotspot/jtreg/runtime/CompressedOops/UseCompressedOops.java
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -63,7 +63,7 @@
  testCompressedOopsModes(args, "-XX:+UseParallelGC");
  testCompressedOopsModes(args, "-XX:+UseParallelOldGC");
  if (GC.Shenandoah.isSupported()) {
-testCompressedOopsModes(args, "-XX:+UseShenandoahGC");
+testCompressedOopsModes(args,
"-XX:+UnlockExperimentalVMOptions", "-XX:+UseShenandoahGC");
  }
  }

Re: [12] RFR for JDK-8214122: JDWP is broken on 32 bit Windows: transport library missing onLoad entry

[Daniel D. Daugherty]

On 12/19/18 9:23 AM, Alexey Ivanov wrote:

Any volunteers from core-libs and serviceability to review?


How about former Serviceability Team members? :-) Alan B. and I
both used to be on the Serviceability Team... And Alan B. is a
current member of Core Libs...

> webrev: http://cr.openjdk.java.net/~aivanov/8214122/webrev.01/

src/jdk.jdwp.agent/share/native/libjdwp/transport.c
    No comments.

The code appears to do what is described in this (very long) review
thread. I don't believe we (Oracle) have any Win32 test setups for
the jdk/jdk repo so this isn't a change that someone from Oracle can
do additional testing on.

Thumbs up!

Dan




Regards,
Alexey

On 12/12/2018 16:43, Magnus Ihse Bursie wrote:


On 2018-12-12 17:38, Alexey Ivanov wrote:

Hi all,

I have updated the summary of JDK-8214122 and the subject accordingly.

Could you please review the following fix?

https://bugs.openjdk.java.net/browse/JDK-8214122
webrev: http://cr.openjdk.java.net/~aivanov/8214122/webrev.01/

Looks good to me.

/Magnus


*Root cause*
jdwpTransport_OnLoad is exported as _jdwpTransport_OnLoad@16 on 32 
bit Windows according to the name decorations [1] for __stdcall [2] 
calling conventions.


*Fix*
On 32 bit Windows, look up the decorated name, 
_jdwpTransport_OnLoad@16, first; if not found, look up the 
undecorated name, jdwpTransport_OnLoad.



Regards,
Alexey

[1] 
https://docs.microsoft.com/en-us/cpp/build/reference/decorated-names?view=vs-2017#FormatC

[2] https://docs.microsoft.com/en-ie/cpp/cpp/stdcall?view=vs-2017

On 12/12/2018 11:19, Magnus Ihse Bursie wrote:



On 2018-12-11 18:16, Alexey Ivanov wrote:

Hi Simon,

Thank you for your comments.

The updated webrev:
http://cr.openjdk.java.net/~aivanov/8214122/webrev.01/

Indeed, it looks much cleaner.

Yes! This seems like the correct fix.

Ship it! :)

/Magnus



Regards,
Alexey

On 11/12/2018 16:43, Simon Tooke wrote:

On 2018-12-11 10:05 a.m., Alexey Ivanov wrote:

Hi everyone,

I came up with the following patch:
http://cr.openjdk.java.net/~aivanov/8214122/webrev.00/

It specifically addresses the problem in JDK-8214122 where on 32 
bit

Windows jdwpTransport_OnLoad can exported with its plain and
__stdcall-mangled name. I used conditional compilation so that for
other platforms the code remains as it is now.

jshell starts successfully with this fix; an IDE debugger works 
as well.



I am not a reviewer, but this patch only works for the specific case
under discussion; the '@16' refers to the reserved stack size in the
Pascal calling convention.  So, the patch only works for 16 bytes of
parameters.  To be generic, the routine needs to have the stack size
passed in by the caller.  If a generic fix isn't desired (and I 
hope it

is), I'd prefer to see the caller simply pass the decorated or
undecorated name depending on the Win32/64 defines.

 #if defined(_WIN32) && !defined(_WIN64) onLoad =
 (jdwpTransport_OnLoad_t) dbgsysFindLibraryEntry(handle,
 "_jdwpTransport_OnLoad@16"); #else onLoad = 
(jdwpTransport_OnLoad_t)

 dbgsysFindLibraryEntry(handle, "jdwpTransport_OnLoad"); #endif


Thanks,
-Simon


Regards,
Alexey

https://bugs.openjdk.java.net/browse/JDK-8214122

On 10/12/2018 15:11, Magnus Ihse Bursie wrote:
Since removing JNICALL is not an option, there are only two 
options:


1. Add |/export| option to the Makefile or pragma-comment to the
source file;
2. Lookup the decorated name |_jdwpTransport_OnLoad@16| for Win32
with fallback to undecorated one.

Yes.

I think the correct solution here is 2.

Re: RFR: JDK-6657100 Rename sparcWorks to solstudio in HotSpot

[Daniel D. Daugherty]

On 8/30/18 8:36 AM, Magnus Ihse Bursie wrote:

Hi folks,

This bug report goes all the way back to 2008. :-) So instead of just 
letting it go on to it's second decade of existence, I went about and 
fixed it.


By now, not many references to sparcWorks exists anymore, so this is 
almost trivial.


I've verified by grep that no more "sparcworks" are present in the 
repo, neither in hotspot nor anywhere else.


Bug: https://bugs.openjdk.java.net/browse/JDK-6657100
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-6657100-rename-sparcWorks-to-solstudio/webrev.01


make/autoconf/toolchain.m4
    No comments.

make/nb_native/nbproject/configurations.xml
    No comments.

src/hotspot/share/interpreter/bytecodeHistogram.hpp
    No comments.

src/hotspot/share/runtime/vmStructs.hpp
    No comments.

src/hotspot/share/utilities/count_trailing_zeros.hpp
    No comments.

src/java.base/solaris/native/libjvm_db/libjvm_db.c
    No comments.

src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/types/basic/BasicTypeDataBase.java
    No comments.

src/hotspot/share/utilities/globalDefinitions_solstudio.hpp 
src/hotspot/share/utilities/globalDefinitions_sparcWorks.hpp

    No comments.

Thumbs up!

It's nice to see this one get resolved. Thanks!

Dan




/Magnus

Re: RFR: JDK-8202384: Introduce altserver jvm variant with speculative execution disabled

[Daniel D. Daugherty]

On 6/8/18 5:50 PM, Erik Joelsson wrote:

On 2018-06-07 17:30, David Holmes wrote:

On 8/06/2018 6:11 AM, Erik Joelsson wrote:
I just don't think the extra work is warranted or should be 
prioritized at this point. I also cannot think of a combination of 
options required for what you are suggesting that wouldn't be 
confusing to the user. If someone truly feels like these flags are 
forced on them and can't live with them, we or preferably that 
person can fix it then. I don't think that's dictatorship. OpenJDK 
is still open source and anyone can contribute.


I don't see why --enable-hardened-jdk and --enable-hardened-hotspot 
to add to the right flags would be either complicated or confusing.


For me the confusion surrounds the difference between 
--enable-hardened-hotspot and --with-jvm-variants=server, hardened and 
making the user understand it. But sure, it is doable. Here is a new 
webrev with those two options as I interpret them. Here is the help text:


 --enable-hardened-jdk   enable hardenening compiler flags for all jdk


Typo: 'hardenening' -> 'hardening'



libraries (except the JVM), typically disabling
  speculative cti. [disabled]
 --enable-hardened-hotspot
  enable hardenening compiler flags for 
hotspot (all


Typo: 'hardenening' -> 'hardening'



jvm variants), typically disabling speculative cti.
  To make hardening of hotspot a runtime choice,
  consider the "hardened" jvm variant instead 
of this

  option. [disabled]

Note that this changes the default for jdk libraries to not enable 
hardening unless the user requests it.


Webrev: http://cr.openjdk.java.net/~erikj/8202384/webrev.04/

/Erik


Dan

Re: RFR(XXXS): 8193407: jdk/hs fails Solaris slowdebug test-image build

[Daniel D. Daugherty]

On 12/13/17 4:01 AM, Erik Joelsson wrote:

On 2017-12-13 03:19, Daniel D. Daugherty wrote:

On 12/12/17 9:16 PM, David Holmes wrote:

Hi Dan,

On 13/12/2017 12:13 PM, Daniel D. Daugherty wrote:

Greetings,

I have a 1-liner fix to solve a Solaris slowdebug test-image build
problem:

 JDK-8193407 jdk/hs fails Solaris slowdebug test-image build
 https://bugs.openjdk.java.net/browse/JDK-8193407

Here is the context diff:

$ hg diff
diff -r a576e1b6784d make/test/JtregNativeHotspot.gmk
--- a/make/test/JtregNativeHotspot.gmk    Tue Dec 12 14:14:06 2017 
-0500
+++ b/make/test/JtregNativeHotspot.gmk    Tue Dec 12 21:07:03 2017 
-0500

@@ -113,6 +113,7 @@
BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libHandshakeTransitionTest := -lc
  BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libHasNoEntryPoint := -lc
  BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libReturnError := -lc
+    BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libCNLookUp := -lc
  endif

  ifeq ($(OPENJDK_TARGET_OS), linux)


David Holmes suggested the possible fix and I have tested the fix with
Solaris X64 'release', 'fastdebug' and 'slowdebug' builds.


Thanks for that!


David is the first (R)eviewer. Since this affects a Compiler team
test, it would be good to hear from someone on the Compiler team.
Since it is a Makefile change, it would be good to hear from someone
on the build-dev@... alias...


I think that is a bit of overkill. I was going to say push this 
under trivial rules!


No problem. I'm just trying to be polite... :-)

I'll go ahead and push it shortly... I'll wait long enough to clear my
email queue and we'll see if anyone else chimes in... :-)

I agree that it's trivial, but appreciate the cross post to build-dev 
so that we are in the loop.


Thanks for the review!

Dan




/Erik


Aside: I've started a separate email thread on why we need to list 
-lc explicitly in the makefile.


Yup. It would be good to get rid of that if we can...

Dan




Thanks,
David


Thanks, in advance, for questions, comments, or suggestions.

Dan

Re: RFR(XXXS): 8193407: jdk/hs fails Solaris slowdebug test-image build

[Daniel D. Daugherty]

On 12/12/17 9:16 PM, David Holmes wrote:

Hi Dan,

On 13/12/2017 12:13 PM, Daniel D. Daugherty wrote:

Greetings,

I have a 1-liner fix to solve a Solaris slowdebug test-image build
problem:

 JDK-8193407 jdk/hs fails Solaris slowdebug test-image build
 https://bugs.openjdk.java.net/browse/JDK-8193407

Here is the context diff:

$ hg diff
diff -r a576e1b6784d make/test/JtregNativeHotspot.gmk
--- a/make/test/JtregNativeHotspot.gmk    Tue Dec 12 14:14:06 2017 -0500
+++ b/make/test/JtregNativeHotspot.gmk    Tue Dec 12 21:07:03 2017 -0500
@@ -113,6 +113,7 @@
BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libHandshakeTransitionTest := -lc
  BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libHasNoEntryPoint := -lc
  BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libReturnError := -lc
+    BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libCNLookUp := -lc
  endif

  ifeq ($(OPENJDK_TARGET_OS), linux)


David Holmes suggested the possible fix and I have tested the fix with
Solaris X64 'release', 'fastdebug' and 'slowdebug' builds.


Thanks for that!


David is the first (R)eviewer. Since this affects a Compiler team
test, it would be good to hear from someone on the Compiler team.
Since it is a Makefile change, it would be good to hear from someone
on the build-dev@... alias...


I think that is a bit of overkill. I was going to say push this under 
trivial rules!


No problem. I'm just trying to be polite... :-)

I'll go ahead and push it shortly... I'll wait long enough to clear my
email queue and we'll see if anyone else chimes in... :-)


Aside: I've started a separate email thread on why we need to list -lc 
explicitly in the makefile.


Yup. It would be good to get rid of that if we can...

Dan




Thanks,
David


Thanks, in advance, for questions, comments, or suggestions.

Dan

Re: RFR(XXXS): 8193407: jdk/hs fails Solaris slowdebug test-image build

[Daniel D. Daugherty]

Vladimir,

Thanks for the review!

Dan


On 12/12/17 9:24 PM, Vladimir Kozlov wrote:

Reviewed. Looks good.

Thanks,
Vladimir

On 12/12/17 6:16 PM, David Holmes wrote:

Hi Dan,

On 13/12/2017 12:13 PM, Daniel D. Daugherty wrote:

Greetings,

I have a 1-liner fix to solve a Solaris slowdebug test-image build
problem:

 JDK-8193407 jdk/hs fails Solaris slowdebug test-image build
 https://bugs.openjdk.java.net/browse/JDK-8193407

Here is the context diff:

$ hg diff
diff -r a576e1b6784d make/test/JtregNativeHotspot.gmk
--- a/make/test/JtregNativeHotspot.gmk    Tue Dec 12 14:14:06 2017 
-0500
+++ b/make/test/JtregNativeHotspot.gmk    Tue Dec 12 21:07:03 2017 
-0500

@@ -113,6 +113,7 @@
BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libHandshakeTransitionTest := -lc
  BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libHasNoEntryPoint := -lc
  BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libReturnError := -lc
+    BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libCNLookUp := -lc
  endif

  ifeq ($(OPENJDK_TARGET_OS), linux)


David Holmes suggested the possible fix and I have tested the fix with
Solaris X64 'release', 'fastdebug' and 'slowdebug' builds.


Thanks for that!


David is the first (R)eviewer. Since this affects a Compiler team
test, it would be good to hear from someone on the Compiler team.
Since it is a Makefile change, it would be good to hear from someone
on the build-dev@... alias...


I think that is a bit of overkill. I was going to say push this under 
trivial rules!


Aside: I've started a separate email thread on why we need to list 
-lc explicitly in the makefile.


Thanks,
David


Thanks, in advance, for questions, comments, or suggestions.

Dan

RFR(XXXS): 8193407: jdk/hs fails Solaris slowdebug test-image build

[Daniel D. Daugherty]

Greetings,

I have a 1-liner fix to solve a Solaris slowdebug test-image build
problem:

    JDK-8193407 jdk/hs fails Solaris slowdebug test-image build
    https://bugs.openjdk.java.net/browse/JDK-8193407

Here is the context diff:

$ hg diff
diff -r a576e1b6784d make/test/JtregNativeHotspot.gmk
--- a/make/test/JtregNativeHotspot.gmk    Tue Dec 12 14:14:06 2017 -0500
+++ b/make/test/JtregNativeHotspot.gmk    Tue Dec 12 21:07:03 2017 -0500
@@ -113,6 +113,7 @@
 BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libHandshakeTransitionTest := -lc
 BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libHasNoEntryPoint := -lc
 BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libReturnError := -lc
+    BUILD_HOTSPOT_JTREG_LIBRARIES_LIBS_libCNLookUp := -lc
 endif

 ifeq ($(OPENJDK_TARGET_OS), linux)


David Holmes suggested the possible fix and I have tested the fix with
Solaris X64 'release', 'fastdebug' and 'slowdebug' builds.

David is the first (R)eviewer. Since this affects a Compiler team
test, it would be good to hear from someone on the Compiler team.
Since it is a Makefile change, it would be good to hear from someone
on the build-dev@... alias...

Thanks, in advance, for questions, comments, or suggestions.

Dan

Re: RFR: JDK-8190418 Stop generating jvmtiEnvStub and jvmtiEnvRecommended

[Daniel D. Daugherty]

Adding Robert Field...  Those files were created before I started doing
any work on JVM/TI...

Dan

On 11/14/17 4:55 PM, David Holmes wrote:

Hi Magnus,

On 14/11/2017 11:06 PM, Magnus Ihse Bursie wrote:
During hotspot gensrc, we create two files, jvmtiEnvStub and 
jvmtiEnvRecommended, which later on are explicitly excluded from the 
build. We should just stop generating them.


cc'd Dan: Need someone who knows to chime in with what these files are 
and why they exist. The stub is like a dummy implementation that may 
have been useful for testing against. No idea what the "recommended" 
file may represent. ??


I have built hotspot with and without this patch, and used the build 
comparison script (compare.sh) to verify that it has not changed.


Bug: https://bugs.openjdk.java.net/browse/JDK-8190418
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8190418-stop-generating-unneeded-jvmti-files/webrev.01 



Change itself seems fine. Not sure if jvmtiEnv.xsl is now a dead file 
that can be removed (if so we'll file a hotspot bug for that).


Thanks,
David



/Magnus

Re: RFR (L, tedious again, sorry) 8189610: Reconcile jvm.h and all jvm_md.h between java.base and hotspot

[Daniel D. Daugherty]

So, we have a variety of opinions.   Please open another ticket to 
discuss this. 


Sorry, I didn't realize that this fix pushed yesterday.
I'm not completely caught up on all of the email. It's
not worth another bug report (and more changes).

Dan



On 11/1/17 2:09 PM, coleen.phillim...@oracle.com wrote:



On 11/1/17 12:51 PM, Daniel D. Daugherty wrote:

Just one comment about the location of "jvm.h".


On 10/30/17 8:07 AM, coleen.phillim...@oracle.com wrote:



On 10/28/17 3:46 AM, David Holmes wrote:

On 28/10/2017 3:47 AM, mandy chung wrote:

On 10/27/17 7:08 AM, coleen.phillim...@oracle.com wrote:



On 10/27/17 9:37 AM, David Holmes wrote:


The one file that is needed is a hotspot file - jvm.h defines 
the interface that hotspot exports via jvm.cpp.


If you leave jvm.h in hotspot/prims then a very large chunk of 
your boilerplate changes are not needed. The JDK code doesn't 
care what the name of the directory is - whatever it is just 
gets added as a -I directive (the JDK code will include "jvm.h" 
not "prims/jvm.h" the way hotspot sources do.


This isn't something we want to change back or move again later. 
Whatever we do now we live with.


I think it belongs with jni.h and I think the core libraries 
group would agree.   It seems more natural there than buried in 
the hotspot prims directory.  I guess this is on hold while we 
have this debate.   Sigh.


Actually with -I directives, changing to jvm.h from prims/jvm.h 
would still work.   Maybe we should change the name to jvm.hpp 
since it's jvm.cpp though?   Or maybe just have two divergent 
copies and close this as WNF. 


I also think hotspot/prims is not a good location. 
src/java.base/share/include is a well-defined location for native 
header files.  Maybe internal header files could be placed in 
include/internal but this is a separate issue . I should create an 
issue for jvm.h and jmm.h (I looked at the files under the include 
directory and jvm.h and jmm.h are the only two internal header 
files in the include directory).


Keeping it in prims avoids the need to touch many hotspot files, 
and with no changes needed on the JDK side because we use a -I 
directive to set the include path anyway. This is the exported VM 
interface so it makes sense to me for it to be located in the VM 
sources.


But I'm not going to oppose this either way so it's up to Coleen.


I've already disagreed that this file belongs in 
src/hotspot/share/prims, so the include directive without prims is 
preferred.  This allows putting jvm.h in a new place if/when that is 
agreed upon.


The jvm.h file describes the internal JVM_* API implemented by 
prims/jvm.cpp.
Because this is an internal interface, the jvm.h file would 
traditionally be
co-located with the implementation (jvm.cpp) (and not in an include 
directory).


So I disagree with the proposal to move jvm.h and believe the single 
copy

should be in prims/jvm.h.


So, we have a variety of opinions.   Please open another ticket to 
discuss this.

thanks,
Coleen


Dan




I do think removing the duplicated copy of jvm.h is a good change. 
This is finally possible with the consolidated repository and we 
no longer need to update two copies of jvm.h for any change to the 
JVM 


Unfortunately we did not do this though - hence the divergence 
between the two. The use of int versus long for jint is causing a 
real problem.


Coleen also hit the other issue on the head. The JNI and JVM 
interfaces are C interfaces, not C++. The JDK code that uses them 
is compiled as C - so all good. But the JVM code that implements 
them is compiled as C++, and that is why we are getting issues with 
differing linkage directives.


Well, there is now one source file for jvm.h and jni.h and their 
machine dependent counterparts and 2500 lines of duplicated code is 
removed with this change.  The issues with jint and linkages are 
resolved and tested as well with this changeset.


Thanks,
Coleen


David
-

interface.   This change will work with -I directive setting to 
the new location, if changed later.


What do you think?

Mandy

Re: RFR (L, tedious again, sorry) 8189610: Reconcile jvm.h and all jvm_md.h between java.base and hotspot

[Daniel D. Daugherty]

Just one comment about the location of "jvm.h".


On 10/30/17 8:07 AM, coleen.phillim...@oracle.com wrote:



On 10/28/17 3:46 AM, David Holmes wrote:

On 28/10/2017 3:47 AM, mandy chung wrote:

On 10/27/17 7:08 AM, coleen.phillim...@oracle.com wrote:



On 10/27/17 9:37 AM, David Holmes wrote:


The one file that is needed is a hotspot file - jvm.h defines the 
interface that hotspot exports via jvm.cpp.


If you leave jvm.h in hotspot/prims then a very large chunk of 
your boilerplate changes are not needed. The JDK code doesn't care 
what the name of the directory is - whatever it is just gets added 
as a -I directive (the JDK code will include "jvm.h" not 
"prims/jvm.h" the way hotspot sources do.


This isn't something we want to change back or move again later. 
Whatever we do now we live with.


I think it belongs with jni.h and I think the core libraries group 
would agree.   It seems more natural there than buried in the 
hotspot prims directory.  I guess this is on hold while we have 
this debate.   Sigh.


Actually with -I directives, changing to jvm.h from prims/jvm.h 
would still work.   Maybe we should change the name to jvm.hpp 
since it's jvm.cpp though?   Or maybe just have two divergent 
copies and close this as WNF. 


I also think hotspot/prims is not a good location. 
src/java.base/share/include is a well-defined location for native 
header files.  Maybe internal header files could be placed in 
include/internal but this is a separate issue .  I should create an 
issue for jvm.h and jmm.h (I looked at the files under the include 
directory and jvm.h and jmm.h are the only two internal header files 
in the include directory).


Keeping it in prims avoids the need to touch many hotspot files, and 
with no changes needed on the JDK side because we use a -I directive 
to set the include path anyway. This is the exported VM interface so 
it makes sense to me for it to be located in the VM sources.


But I'm not going to oppose this either way so it's up to Coleen.


I've already disagreed that this file belongs in 
src/hotspot/share/prims, so the include directive without prims is 
preferred.  This allows putting jvm.h in a new place if/when that is 
agreed upon.


The jvm.h file describes the internal JVM_* API implemented by 
prims/jvm.cpp.

Because this is an internal interface, the jvm.h file would traditionally be
co-located with the implementation (jvm.cpp) (and not in an include 
directory).


So I disagree with the proposal to move jvm.h and believe the single copy
should be in prims/jvm.h.

Dan




I do think removing the duplicated copy of jvm.h is a good change. 
This is finally possible with the consolidated repository and we no 
longer need to update two copies of jvm.h for any change to the JVM 


Unfortunately we did not do this though - hence the divergence 
between the two. The use of int versus long for jint is causing a 
real problem.


Coleen also hit the other issue on the head. The JNI and JVM 
interfaces are C interfaces, not C++. The JDK code that uses them is 
compiled as C - so all good. But the JVM code that implements them is 
compiled as C++, and that is why we are getting issues with differing 
linkage directives.


Well, there is now one source file for jvm.h and jni.h and their 
machine dependent counterparts and 2500 lines of duplicated code is 
removed with this change.  The issues with jint and linkages are 
resolved and tested as well with this changeset.


Thanks,
Coleen


David
-

interface.   This change will work with -I directive setting to the 
new location, if changed later.


What do you think?

Mandy

Re: RFR (2xS): 8181318: Allow C++ library headers on Solaris Studio

[Daniel D. Daugherty]

On 6/5/17 10:59 AM, Erik Osterlund wrote:

Hi Dan,


On 5 Jun 2017, at 18:31, Daniel D. Daugherty  
wrote:

On 6/5/17 10:19 AM, Erik Österlund wrote:
Hi David,


On 2017-06-05 14:45, David Holmes wrote:
Hi Erik,


On 5/06/2017 8:38 PM, Erik Österlund wrote:
Hi David,


On 2017-06-02 03:30, David Holmes wrote:
Hi Erik,


On 2/06/2017 12:50 AM, Erik Österlund wrote:
Hi David,


On 2017-06-01 14:33, David Holmes wrote:
Hi Erik,

Just to be clear it is not the use of  that I am concerned about, it is 
the -library=stlport4. It is the use of that flag that I would want to check in terms 
of having no affect on any existing code generation.

Thank you for the clarification. The use of -library=stlport4 should not have 
anything to do with code generation. It only says where to look for the standard 
library headers such as  that are used in the compilation units.

The potential problem is that the stlport4 include path eg:

./SS12u4/lib/compilers/include/CC/stlport4/

doesn't only contain the C++ headers (new, limits, string etc) but also a whole 
bunch of regular 'standard' .h headers that are _different_ to those found 
outside the stlport4 directory ie the ones we would currently include. I don't 
know if the differences are significant, nor whether those others may be found 
ahead of the stlport4 version. But that is my concern about the effects on the 
code.

While I do not think exchanging these headers will have any behavioral impact, 
I agree that we can not prove so as they are indeed different header files. 
That is a good point.

However, I think that makes the stlport4 case stronger rather than weaker. We 
already use stlport4 for our gtest testing (because it is required and does not 
build without it). And if those headers would indeed have slightly different 
behaviour as you imply, it further motivates using the same standard library 
when compiling the product as the testing code. If they were to behave slightly 
differently, it might be that our gtest tests does not catch hidden bugs that 
only manifest when building with a different set of headers used for the 
product build. I therefore find it exceedingly dangerous to stay on two 
standard libraries (depending on if test code or product code is compiled) 
compared to consistently using the same standard library across all 
compilations. So for me, the larger the risk is of them behaving differently 
is, the bigger the motivation is to use stlport4 consistently.

Regardless of what gtest does if you want to switch the standard libraries used 
by the product then IMHO that should go through a vetting process no weaker 
than that for changing the toolchain, as you effectively are doing that.

I talked to Erik Joelsson about how to compare two builds. He introduced me to 
our compare.sh script that is used to compare two builds.
I built a baseline without these changes and a new build with these changes 
applied, both on a Solaris SPARC T7 machine. Then I compared them with 
./compare.sh -2dirs {$BUILD1}/hotspot/variant-server/libjvm/objs 
{$BUILD2}/hotspot/variant-server/libjvm/objs -libs --strip

This compares the object files produced when compiling hotspot in build 1 and 
build 2 after stripping symbols.

First it reported:
Libraries...
   Size: Symbols :  Deps   : Disass   :
   :* diff  *: :  : ./dtrace.o
   :* diff  *: :*   38918*: ./jni.o
   :* diff  *: :*   23226*: ./unsafe.o

It seems like all symbols were not stripped here on these mentioned files and 
constituted all differences in the disassembly. So I made a simple sed filter to 
filter out symbol names in the disassembly with the regexp <.*>.

The result was:
Libraries...
   Size: Symbols :  Deps   : Disass   :
   :* diff  *: :  : ./dtrace.o
   :* diff  *: :  : ./jni.o
   :* diff  *: :  : ./unsafe.o

This shows that not a single instruction was emitted differently between the 
two builds.

I also did the filtering manually on jni.o and unsafe.o in emacs to make sure I 
did not mess up.

Are we happy with this, or do you still have doubts that this might result in 
different code or behavior?

Just to be clear: The current experiment changes both the header and
the standard library right? If so, then the compare.sh run works for
validating that using the new header file will not result in a change
in behavior. However, that comparison doesn't do anything for testing
a switch in the standard libraries right?

The -xnolib guards are still there in the LDFLAGS. That is, the linker will not 
allow anything to link against either standard library. I have manually 
confirmed this by doing the sanity check of comparing the NEEDED entries in the 
dynamic section of the libjvm.so elf file using elfdump. It has no references 
to neither libstlport4 nor libCstd with or without my changes.

Summary: the changes do not add any 

Re: RFR (2xS): 8181318: Allow C++ library headers on Solaris Studio

[Daniel D. Daugherty]

On 6/5/17 10:19 AM, Erik Österlund wrote:

Hi David,

On 2017-06-05 14:45, David Holmes wrote:

Hi Erik,

On 5/06/2017 8:38 PM, Erik Österlund wrote:

Hi David,

On 2017-06-02 03:30, David Holmes wrote:

Hi Erik,

On 2/06/2017 12:50 AM, Erik Österlund wrote:

Hi David,

On 2017-06-01 14:33, David Holmes wrote:

Hi Erik,

Just to be clear it is not the use of  that I am 
concerned about, it is the -library=stlport4. It is the use of 
that flag that I would want to check in terms of having no affect 
on any existing code generation.


Thank you for the clarification. The use of -library=stlport4 
should not have anything to do with code generation. It only says 
where to look for the standard library headers such as  
that are used in the compilation units.


The potential problem is that the stlport4 include path eg:

./SS12u4/lib/compilers/include/CC/stlport4/

doesn't only contain the C++ headers (new, limits, string etc) but 
also a whole bunch of regular 'standard' .h headers that are 
_different_ to those found outside the stlport4 directory ie the 
ones we would currently include. I don't know if the differences 
are significant, nor whether those others may be found ahead of the 
stlport4 version. But that is my concern about the effects on the 
code.


While I do not think exchanging these headers will have any 
behavioral impact, I agree that we can not prove so as they are 
indeed different header files. That is a good point.


However, I think that makes the stlport4 case stronger rather than 
weaker. We already use stlport4 for our gtest testing (because it is 
required and does not build without it). And if those headers would 
indeed have slightly different behaviour as you imply, it further 
motivates using the same standard library when compiling the product 
as the testing code. If they were to behave slightly differently, it 
might be that our gtest tests does not catch hidden bugs that only 
manifest when building with a different set of headers used for the 
product build. I therefore find it exceedingly dangerous to stay on 
two standard libraries (depending on if test code or product code is 
compiled) compared to consistently using the same standard library 
across all compilations. So for me, the larger the risk is of them 
behaving differently is, the bigger the motivation is to use 
stlport4 consistently.


Regardless of what gtest does if you want to switch the standard 
libraries used by the product then IMHO that should go through a 
vetting process no weaker than that for changing the toolchain, as 
you effectively are doing that.


I talked to Erik Joelsson about how to compare two builds. He 
introduced me to our compare.sh script that is used to compare two 
builds.
I built a baseline without these changes and a new build with these 
changes applied, both on a Solaris SPARC T7 machine. Then I compared 
them with ./compare.sh -2dirs 
{$BUILD1}/hotspot/variant-server/libjvm/objs 
{$BUILD2}/hotspot/variant-server/libjvm/objs -libs --strip


This compares the object files produced when compiling hotspot in 
build 1 and build 2 after stripping symbols.


First it reported:
Libraries...
   Size: Symbols :  Deps   : Disass   :
   :* diff  *: :  : ./dtrace.o
   :* diff  *: :*   38918*: ./jni.o
   :* diff  *: :*   23226*: ./unsafe.o

It seems like all symbols were not stripped here on these mentioned 
files and constituted all differences in the disassembly. So I made a 
simple sed filter to filter out symbol names in the disassembly with 
the regexp <.*>.


The result was:
Libraries...
   Size: Symbols :  Deps   : Disass   :
   :* diff  *: :  : ./dtrace.o
   :* diff  *: :  : ./jni.o
   :* diff  *: :  : ./unsafe.o

This shows that not a single instruction was emitted differently 
between the two builds.


I also did the filtering manually on jni.o and unsafe.o in emacs to 
make sure I did not mess up.


Are we happy with this, or do you still have doubts that this might 
result in different code or behavior?


Just to be clear: The current experiment changes both the header and
the standard library right? If so, then the compare.sh run works for
validating that using the new header file will not result in a change
in behavior. However, that comparison doesn't do anything for testing
a switch in the standard libraries right?

Dan




Thanks,
/Erik


Cheers,
David



Thanks,
/Erik


Thanks,
David
-



Specifically, the man pages for CC say:


-library=lib[,lib...]

Incorporates  specified  CC-provided libraries into 
compilation and

linking.

When the -library option is used to specify a 
CC-provided library,
the  proper  -I paths are set during compilation and 
the proper -L,
-Y, -P, and -R paths and -l options are set during 
linking.



As we are setting this during compilatio

Re: (10) (M) RFR: 8174231: Factor out and share PlatformEvent and Parker code for POSIX systems

[Daniel D. Daugherty]

On 5/18/17 12:25 AM, David Holmes wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8174231

webrevs:

Build-related: http://cr.openjdk.java.net/~dholmes/8174231/webrev.top/


General comment(s):
  - Sometimes you've updated the copyright year for the file and
sometimes you haven't. Please check before pushing.


common/autoconf/flags.m4
No comments.

common/autoconf/generated-configure.sh
No comments.



hotspot: http://cr.openjdk.java.net/~dholmes/8174231/webrev.hotspot/


src/os/aix/vm/os_aix.cpp
No comments; did not try to compare deleted code with os_posix.cpp.

src/os/aix/vm/os_aix.hpp
No comments; did not try to compare deleted code with os_posix.hpp.

src/os/bsd/vm/os_bsd.cpp
No comments; compared deleted code with os_posix.cpp version; nothing
jumped out as wrong.

src/os/bsd/vm/os_bsd.hpp
No comments; compared deleted code with os_posix.hpp version; nothing
jumped out as wrong.

src/os/linux/vm/os_linux.cpp
No comments; compared deleted code with os_posix.cpp version; nothing
jumped out as wrong.

src/os/linux/vm/os_linux.hpp
No comments; compared deleted code with os_posix.hpp version; nothing
jumped out as wrong.

src/os/posix/vm/os_posix.cpp
L1401: // Not currently usable by Solaris
L1408: // time-of-day clock
nit - needs period at end of the sentence

L1433: // build time support then there can not be
typo - "can not" -> "cannot"

L1435: // int or int64_t.
typo - needs a ')' before the period.

L1446: // determine what POSIX API's are present and do appropriate
L1447: // configuration
nits - 'determine' -> 'Determine'
 - needs period at end of the sentence

L1455:   // 1. Check for CLOCK_MONOTONIC support
nit - needs period at end of the sentence

L1462:   // we do dlopen's in this particular order due to bug in linux
L1463:   // dynamical loader (see 6348968) leading to crash on exit
nits - 'we' -> 'We'
 - needs period at end of the sentence

typo - 'dynamical' -> 'dynamic'

L1481: // we assume that if both clock_gettime and clock_getres 
support
L1482: // CLOCK_MONOTONIC then the OS provides true high-res 
monotonic clock

nits - 'we' -> 'We'
 - needs period at end of the sentence

L1486: clock_gettime_func(CLOCK_MONOTONIC, &tp) == 0) {
nit - extra space before '=='

L1487:   // yes, monotonic clock is supported
nits - 'yes' -> 'Yes'
 - needs period at end of the sentence

L1491:   // close librt if there is no monotonic clock
nits - 'close' -> 'Close'
 - needs period at end of the sentence

L1499:   // 2. Check for pthread_condattr_setclock support
L1503:   // libpthread is already loaded
L1511:   // Now do general initialization
nit - needs period at end of the sentence

L1591:   if (timeout < 0)
L1592: timeout = 0;
nit - missing braces

L1609:   // More seconds than we can add, so pin to max_secs
L1658: // More seconds than we can add, so pin to max_secs
nit - needs period at end of the sentence

L1643: // Absolue seconds exceeds allow max, so pin to max_secs
typo - 'Absolue' -> 'Absolute'
nit - needs period at end of the sentence

src/os/posix/vm/os_posix.hpp
L149:   ~PlatformEvent() { guarantee(0, "invariant"); }
L185:   ~PlatformParker() { guarantee(0, "invariant"); }
nit - '0' should be 'false' or just call fatal()

src/os/solaris/vm/os_solaris.cpp
No comments.

src/os/solaris/vm/os_solaris.hpp
No comments.


As Robbin said, this is very hard to review and be sure that everything
is relocated correctly. I tried to look at this code a couple of different
ways and nothing jumped out at me as wrong.

I did my usual crawl style review through posix.cpp and posix.hpp. I only
found nits and typos that you can chose to ignore since you're on a time
crunch here.

Thumbs up!

Dan





First a big thank you to Thomas Stuefe for testing various versions of 
this on AIX.


This is primarily a refactoring and cleanup exercise (ie lots of 
deleted duplicated code!).


I have taken the PlatformEvent, PlatformParker and Parker::* code, out 
of os_linux and moved it into os_posix for use by Linux, OSX, BSD, AIX 
and perhaps one day Solaris (more on that later).


The Linux code was the most functionally complete, dealing with 
correct use of CLOCK_MONOTONIC for relative timed waits, and the 
default wall-clock for absolute timed waits. That functionality is 
not, unfortunately, supported by all our POSIX platforms so there are 
some configure time build checks to set some #defines, and then some 
dynamic lookup at runtime**. We allow for the runtime environment to 
be less capable than the build environment, but not the other way 
around (without build time support we don't know the runtime types 
neede

Re: RFR: JDK-8069540: Remove universal binaries support from hotspot build

[Daniel D. Daugherty]

Historical note: During the MacOS X port project, the "universal binaries"
support was removed at one point (I think Jim Melvin did that work) and
I later restored it. There was grumbling in the community about there only
being 64-bit Server VM support for MacOS X in OpenJDK so I restored the
universal support just in case someone in the community decided to restore
32-bit VM support. That never happened (AFAIK).

Yes, it is time to remove universal support from the HotSpot build.

Dan


On 8/8/16 10:04 AM, Erik Joelsson wrote:

Hello,

The hotspot native libraries on macosx are currently being packaged as 
"universal binaries" (while all other libraries and executables are 
not). There seems to be no point in doing so since we only build for 
one architecture anyway. I believe this is just something that was 
never cleaned up a long time ago and has just been around since. In 
the new hotspot build, we duplicated the behavior with the intention 
of removing it later. That time has now come.


I have verified that a hotspot job for macosx still works.

Bug: https://bugs.openjdk.java.net/browse/JDK-8069540

Webrev: http://cr.openjdk.java.net/~erikj/8069540/webrev.01/

/Erik

Re: RFR: JDK-8161057 Solaris: deprecated/obsolete compiler flags should be removed

[Daniel D. Daugherty]

We have three reviewers on this changeset. We don't have a review
from a current Serviceability team member, but I think Tim and I
can be considered as sufficient review since we're both former
Serviceability team members. :-)

Alan, please confirm that this is good to go.

This change is applied in the repo I have setup for this change:

> jdk/src/demo/share/jvmti/java_crw_demo/sample.makefile.txt
>Copyright year updated to '2015' instead of '2016'.

Dan

On 7/20/16 10:05 AM, Alan Burlison wrote:

On 20/07/2016 03:17, David Holmes wrote:

There were some JPRT test failures on OSX and Windows, as this 
changeset

doesn't affect those platforms I believe they can be ignored.


If this is with "-testset hotspot" then it shouldn't happen, in that
case please drop me an email pointing to the job. For other testsets,
yes they are transient/intermittent failures.


The "-testset hostpot" run was clean, it was "-testset core" that 
failed, with errors in the same areas that I've seen in other 
unrelated runs.

Re: RFR: JDK-8161057 Solaris: deprecated/obsolete compiler flags should be removed

[Daniel D. Daugherty]

On 7/19/16 11:19 AM, Alan Burlison wrote:
This removes redundant/incorrect compilation flags from the Solaris 
build environment.


https://bugs.openjdk.java.net/browse/JDK-8161057
http://cr.openjdk.java.net/~dcubed/for_alanbur/8161057-webrev/0-jdk9-hs-all/ 



I already have one commend from DanD:


jdk/src/demo/share/jvmti/java_crw_demo/sample.makefile.txt
Copyright year updated to '2015' instead of '2016'.


I've already fixed that in my workspace but it's still present in the 
above webrev.


common/autoconf/flags.m4
No comments.

common/autoconf/generated-configure.sh
No comments.

jdk/src/demo/share/jvmti/compiledMethodLoad/sample.makefile.txt
No comments.

jdk/src/demo/share/jvmti/gctest/sample.makefile.txt
No comments.

jdk/src/demo/share/jvmti/heapTracker/sample.makefile.txt
No comments.

jdk/src/demo/share/jvmti/heapViewer/sample.makefile.txt
No comments.

jdk/src/demo/share/jvmti/java_crw_demo/sample.makefile.txt
No additional comments.

jdk/src/demo/share/jvmti/minst/sample.makefile.txt
No comments.

jdk/src/demo/share/jvmti/mtrace/sample.makefile.txt
No comments.

jdk/src/demo/share/jvmti/versionCheck/sample.makefile.txt
No comments.

Thumbs up.

Dan




There were some JPRT test failures on OSX and Windows, as this 
changeset doesn't affect those platforms I believe they can be ignored.


There is some related cleanup in deploy, that will be covered by 
https://bugs.openjdk.java.net/browse/JDK-8161081, by removing the 
Solaris-related scripts.

Re: RFR: JDK-8152666: The new Hotspot Build System

[Daniel D. Daugherty]

> I'm not sure if I'm formally allowed to be a reviewer, since I've
> wrote the absolute majority of the code myself.

The way I've done this in the past is a "Contributed-by:" line listing
all of the folks that contributed and a "Reviewed-by:" line listing all
the reviewers. Magnus, you reviewed Erik's changes and vice versa...

Dan

On 4/7/16 3:57 AM, Magnus Ihse Bursie wrote:

On 2016-04-06 11:10, Erik Joelsson wrote:
Hello Dan and thank you for the review! I know it's a lot to chew 
through.


I have incorporated your changes and published a new webrev:
http://cr.openjdk.java.net/~erikj/8152666/webrev.02/


I'm not sure if I'm formally allowed to be a reviewer, since I've 
wrote the absolute majority of the code myself.


Nevertheless, I've looked through the webrev carefully, including the 
latest changes by you, and it looks good to me. Ship it! :-)


Just a few minor comments:

In compare.sh.in:
Why the added export of DEBUG_LEVEL? I can't find any reference to it 
in the changes in compare.sh. Was it references in some earlier change 
and we missed to export it?


In flags.m4/platform.m4:
It is unfortunate that we needed to make the build/target duplication 
in this change. It makes the messy addition of the JVM_CFLAGS even 
messier. :( But then again, we've always planned a follow-up 
restructuring of the flag handling after the integration of the new 
Hotspot build system. It just got a bit more urgent.


/Magnus




On 2016-04-05 20:10, Daniel D. Daugherty wrote:



> The new build supports the following variants:
>
>  * server (C1+C2)

The above "server" variant is the "tiered server". Does the new
build system support the "C2 server" variant? What about the
32-bit server and 64-bit server build variants? For example,
on Linux you can have:

  * C1/Client, 32-bit
  * C2/Server, 32-bit
  * Tiered (C1 & C2), 32-bit
  * C2/Server, 64-bit
  * Tiered (C1 + C2), 64-bit

The above wide range of variants is also true for Win*.

There is a way to achieve this even if it's not as straight forward. 
It's controlled through the new "jvm-feature" setting. To build a 
completely custom set of features for a jvm, you set the 
--with-jvm-variants=custom and then define the full feature set using 
--with-jvm-features=compiler2,... For "server, client, core, minimal, 
zero and zeroshark" there is a predefined set of features while the 
custom variant has no features by default.


General
Please make sure all the copyrights are updated.


Done


common/autoconf/basics.m4
No comments.

common/autoconf/build-performance.m4
No comments.

common/autoconf/buildjdk-spec.gmk.in
No comments.

common/autoconf/compare.sh.in
No comments.

common/autoconf/configure
No comments.

common/autoconf/configure.ac
No comments.

common/autoconf/flags.m4
L274: SHARED_LIBRARY_FLAGS="-dynamiclib 
-compatibility_version 1.0.0 -current_version 1.0.0 $PICFLAG"

L275: JVM_CFLAGS="$JVM_CFLAGS -fPIC"

L275 is new, but seeing it next to L274 makes me wonder if
$PICFLAG should be used instead of the literal '-fPIC'?

Fixed


L303: JVM_CFLAGS="$JVM_CFLAGS -fPIC"
Same question about literal '-fPIC'.

Not sure, leaving for now. It seems we leave the PICFLAG empty for 
the JDK build and only add it to the hotspot build. This should be 
addressed in a followup where we try to align flag usage more between 
the different libraries.

For most of the changes to flags.m4, I can't see how any of it
relates to the new HotSpot build.

Update: Now I'm wondering if this is one of those files that
we typically don't review because it is auto generated.
Sorry, don't remember for sure.
It's a file that should be reviewed, only generated-configure.sh can 
be ignored. The majority of the changes in here are related to cross 
compiling in the modular world. When cross compiling now, we need to 
also build a jvm for the build platform in order to run jlink and 
jmod when building images. With the old hotspot build, that was 
simpler, just invoke the hotspot build with some ARCH and compiler 
related variables set. For the rest of the JDK build, an 
approximation of flags used was enough so the problem was never fully 
solved.


In the new build, we derive all the compiler options in configure so 
I had to introduce a more proper solution. I did this by 
parameterizing some macros in flags.m4 and platform.m4 so that we can 
run them twice, once for the "target" toolchain" and one for the 
"build" toolchain. These are the majority of the changes you are 
seeing. I also removed the old hard coded "build" versions of certain 
flag and platform variables.

common/autoconf/generated-configure

Re: RFR: JDK-8152666: The new Hotspot Build System

[Daniel D. Daugherty]

I'm good with all of your replies/resolutions.

I did forget to say thanks for sticking with the rework of the
HotSpot build system! There have been a few attempts in the
past to rework the HotSpot build system, but no one was able
to make much progress let alone finish it. Kudos!!

Dan


On 4/6/16 3:10 AM, Erik Joelsson wrote:

Hello Dan and thank you for the review! I know it's a lot to chew through.

I have incorporated your changes and published a new webrev:
http://cr.openjdk.java.net/~erikj/8152666/webrev.02/

On 2016-04-05 20:10, Daniel D. Daugherty wrote:



> The new build supports the following variants:
>
>  * server (C1+C2)

The above "server" variant is the "tiered server". Does the new
build system support the "C2 server" variant? What about the
32-bit server and 64-bit server build variants? For example,
on Linux you can have:

  * C1/Client, 32-bit
  * C2/Server, 32-bit
  * Tiered (C1 & C2), 32-bit
  * C2/Server, 64-bit
  * Tiered (C1 + C2), 64-bit

The above wide range of variants is also true for Win*.

There is a way to achieve this even if it's not as straight forward. 
It's controlled through the new "jvm-feature" setting. To build a 
completely custom set of features for a jvm, you set the 
--with-jvm-variants=custom and then define the full feature set using 
--with-jvm-features=compiler2,... For "server, client, core, minimal, 
zero and zeroshark" there is a predefined set of features while the 
custom variant has no features by default.


General
Please make sure all the copyrights are updated.


Done


common/autoconf/basics.m4
No comments.

common/autoconf/build-performance.m4
No comments.

common/autoconf/buildjdk-spec.gmk.in
No comments.

common/autoconf/compare.sh.in
No comments.

common/autoconf/configure
No comments.

common/autoconf/configure.ac
No comments.

common/autoconf/flags.m4
L274: SHARED_LIBRARY_FLAGS="-dynamiclib 
-compatibility_version 1.0.0 -current_version 1.0.0 $PICFLAG"

L275: JVM_CFLAGS="$JVM_CFLAGS -fPIC"

L275 is new, but seeing it next to L274 makes me wonder if
$PICFLAG should be used instead of the literal '-fPIC'?

Fixed


L303: JVM_CFLAGS="$JVM_CFLAGS -fPIC"
Same question about literal '-fPIC'.

Not sure, leaving for now. It seems we leave the PICFLAG empty for the 
JDK build and only add it to the hotspot build. This should be 
addressed in a followup where we try to align flag usage more between 
the different libraries.

For most of the changes to flags.m4, I can't see how any of it
relates to the new HotSpot build.

Update: Now I'm wondering if this is one of those files that
we typically don't review because it is auto generated.
Sorry, don't remember for sure.
It's a file that should be reviewed, only generated-configure.sh can 
be ignored. The majority of the changes in here are related to cross 
compiling in the modular world. When cross compiling now, we need to 
also build a jvm for the build platform in order to run jlink and jmod 
when building images. With the old hotspot build, that was simpler, 
just invoke the hotspot build with some ARCH and compiler related 
variables set. For the rest of the JDK build, an approximation of 
flags used was enough so the problem was never fully solved.


In the new build, we derive all the compiler options in configure so I 
had to introduce a more proper solution. I did this by parameterizing 
some macros in flags.m4 and platform.m4 so that we can run them twice, 
once for the "target" toolchain" and one for the "build" toolchain. 
These are the majority of the changes you are seeing. I also removed 
the old hard coded "build" versions of certain flag and platform 
variables.

common/autoconf/generated-configure.sh
2642 lines changed... I think this is one of those files
you're supposed to skip in build-dev review... :-|

Yes, please do.


common/autoconf/help.m4
L179: $PRINTF "Which are valid to use depends on the target 
platform.\n  "

L180: $PRINTF "%s " $VALID_JVM_FEATURES
Why are there blanks after the last '\n' on L179 instead of
at the beginning of L180?

If you do $PRINTF "  %s " $VALID_JVM_FEATURES, it adds those spaces 
between every element in VALID_JVM_FEATURES.

common/autoconf/hotspot-spec.gmk.in
No comments.

common/autoconf/hotspot.m4
L46: # Check if the specified JVM features are explicitely 
enabled. To be used in

Typo: 'explicitely' -> 'explicitly'

L59: #   server: normal interpreter, and a tiered C1/C2 compiler
So no support for a C2-only server config?

L77:   # Have the user listed more than one variant?
Typo: 'Have

Re: RFR: JDK-8152666: The new Hotspot Build System

[Daniel D. Daugherty]

On 3/25/16 2:03 AM, Erik Joelsson wrote:

Hello,

Here is the initial review for the new Hotspot Build System, as 
described in " JEP 284: New HotSpot Build System". This patch adds the 
new build system along side the old and makes the new system the 
default. The old build system will remain for a (hopefully) short 
while until we feel confident it is no longer needed. This enables us 
to iron out any details that we might have missed with minimal 
disruption for the users. The goal is to remove the old system after 
one week of the new going in. During that time, both build systems 
will have to be kept in sync. For that to be possible, all changes 
touching anything in the make directory need to be reviewed by me.


In this patch, the makefiles for the new build system are located in 
hotspot/makefiles. When we apply the second phase, where we remove the 
old build system, the new will move into the proper hotspot/make 
directory.


To activate the old build system after this patch has been applied, 
use the configure arg "--disable-new-hotspot-build".


For more information about how the new build works and how to interact 
with it, Magnus wrote a document that is still relevant:
http://hg.openjdk.java.net/build-infra/jdk9/file/tip/support/new-hotspot-build.md 



> The hotspot build differs from all other libraries in the JDK in that the
> library is (potentially) built multiple times with different 
conditions (-D
> flags, for instance). The most common combination is building both 
the 'client'
> and 'server' variant, but other combinations are possible. While this 
state of
> affairs is not universally appreciated :-), it still is a use case 
that we need

> to support, and it affects the entire build system for hotspot.

Thanks for the humor and for retaining this important difference in
the way HotSpot builds versus more sane/simple systems.


> The new build supports the following variants:
>
>  * server (C1+C2)

The above "server" variant is the "tiered server". Does the new
build system support the "C2 server" variant? What about the
32-bit server and 64-bit server build variants? For example,
on Linux you can have:

  * C1/Client, 32-bit
  * C2/Server, 32-bit
  * Tiered (C1 & C2), 32-bit
  * C2/Server, 64-bit
  * Tiered (C1 + C2), 64-bit

The above wide range of variants is also true for Win*.

The main method of verification for this patch has been running the 
compare.sh script to verify that the output is equivalent to the old 
build in as many cases as possible. In most configurations we have 
reached a high level of confidence that we produce equivalent 
binaries, but there are some exceptions that should be mentioned:


* Solaris sparcv9 slowdebug produces differences when comparing 
disassembly output from libjvm.so. I have not been able to find any 
meaningful differences in compiler or linker flags to explain this.
* Windows server jvm.dll ends up with some functions in different 
order in the disassembly output. From what I can tell, the bits are 
otherwise equivalent.


We have also run the runtime nightlies with no notable failures.

This is a pretty big patch and I expect it to take some time to get 
properly reviewed. It contains contributions from Magnus Ihse Bursie, 
Erik Joelsson and Ingemar Åberg.


Bug: https://bugs.openjdk.java.net/browse/JDK-8152666
Webrev: http://cr.openjdk.java.net/~erikj/8152666/webrev.01/index.html


General
Please make sure all the copyrights are updated.


common/autoconf/basics.m4
No comments.

common/autoconf/build-performance.m4
No comments.

common/autoconf/buildjdk-spec.gmk.in
No comments.

common/autoconf/compare.sh.in
No comments.

common/autoconf/configure
No comments.

common/autoconf/configure.ac
No comments.

common/autoconf/flags.m4
L274: SHARED_LIBRARY_FLAGS="-dynamiclib 
-compatibility_version 1.0.0 -current_version 1.0.0 $PICFLAG"

L275: JVM_CFLAGS="$JVM_CFLAGS -fPIC"

L275 is new, but seeing it next to L274 makes me wonder if
$PICFLAG should be used instead of the literal '-fPIC'?

L303: JVM_CFLAGS="$JVM_CFLAGS -fPIC"
Same question about literal '-fPIC'.

For most of the changes to flags.m4, I can't see how any of it
relates to the new HotSpot build.

Update: Now I'm wondering if this is one of those files that
we typically don't review because it is auto generated.
Sorry, don't remember for sure.

common/autoconf/generated-configure.sh
2642 lines changed... I think this is one of those files
you're supposed to skip in build-dev review... :-|

common/autoconf/help.m4
L179: $PRINTF "Which are valid to use depends on the target 
platform.\n  "

L180: $PRINTF "%s " $VALID_JVM_FEATURES
Why are there blanks after the last '\n' on L179 instead of
at the beginning of L180?

common/autoconf/hotspot-spec.gmk.in
No comments.

common/autoconf/hotspot.m4
L46: # Check if the specified JV

Re: RFR: JDK-8145564: 8036003: startup regression on linux fastdebug builds

[Daniel D. Daugherty]

DEBUG_BINARIES is one of those "hidden" HotSpot big hammers that only
affects Linux (IIRC). Basically, many years ago someone got tired of
trying to figure out how to get completely debuggable HotSpot build
on Linux and this big hammer was dropped in. I could chase down who
and when, but I don't think that really matters...

When I did FDS a few years back, I was asked to not break the semantics
of DEBUG_BINARIES and so the big hammer was left in. Solaris is my
primary dev platform and DEBUG_BINARIES doesn't work there so I didn't
mind leaving in DEBUG_BINARIES for the Linux folks...

Fast forward to today... I don't think the entire patch needs to be
backed out. Not touching DEBUG_BINARIES via configure is a "good idea"
(TM) because it is such a big hammer. I do recommend trying to deprecate
the DEBUG_BINARIES setting in the big HotSpot Makefile rewrite...

I'm very much looking forward to a cleaner HotSpot build...

Dan


On 12/17/15 2:24 AM, Erik Joelsson wrote:



On 2015-12-17 09:08, David Holmes wrote:

On 17/12/2015 5:54 PM, Erik Joelsson wrote:



On 2015-12-17 01:40, David Holmes wrote:

On 17/12/2015 7:35 AM, Erik Joelsson wrote:
One more thing, where should this fix be pushed? Do you need it 
urgently

in hs-rt?


It is urgently needed in both the hs-rt and hs-comp repos as it
affects nightly testing and JPRT. If Alejandro agrees I suggest
pushing to jdk9/hs and it can then be pulled down to the team repos.


Will do.

With regard to the fix ... previously DEBUG_BINARIES was never set in
spec.gmk and so was never externally introduced into the hotspot build
this way. So why not simply change the name of the variable so that it
has no affect on the hotspot part of the build?


Well, we don't want it affecting the jdk part of the build either at
this point. This patch aims to restore the "external" and "zipped"
settings to exactly what they were before the patch, as was promised.


I had not inferred from any of this that what was being done via 
NativeCompilation.gmk was in any way a problem. I would have expected 
any problems there to be readily seen before this was reviewed and 
pushed. So I'm a bit confused about this.


I didn't follow this particular review closely as Magnus was on it and 
so I had missed the NativeCompilation part. It's true that it is very 
unlikely to be part of the problem described in this bug, but I still 
feel that the safest action at this point is to restore the behavior 
of "external" and "zipped" to what they used to be. Magnus is working 
on a complete fix where debug symbols will be enabled for everything 
in NativeCompilation.

I'm tempted to say rollback the whole thing rather than hack at it.

Rolling back will be much more work for me than submitting this patch. 
There are two changes already that depend on this change. I also don't 
dislike the idea of the new configure parameter.

And apologies as I'm just about to go offline for a few hours at least.

I hope you won't object to me pushing this with just ihse as reviewer. 
I feel this is rather a priority to get fixed.


/Erik

Re: [8u] Request for Approval and Review: JDK-8136980: build for 8u65 and 8u66 for solaris platforms is failing

[Daniel D. Daugherty]

Sorry for missing this review request. I'm still learning to deal
with my new e-mail filters (restored from memory after losing my
MacBook Pro env).


On 9/30/15 6:43 AM, Erik Joelsson wrote:

Hello,

Please approve and review this fix for 8u.

My last fix for this issue, JDK-8136691, was not enough. I made a 
mistake while verifying the fix and the problem is still there.


There are more discrepancies between Solaris and the other platform 
makefiles. The excludeSrc.gmk file is not included anywhere when 
building on Solaris. The variable Src_Files_EXCLUDE is overwritten in 
vm.make instead of appended to, so even when including excludeSrc.gmk, 
it wasn't enough. The other platform specific buildtree.make files 
also set INCLUDE_TRACE in the generated makefile.


Bug: https://bugs.openjdk.java.net/browse/JDK-8136980
Webrev: http://cr.openjdk.java.net/~erikj/8136980/webrev.hotspot.01/


make/solaris/makefiles/buildtree.make
No comments.

make/solaris/makefiles/vm.make
No comments.

Thumbs up.

Dan





/Erik

Re: [8u66] RFR 8079410: Hotspot version to share the same update and build version from JDK

[Daniel D. Daugherty]

On 7/20/15 6:45 PM, Alejandro E Murillo wrote:


Please review the following change that allows setting
the Hotspot minor version and build number to that
of the "--with-update-version" and "--with-build-number"
configure parameters when provided. 8u  builds only.

webrev:
http://javaweb.us.oracle.com/~amurillo/webrevs/8079410/


hotspot/make/defs.make
No comments.

hotspot/make/hotspot_version
No comments.

Thumbs up.

Just FYI: the patch link in that webrev goes to an empty page.

Dan





Background (since bug was originally filed as internal):

Currently, for 8u builds and earlier, the hotspot version looks like this
(remnant from the hotspot express days):

Java HotSpot(TM) Client VM (build 25.66-b00, mixed mode, sharing)


By convention, minor version (66 above) always matches the JDK update 
version
and hotspot build number is managed independently of the JDK build 
number.

Both values  are defined by default  in "hotspot/make/hotspot_version".
With this change they can now be setup using the corresponding JDK
configure parameters.

Consequences:

(1)  For promoted and other milestone builds, the hotspot minor version
will corresponds to the JDK update version and the hotspot build number
will match  the JDK build number.

(2) Hotspot snapshots will no longer need to change the hotspot build 
number

as that will be set at promotion time (matching the JDK build number).
Since this is stored in the file mentioned above, a  repo push
(and the corresponding bug) was required to  change it.
That will no longer be necessary.

(3)  Since JPRT configures both the update and build numbers,
 when building via JPRT, the hotspot build number for those builds
will always be  'b00' (matching the JDK build number). The Hotspot
minor version will match the update version defined in 
make/jprt.prtoperties:


java version "1.8.0_66-internal"
#   Java(TM) SE Runtime Environment (build 
1.8.0_66-internal-20150720195933.amurillo.8079410-control-b00)

#   Java HotSpot(TM) Client VM (build 25.66-b00, mixed mode, sharing)

(4) Since the version string is not actually changing, I do not expect 
this to have

any impact on external tools or apps, but let me know if so.

Thanks

Re: RFR(XXS) 8131128: fix merge error in jprt.properties

[Daniel D. Daugherty]

Vladimir,

Thanks for the fast review!

Folks, I believe we are now covered for reviews.

Dan


On 7/14/15 10:24 AM, Vladimir Kozlov wrote:

Good.

Thanks,
Vladimir

On 7/14/15 9:22 AM, Daniel D. Daugherty wrote:

On 7/14/15 10:19 AM, Tim Bell wrote:

Hi Dan:


I have a fix for the following bug:

JDK-8131128 Merge error in jprt.properties leads to missing 
devkit argument

https://bugs.openjdk.java.net/browse/JDK-8131128

I made a merge error on 2015.07.03 during my gatekeeping work for a
Main_Baseline -> RT_Baseline sync-down. Thanks for Mikael for finding
this issue.

Webrev URL: 
http://cr.openjdk.java.net/~dcubed/8131128-webrev/0-jdk9-hs-rt/


If you're interested in the whitespace changes that I made along
with adding the line break, you'll need to use the 'cdiffs' link.


Looks good to me.  That is a very tricky file to change.


Tim,

Thanks for the fast review!

Folks, I need (R)eviewer to take a look here.

Dan




Tim

Re: RFR(XXS) 8131128: fix merge error in jprt.properties

[Daniel D. Daugherty]

On 7/14/15 10:19 AM, Tim Bell wrote:

Hi Dan:


I have a fix for the following bug:

JDK-8131128 Merge error in jprt.properties leads to missing 
devkit argument

https://bugs.openjdk.java.net/browse/JDK-8131128

I made a merge error on 2015.07.03 during my gatekeeping work for a
Main_Baseline -> RT_Baseline sync-down. Thanks for Mikael for finding
this issue.

Webrev URL: 
http://cr.openjdk.java.net/~dcubed/8131128-webrev/0-jdk9-hs-rt/


If you're interested in the whitespace changes that I made along
with adding the line break, you'll need to use the 'cdiffs' link.


Looks good to me.  That is a very tricky file to change.


Tim,

Thanks for the fast review!

Folks, I need (R)eviewer to take a look here.

Dan




Tim

RFR(XXS) 8131128: fix merge error in jprt.properties

[Daniel D. Daugherty]

Greetings,

I have a fix for the following bug:

JDK-8131128 Merge error in jprt.properties leads to missing devkit 
argument

https://bugs.openjdk.java.net/browse/JDK-8131128

I made a merge error on 2015.07.03 during my gatekeeping work for a
Main_Baseline -> RT_Baseline sync-down. Thanks for Mikael for finding
this issue.

Webrev URL: http://cr.openjdk.java.net/~dcubed/8131128-webrev/0-jdk9-hs-rt/

If you're interested in the whitespace changes that I made along
with adding the line break, you'll need to use the 'cdiffs' link.

Thanks, in advance, for any comments, questions or suggestions.

Dan

Re: (XXS)RFR: 8076581: Need a NON-PCH build to quickly detect missing dependencies in the source base

[Daniel D. Daugherty]

On 7/6/15 3:10 AM, David Holmes wrote:

webrev: http://cr.openjdk.java.net/~dholmes/8076581/webrev/


make/jprt.properties
No comments.

Thumbs up.

Dan





In response to the discussions in:

http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-April/017826.html

I'm now in a position to add No-PCH builds to the set of JPRT builds. 
I've simply disabled PCH for fastdebug builds. This isn't complete in 
terms of coverage but is a reasonable compromise I think. Disabling 
PCH had no adverse affects on the build times in JPRT.


Thanks,
David

Re: RFR: JDK-8085822 JEP 223: New Version-String Scheme (initial integration)

[Daniel D. Daugherty]

On 6/9/15 7:12 AM, Magnus Ihse Bursie wrote:

Hi Daniel,

Thank you for your thorough review!


This was my (failing) attempt at a "fast pass" review... :-)




On 2015-06-09 01:31, Daniel D. Daugherty wrote:
> 
http://cr.openjdk.java.net/~ihse/JDK-8085822-JEP-223-initial-patch/webrev.01 



General comment: Not all copyright years were updated.
General comment: It looks like support for the 'patch' value is not 
completely
implemented through all the Makefiles. I didn't audit for this, 
but it's

just my impression.


You are basically correct. The makefiles all propagate the patch value 
where needed, but the actual source code changes in hotspot and jdk 
ignores the patch field as of now. This will be corrected in a later 
add-on patch, submitted by someone from the jdk and/or hotspot team 
rather than the build team.




common/autoconf/generated-configure.sh
There are multiple Copyright notices in this file. Why?
Oh dear, you've reviewed the generated file. :-& I'm really impressed 
by your effort! 


Ahhh... now that you say it... it sounds familiar... I may have
made this same mistake before. I'll try to remember next time.


However, the generated-configure.sh file is automatically created by 
the autoconf framework from the rest of the files in 
common/autoconf/*, so we cannot direcly modify it's contents - only 
indirectly. The reason it's checked in, is that otherwise every user 
would need to generate it before being able to run configure. In build 
reviews, we usually either revert changes to generated-configure.sh 
before posting a review to hide it (and re-generate it before 
pushing), or we just ignore it during reviews. I should have done 
that, or pointed out that it was not needed nor possible to review 
when I cross-posted. I'm sorry.




L4076: # Verify that a given string represent a valid version 
number, and assing it to

L4077: # a variable.
Fixed two typos and reformat a bit:
  # Verify that a given string represents a valid version 
number, and

  # assigning it to a variable.

I'll put that fix in the source .m4 file instead. Thanks.


L20262: as_fn_error $? "--with--version-string must have a 
value" "$LINENO" 5
The '--with--version...' part doesn't match previous usages 
where

'--with-version...' is used
Yes, you're right. Erik pointed it out as well. It's a typo in the 
error message. It's all over the place due to copied code. I'll fix it 
in the source .m4 file.


(As a side note, I have a half-finished follow-up patch that will 
reduce the amount of code duplication, but it requires some framework 
changes so it'll have to be a separate thing.)




L20275:   # Unspecified numerical fields is interpreted as 0.
Grammar: 'is interpreted' -> 'are interpreted'

L20286: as_fn_error $? "Version string contains + but 
both 'BUILD' and 'OPT' is missing" "$LINENO" 5

Grammar: 'is missing' -> 'are missing'
Those darn English plural forms! Why can't you all do as we sensible 
Swedes and get rid of them? ;-)


(I'll fix)



L20388:username=`$ECHO "$USER" | $TR -d -c 
'[a-z][A-Z][0-9]'`

This assumes that the "USER" variable is set. Should there
be a check for "" and perhaps use "no_user_specified" or
something like that? Perhaps the build setup always makes
sure that "USER" is set to something. Don't know.
Hm. I think the worst thing that'll happen is that the username part 
of the opt string gets empty. This part is basically copied right off 
the old build, where we have relied on the $USER variable being 
present for all the time with no problems, so I think it's quite safe 
to assume.


common/autoconf/jdk-options.m4
Don't know why the 'elliptic curve crypto implementation' support
is relocated as part of this changeset, but ...
It was incorrectly placed not at the top indentation level, but in the 
middle of the function definition where the old versioning code were 
handled. (Which, mostly by chance, happens to work in autoconf, but is 
really bad style).



make/Javadoc.gmk
Did you mean to remove the 'clean' target?
Yep. Cleaning is done by the top-level Main.gmk makefile nowadays, 
that's just some dead code.




hotspot/make/windows/makefiles/compile.make
No changes in the frames view.
Update: udiff view shows a blank line deleted at the end of the 
file.


That's probably the result of some change going forth and then back 
again during development. But then again, removing extra blank linkes 
is not a bad thing. (jcheck unfortunately does not enforce any style 
ch

Re: RFR: JDK-8085822 JEP 223: New Version-String Scheme (initial integration)

[Daniel D. Daugherty]

> 
http://cr.openjdk.java.net/~ihse/JDK-8085822-JEP-223-initial-patch/webrev.01 



General comment: Not all copyright years were updated.
General comment: It looks like support for the 'patch' value is not 
completely
implemented through all the Makefiles. I didn't audit for this, but 
it's

just my impression.

common/autoconf/configure.ac
No comments.

common/autoconf/flags.m4
No comments.

common/autoconf/generated-configure.sh
There are multiple Copyright notices in this file. Why?

L4076: # Verify that a given string represent a valid version 
number, and assing it to

L4077: # a variable.
Fixed two typos and reformat a bit:
  # Verify that a given string represents a valid version 
number, and

  # assigning it to a variable.

L20186-20189: indent for the block is off

L20256-20259: indent for the block is off

L20262: as_fn_error $? "--with--version-string must have a 
value" "$LINENO" 5

The '--with--version...' part doesn't match previous usages where
'--with-version...' is used

L20275:   # Unspecified numerical fields is interpreted as 0.
Grammar: 'is interpreted' -> 'are interpreted'

L20286: as_fn_error $? "Version string contains + but both 
'BUILD' and 'OPT' is missing" "$LINENO" 5

Grammar: 'is missing' -> 'are missing'

L20292:   as_fn_error $? "--with--version-string fails to parse
The '--with--version...' part doesn't match previous usages where
'--with-version...' is used

L20297-L20302: indent for the block is off

L20307:   as_fn_error $? "--with--version-pre-base must have a 
value" "$LINENO" 5
L20315: { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: 
--with--version-pre-base value...

L20316: $as_echo "$as_me: WARNING: --with--version-pre-base value...
The '--with--version...' part doesn't match previous usages where
'--with-version...' is used

L20327-20332: indent for the block is off

L20337:   as_fn_error $? "--with--version-pre-debuglevel must 
have...
L20345: { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: 
--with--version-pre-debuglevel value...

L20346: $as_echo "$as_me: WARNING: --with--version-pre-debuglevel value
The '--with--version...' part doesn't match previous usages where
'--with-version...' is used

L20361-20366: indent for the block is off

L20371:   as_fn_error $? "--with--version-opt must have...
L20379: { $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: 
--with--version-opt value

L20380: $as_echo "$as_me: WARNING: --with--version-opt value has been
The '--with--version...' part doesn't match previous usages where
'--with-version...' is used

At this point, I'm going to stop pointing out the 
'--with-version...'
and '--with--version...' differences; don't know which usage is 
right.


L20388-L20388: indent is off by one

L20388:username=`$ECHO "$USER" | $TR -d -c '[a-z][A-Z][0-9]'`
This assumes that the "USER" variable is set. Should there
be a check for "" and perhaps use "no_user_specified" or
something like that? Perhaps the build setup always makes
sure that "USER" is set to something. Don't know.

L20395-L20400: indent for the block is off

L20413-L20431: indent of all blocks in this range are off

L20444-L20449: indent for the block is off

L20457-L20475: indent of all blocks in this range are off

L20486-L20491: indent for the block is off

L20504-L20522: indent of all blocks in this range are off

L20533-L20538: indent for the block is off

L20551-L20569: indent of all blocks in this range are off

L20580-L20585: indent for the block is off

L20598-L20616: indent of all blocks in this range are off

common/autoconf/help.m4
No comments.

common/autoconf/jdk-options.m4
Don't know why the 'elliptic curve crypto implementation' support
is relocated as part of this changeset, but ...

No comments.

common/autoconf/spec.gmk.in
No comments.

common/autoconf/version-numbers
No comments.

common/nb_native/nbproject/configurations.xml
No comments.

make/Images.gmk
No comments.

make/Install.gmk
No comments.

make/Javadoc.gmk
Did you mean to remove the 'clean' target?

make/JrtfsJar.gmk
No comments.

make/MacBundles.gmk
No comments.

make/jprt.properties
No comments.

hotspot/make/Makefile
No comments.

hotspot/make/aix/Makefile
No comments.

hotspot/make/aix/makefiles/buildtree.make
No comments.

hotspot/make/aix/makefiles/defs.make
No comments.

hotspot/make/aix/makefiles/vm.make
No comments.

hotspot/make/bsd/Makefile
No comments.

hotspot/make/bsd/makefiles/buildtree.make
No comments.

hotspot/make/bsd/makefiles/defs.make
No comments.

hotspot/make/bsd/makefiles/vm.make
No comments.

hotspot/make/d

Re: De-universalizing hotspot in jdk8u

[Daniel D. Daugherty]

On 1/12/15 8:17 AM, David DeHaven wrote:

We have this nice little comment in common/autoconf/jdk-options.m4:
   # On Macosx universal binaries are produced, but they only contain
   # 64 bit intel. This invalidates control of which jvms are built
   # from configure, but only server is valid anyway. Fix this
   # when hotspot makefiles are rewritten.
   if test "x$MACOSX_UNIVERSAL" = xtrue; then
 HOTSPOT_TARGET=universal_${HOTSPOT_EXPORT}
   fi


So.. I turned it off:
   if test "x$OPENJDK_TARGET_OS" = "xmacosx"; then
#MACOSX_UNIVERSAL="true"
 MACOSX_UNIVERSAL="false"
   fi

And in hotspot/make/bsd/makefiles/defs.make:
# Universal build settings
ifeq ($(OS_VENDOR), Darwin)
   # Build universal binaries by default on Mac OS X
#  MACOSX_UNIVERSAL = true
   MACOSX_UNIVERSAL = false
   ifneq ($(ALT_MACOSX_UNIVERSAL),)

hotspot still seems to build happily (I'm kicking off tests now). Is
there are particular reason this hasn't been done yet? I'd like to
know before I pursue this as a means of eliminating our dependency on
lipo which is causing an inordinate amount of grief when trying to
build on 10.9 and later when Xcode 5+ is coinstalled. I have some
logic to work around using the broken /usr/bin/lipo, but it doesn't
help later in some "other" build target that ends up using '-arch
i386 -arch x86_64'. It does not explicitly run lipo, it relies on gcc
to do the fattening itself and so it fails even though I've gone to
the effort of working around the broken lipo (and it isn't necessary
anyways because it doesn't even build the i386 binaries even though
it's supposed to be "universal").

The hotspot makefiles still haven't been rewritten (though Magnus had
been working on it).

Also I was under the impression that we used the universal stuff
because we had to deliver in a particular way on OSX. But I don't know
the details and the people who dealt with the original OSX port effort
are no longer here.


I don't know why Hotspot is packaged as a universal binary. In the jdk,
only libJObjC was built as a universal binary and that lib was removed
before JDK 8 shipped. What part of Macosx would need to interact
directly with libjvm.dylib in a way that required a (fake) universal
format, but no other libs in the jdk? I would say go ahead and change it.

In the spirit of "never take down a fence until you know why it was put up in the 
first place" I've cc'd macosx-port-dev to see if there is anyone around who recalls 
why hotspot is using the universal binary feature.

I hear you.

My best guess is we'd initially planned on supporting 32 bit but (???) and the 
easiest way to deliver is in a universal binary. Note that we don't actually 
deliver universal binaries as the JVM is 64 bit only, or at least there are not 
nearly enough components there to run a full 32 bit JVM.

-DrD-


We (Jim Melvin did the work) included universal support in HotSpot
to not preclude someone else from doing the 32-bit MacOS X port and
providing it via OpenJDK channels. The rest of the JDK did not
include universal support, but the hotspot code would have served
as an example way forward...

However, we never heard from anyone willing to do the 32-bit port...

Dan

Re: [9] RFR (XS) 8066508: JTReg tests timeout on slow devices when run using JPRT

[Daniel D. Daugherty]

Yes. So '2013' -> '2014'.

Dan


On 12/5/14 3:17 PM, Chris Plummer wrote:
Is the copyright rule/policy to leave the oldest date and update the 
most recent date to the current year?


Chris

On 12/5/14 1:48 PM, Daniel D. Daugherty wrote:

> http://cr.openjdk.java.net/~cjplummer/8066508/webrev.00/

test/Makefile
No comments.

Thumbs up. Please fix the Copyright year before you push.

Dan


On 12/4/14 4:52 PM, David Holmes wrote:

Hi Chris,

Sorry I mis-directed you to send this one to build-dev, as it is a 
hotspot test/Makefile fix it should be reviewed by hotspot-dev now 
cc'd.


Fix looks good to me.

Thanks,
David

On 5/12/2014 6:37 AM, Chris Plummer wrote:

Please review the following fix to address JPRT timeout issues when
using -rtests to run hotspot JTReg tests on slow devices. The same 
logic

has been in place for jdk/test/Makefile for a while now, so I just
copied from there to hotspot/test/Makefile.

https://bugs.openjdk.java.net/browse/JDK-8066508
http://cr.openjdk.java.net/~cjplummer/8066508/webrev.00/

thanks,

Chris

Re: [9] RFR (XS) 8066508: JTReg tests timeout on slow devices when run using JPRT

[Daniel D. Daugherty]

> http://cr.openjdk.java.net/~cjplummer/8066508/webrev.00/

test/Makefile
No comments.

Thumbs up. Please fix the Copyright year before you push.

Dan


On 12/4/14 4:52 PM, David Holmes wrote:

Hi Chris,

Sorry I mis-directed you to send this one to build-dev, as it is a 
hotspot test/Makefile fix it should be reviewed by hotspot-dev now cc'd.


Fix looks good to me.

Thanks,
David

On 5/12/2014 6:37 AM, Chris Plummer wrote:

Please review the following fix to address JPRT timeout issues when
using -rtests to run hotspot JTReg tests on slow devices. The same logic
has been in place for jdk/test/Makefile for a while now, so I just
copied from there to hotspot/test/Makefile.

https://bugs.openjdk.java.net/browse/JDK-8066508
http://cr.openjdk.java.net/~cjplummer/8066508/webrev.00/

thanks,

Chris

Re: [9] RFR (XS) 8066508: JTReg tests timeout on slow devices when run using JPRT

[Daniel D. Daugherty]

On 12/5/14 2:45 AM, David Holmes wrote:

On 5/12/2014 7:22 PM, David Holmes wrote:

On 5/12/2014 6:11 PM, Staffan Larsen wrote:

Running with longer timeouts on fast machines makes the testing less
responsive (if a test is on its way to timeout it takes longer for us
to detect it). Ideally the timeout factor should be tuned according to
the machine type we are running on. I’m not sure that is possible,
though?


We don't have that level of control unfortunately.


Sorry that's not true. As this is our Makefile we could make the 
timeout value platform specific (though remember this is an open 
file). And if we have some means of defining a performance metric for 
a machine, we could even tune it for the current machine - but I'm not 
sure we'd want to go that path anyway. Perhaps a RFE.


What Chris is proposing addresses a current problem and brings hotspot 
testing into line with what the JDK testing has been doing since 2009. :)


I'm fairly certain that VM/SQE nightly tunes the JavaTest/JTREG
timeout factor depending on the machine type or the machine name
itself; I don't know the exact details. I suspect that this is
possible in VM/SQE nightly/Aurora because there is a database
of per-machine info.

It might be possible to do something similar for JPRT if it
tracks information per client machine...

Dan




David


David


On 5 dec 2014, at 00:52, David Holmes  wrote:

Hi Chris,

Sorry I mis-directed you to send this one to build-dev, as it is a
hotspot test/Makefile fix it should be reviewed by hotspot-dev now 
cc'd.


Fix looks good to me.

Thanks,
David

On 5/12/2014 6:37 AM, Chris Plummer wrote:

Please review the following fix to address JPRT timeout issues when
using -rtests to run hotspot JTReg tests on slow devices. The same
logic
has been in place for jdk/test/Makefile for a while now, so I just
copied from there to hotspot/test/Makefile.

https://bugs.openjdk.java.net/browse/JDK-8066508
http://cr.openjdk.java.net/~cjplummer/8066508/webrev.00/

thanks,

Chris

Re: RFR: 8065656: Use DWARF debug symbols for Solaris

[Daniel D. Daugherty]

> http://cr.openjdk.java.net/~ehelin/8065656/webrev.00/

make/solaris/makefiles/gcc.make
No comments.

make/solaris/makefiles/sparcWorks.make
No comments.

Thumbs up!

Dan


On 11/21/14 8:30 AM, Erik Helin wrote:

Hi all,

this patch changes the debug symbols format on Solaris from STABS
[0] to DWARF [1] for libjvm.so. Since the supported compiler on Solaris
has been updated to Oracle Solaris Studio 12.3 [2], the STABS debug format
is now deprecated in the supported compiler [3]:

   -xdebugformat=stabs generates debugging information
   using the stabs standard format. The stabs format is no
   longer supported.

Furthermore, in Oracle Solaris Studio 12.4, the release notes says [4]:

   The –xdebugformat=stabs for all compilers might be removed in a future
   release. The only debugger format option will be –xdebugformat=dwarf,
   which is currently the default.

So, it seems to be a good time to change the debug format to DWARF when
compiling with Oracle Solaris Studio. I also changed the debug format for
GCC on Solaris to be DWARF, since the STABS support in GCC is in
maintenance mode [5].

More reasons for using DWARF instead of STABS are:
- Better support by Oracle Studio Performance Analyzer (the performance
   team have requested that we use DWARF v2 or later instead of STABS).
- DWARF provides a better debugging experience for C++ compared to STABS.

The one drawback of using DWARF compared to STABS is that the size of the
debuginfo increases. For a SPARC fastdebug build the size of
libjvm.debuginfo built with STABS is 782 MB and with DWARF 1002 MB.

To summarize, we need to change from STABS to DWARF because STABS is
deprecated in 12.3 (even "more" deprecated 12.4 given the wording in the
release notes). I would suggest to change sooner rather than later, given
that the change to DWARF also brings Oracle Studio Performance Analyzer
support as well as a better C++ debugging experience in dbx.

Webrev:
http://cr.openjdk.java.net/~ehelin/8065656/webrev.00/

Bug:
https://bugs.openjdk.java.net/browse/JDK-8065656

Testing:
- Compiled with Oracle Solaris Studio 12.3 on both Solaris 11.1 on SPARC
   and Solaris 11.1 on x86-64 using JPRT.
- Verified that DWARF v2 symbols are produced with objdump.

Thanks,
Erik

[0]: http://www.sourceware.org/gdb/onlinedocs/stabs.html
[1]: http://www.dwarfstd.org/
[2]: http://mail.openjdk.java.net/pipermail/jdk9-dev/2014-October/001489.html
[3]: https://docs.oracle.com/cd/E24457_01/html/E22003/cplusplus.1.html
[4]: https://docs.oracle.com/cd/E37069_01/html/E37070/gnxfn.html
[5]: https://sourceware.org/ml/binutils/2013-01/msg00028.html

Re: RFR(S) Solaris Full Debug Symbols (FDS) fix for 8033602 and 8034005

[Daniel D. Daugherty]

Thanks!

Dan


On 11/15/14 11:57 AM, Dmitry Samersoff wrote:

Dan,

The fix looks good for me.

-Dmitry


On 2014-11-15 00:30, Daniel D. Daugherty wrote:

I presume I don't need to sent out another webrev...

I have to change my mind on this because this fix needs to be
backported to JDK8u-hs-dev.

Here's the updated JDK9 webrev:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/1-jdk9-hs-rt/

And here's the JDK8u-hs-dev backport:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/1-jdk8u-hs-dev/

Because of improvements to the JDK9 makefiles, a bunch of the
anchor text has changed. The best way to sanity check the backport
is to download the two patch files and look at them in your favorite
diff tool:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/1-jdk9-hs-rt/hotspot.patch

http://cr.openjdk.java.net/~dcubed/8033602-webrev/1-jdk8u-hs-dev/8033602_for_jdk8u_hs_dev.patch


I need just one sanity check on the backport...

Thanks, in advance, for any comments, questions or suggestions.

Dan


On 11/13/14 11:18 AM, Daniel D. Daugherty wrote:

Magnus,

Thanks for the review!

Replies embedded below...

On 11/13/14 7:44 AM, Magnus Ihse Bursie wrote:

On 2014-11-11 01:00, Daniel D. Daugherty wrote:

Greetings,

I have a Solaris Full Debug Symbols (FDS) fix ready for review.
Yes, it is a small fix, but it is in Makefiles so feel free to
run screaming from the room... :-)  On the plus side the fix does
delete two work around source files (Coleen would say that's a
Good Thing (TM)!)

... but you're only deleting the make files?

Good catch! Looks like when I resurrected this fix from my JDK8
queue I missed a couple of deletes.



src/os/solaris/add_gnu_debuglink/add_gnu_debuglink.c and
src/os/solaris/fix_empty_sec_hdr_flags/fix_empty_sec_hdr_flags.c
could be deleted as well, right?

Yes, these should be deleted and I'll do that in this fix.
Since these are two deletes of files that can no longer be
built anyway, I presume I don't need to sent out another
webrev...



Good idea for the fix, anyway. I opened
https://bugs.openjdk.java.net/browse/JDK-8064808 to implement a
similar solution in configure.

Sounds good to me.

Dan



/Magnus



On 11/10/14 5:00 PM, Daniel D. Daugherty wrote:

Greetings,

I have a Solaris Full Debug Symbols (FDS) fix ready for review.
Yes, it is a small fix, but it is in Makefiles so feel free to
run screaming from the room... :-)  On the plus side the fix does
delete two work around source files (Coleen would say that's a
Good Thing (TM)!)

The fix is to detect the version of GNU objcopy that is being
used on the machine and only enable Full Debug Symbols when that
version is 2.21.1 or newer. If you don't have the right version,
then the build drops back to pre-FDS build configs with a message
like this:

WARNING: /usr/sfw/bin/gobjcopy --version info:
WARNING: GNU objcopy 2.15
WARNING: an objcopy version of 2.21.1 or newer is needed to create

valid .debuginfo files.

WARNING: ignoring above objcopy command.
WARNING: patch 149063-01 or newer contains the correct Solaris 10

SPARC version.

WARNING: patch 149064-01 or newer contains the correct Solaris 10 X86

version.

WARNING: Solaris 11 Update 1 contains the correct version.
INFO: no objcopy cmd found so cannot create .debuginfo files.
INFO: ENABLE_FULL_DEBUG_SYMBOLS=0

This work is being tracked by the following bug IDs:

 JDK-8033602 wrong stabs data in libjvm.debuginfo on JDK 8 - SPARC
 https://bugs.openjdk.java.net/browse/JDK-8033602

 JDK-8034005 cannot debug in synchronizer.o or objectMonitor.o on

Solaris X86

 https://bugs.openjdk.java.net/browse/JDK-8034005

Here is the webrev URL:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/0-jdk9-hs-rt/

Testing:

- JPRT test jobs to verify that the current JPRT Solaris hosts
   are happy
- local builds on my Solaris 10 X86 machine to verify that the
   wrong version of GNU objcopy is caught

Thanks, in advance, for any comments, questions or suggestions.

Dan

Re: RFR(S) Solaris Full Debug Symbols (FDS) fix for 8033602 and 8034005

[Daniel D. Daugherty]

> I presume I don't need to sent out another webrev...

I have to change my mind on this because this fix needs to be
backported to JDK8u-hs-dev.

Here's the updated JDK9 webrev:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/1-jdk9-hs-rt/

And here's the JDK8u-hs-dev backport:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/1-jdk8u-hs-dev/

Because of improvements to the JDK9 makefiles, a bunch of the
anchor text has changed. The best way to sanity check the backport
is to download the two patch files and look at them in your favorite
diff tool:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/1-jdk9-hs-rt/hotspot.patch
http://cr.openjdk.java.net/~dcubed/8033602-webrev/1-jdk8u-hs-dev/8033602_for_jdk8u_hs_dev.patch

I need just one sanity check on the backport...

Thanks, in advance, for any comments, questions or suggestions.

Dan


On 11/13/14 11:18 AM, Daniel D. Daugherty wrote:

Magnus,

Thanks for the review!

Replies embedded below...

On 11/13/14 7:44 AM, Magnus Ihse Bursie wrote:

On 2014-11-11 01:00, Daniel D. Daugherty wrote:

Greetings,

I have a Solaris Full Debug Symbols (FDS) fix ready for review.
Yes, it is a small fix, but it is in Makefiles so feel free to
run screaming from the room... :-)  On the plus side the fix does
delete two work around source files (Coleen would say that's a
Good Thing (TM)!)


... but you're only deleting the make files?


Good catch! Looks like when I resurrected this fix from my JDK8
queue I missed a couple of deletes.


src/os/solaris/add_gnu_debuglink/add_gnu_debuglink.c and 
src/os/solaris/fix_empty_sec_hdr_flags/fix_empty_sec_hdr_flags.c 
could be deleted as well, right?


Yes, these should be deleted and I'll do that in this fix.
Since these are two deletes of files that can no longer be
built anyway, I presume I don't need to sent out another
webrev...




Good idea for the fix, anyway. I opened 
https://bugs.openjdk.java.net/browse/JDK-8064808 to implement a 
similar solution in configure.


Sounds good to me.

Dan




/Magnus




On 11/10/14 5:00 PM, Daniel D. Daugherty wrote:
> Greetings,
>
> I have a Solaris Full Debug Symbols (FDS) fix ready for review.
> Yes, it is a small fix, but it is in Makefiles so feel free to
> run screaming from the room... :-)  On the plus side the fix does
> delete two work around source files (Coleen would say that's a
> Good Thing (TM)!)
>
> The fix is to detect the version of GNU objcopy that is being
> used on the machine and only enable Full Debug Symbols when that
> version is 2.21.1 or newer. If you don't have the right version,
> then the build drops back to pre-FDS build configs with a message
> like this:
>
> WARNING: /usr/sfw/bin/gobjcopy --version info:
> WARNING: GNU objcopy 2.15
> WARNING: an objcopy version of 2.21.1 or newer is needed to create 
valid .debuginfo files.

> WARNING: ignoring above objcopy command.
> WARNING: patch 149063-01 or newer contains the correct Solaris 10 
SPARC version.
> WARNING: patch 149064-01 or newer contains the correct Solaris 10 X86 
version.

> WARNING: Solaris 11 Update 1 contains the correct version.
> INFO: no objcopy cmd found so cannot create .debuginfo files.
> INFO: ENABLE_FULL_DEBUG_SYMBOLS=0
>
> This work is being tracked by the following bug IDs:
>
> JDK-8033602 wrong stabs data in libjvm.debuginfo on JDK 8 - SPARC
> https://bugs.openjdk.java.net/browse/JDK-8033602
>
> JDK-8034005 cannot debug in synchronizer.o or objectMonitor.o on 
Solaris X86

> https://bugs.openjdk.java.net/browse/JDK-8034005
>
> Here is the webrev URL:
>
> http://cr.openjdk.java.net/~dcubed/8033602-webrev/0-jdk9-hs-rt/
>
> Testing:
>
> - JPRT test jobs to verify that the current JPRT Solaris hosts
>   are happy
> - local builds on my Solaris 10 X86 machine to verify that the
>   wrong version of GNU objcopy is caught
>
> Thanks, in advance, for any comments, questions or suggestions.
>
> Dan

Re: RFR(S) Solaris Full Debug Symbols (FDS) fix for 8033602 and 8034005

[Daniel D. Daugherty]

Magnus,

Thanks for the review!

Replies embedded below...

On 11/13/14 7:44 AM, Magnus Ihse Bursie wrote:

On 2014-11-11 01:00, Daniel D. Daugherty wrote:

Greetings,

I have a Solaris Full Debug Symbols (FDS) fix ready for review.
Yes, it is a small fix, but it is in Makefiles so feel free to
run screaming from the room... :-)  On the plus side the fix does
delete two work around source files (Coleen would say that's a
Good Thing (TM)!)


... but you're only deleting the make files?


Good catch! Looks like when I resurrected this fix from my JDK8
queue I missed a couple of deletes.


src/os/solaris/add_gnu_debuglink/add_gnu_debuglink.c and 
src/os/solaris/fix_empty_sec_hdr_flags/fix_empty_sec_hdr_flags.c could 
be deleted as well, right?


Yes, these should be deleted and I'll do that in this fix.
Since these are two deletes of files that can no longer be
built anyway, I presume I don't need to sent out another
webrev...




Good idea for the fix, anyway. I opened 
https://bugs.openjdk.java.net/browse/JDK-8064808 to implement a 
similar solution in configure.


Sounds good to me.

Dan




/Magnus

Re: RFR(S) Solaris Full Debug Symbols (FDS) fix for 8033602 and 8034005

[Daniel D. Daugherty]

Thanks! I was still in need of a (R)eviewer and a Runtime
team member so thanks for covering both...

Dan


On 11/12/14 8:10 PM, David Holmes wrote:

Hi Dan,

If you still need a Reviewer, looks okay to me.

Thanks,
David

On 11/11/2014 10:00 AM, Daniel D. Daugherty wrote:

Greetings,

I have a Solaris Full Debug Symbols (FDS) fix ready for review.
Yes, it is a small fix, but it is in Makefiles so feel free to
run screaming from the room... :-)  On the plus side the fix does
delete two work around source files (Coleen would say that's a
Good Thing (TM)!)

The fix is to detect the version of GNU objcopy that is being
used on the machine and only enable Full Debug Symbols when that
version is 2.21.1 or newer. If you don't have the right version,
then the build drops back to pre-FDS build configs with a message
like this:

WARNING: /usr/sfw/bin/gobjcopy --version info:
WARNING: GNU objcopy 2.15
WARNING: an objcopy version of 2.21.1 or newer is needed to create valid
.debuginfo files.
WARNING: ignoring above objcopy command.
WARNING: patch 149063-01 or newer contains the correct Solaris 10 SPARC
version.
WARNING: patch 149064-01 or newer contains the correct Solaris 10 X86
version.
WARNING: Solaris 11 Update 1 contains the correct version.
INFO: no objcopy cmd found so cannot create .debuginfo files.
INFO: ENABLE_FULL_DEBUG_SYMBOLS=0

This work is being tracked by the following bug IDs:

 JDK-8033602 wrong stabs data in libjvm.debuginfo on JDK 8 - SPARC
 https://bugs.openjdk.java.net/browse/JDK-8033602

 JDK-8034005 cannot debug in synchronizer.o or objectMonitor.o on
Solaris X86
 https://bugs.openjdk.java.net/browse/JDK-8034005

Here is the webrev URL:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/0-jdk9-hs-rt/

Testing:

- JPRT test jobs to verify that the current JPRT Solaris hosts
   are happy
- local builds on my Solaris 10 X86 machine to verify that the
   wrong version of GNU objcopy is caught

Thanks, in advance, for any comments, questions or suggestions.

Dan

Re: RFR(S) Solaris Full Debug Symbols (FDS) fix for 8033602 and 8034005

[Daniel D. Daugherty]

Thanks for the review!


On 11/11/14 3:04 PM, serguei.spit...@oracle.com wrote:

Dan,

The fix looks good.


Thanks!



Nice cleanup from workarounds: Good Thing (TM)! :)


Yes, this has been in the queue for quite a while... :-)

Dan




Thanks,
Serguei

On 11/10/14 4:00 PM, Daniel D. Daugherty wrote:

Greetings,

I have a Solaris Full Debug Symbols (FDS) fix ready for review.
Yes, it is a small fix, but it is in Makefiles so feel free to
run screaming from the room... :-)  On the plus side the fix does
delete two work around source files (Coleen would say that's a
Good Thing (TM)!)

The fix is to detect the version of GNU objcopy that is being
used on the machine and only enable Full Debug Symbols when that
version is 2.21.1 or newer. If you don't have the right version,
then the build drops back to pre-FDS build configs with a message
like this:

WARNING: /usr/sfw/bin/gobjcopy --version info:
WARNING: GNU objcopy 2.15
WARNING: an objcopy version of 2.21.1 or newer is needed to create 
valid .debuginfo files.

WARNING: ignoring above objcopy command.
WARNING: patch 149063-01 or newer contains the correct Solaris 10 
SPARC version.
WARNING: patch 149064-01 or newer contains the correct Solaris 10 X86 
version.

WARNING: Solaris 11 Update 1 contains the correct version.
INFO: no objcopy cmd found so cannot create .debuginfo files.
INFO: ENABLE_FULL_DEBUG_SYMBOLS=0

This work is being tracked by the following bug IDs:

JDK-8033602 wrong stabs data in libjvm.debuginfo on JDK 8 - SPARC
https://bugs.openjdk.java.net/browse/JDK-8033602

JDK-8034005 cannot debug in synchronizer.o or objectMonitor.o on 
Solaris X86

https://bugs.openjdk.java.net/browse/JDK-8034005

Here is the webrev URL:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/0-jdk9-hs-rt/

Testing:

- JPRT test jobs to verify that the current JPRT Solaris hosts
  are happy
- local builds on my Solaris 10 X86 machine to verify that the
  wrong version of GNU objcopy is caught

Thanks, in advance, for any comments, questions or suggestions.

Dan

Re: RFR(S) Solaris Full Debug Symbols (FDS) fix for 8033602 and 8034005

[Daniel D. Daugherty]

Thanks for closing the loop on this!

Dan


On 11/11/14 9:21 AM, Dmitry Samersoff wrote:

Dan,

Thank you for the explanation.

The fix looks good for me.

-Dmitry

On 2014-11-11 18:40, Daniel D. Daugherty wrote:

Dmitry,

Thanks for the quick review!

Replies embedded below...


On 11/11/14 1:35 AM, Dmitry Samersoff wrote:

Dan,

1. defs.make:

It might be better to join obcopy version check and condition at ll.190

I looked at that... The seemingly natural place to put the version check
is actually in the else branch on line 194... However, if the version
check is bad, then you have to make a second check for a reset OBJCOPY
value (along with indenting all the code another level or two).

It just looked ugly... it seemed better to keep the version check
separate from the other logic.



otherwise the user will have a wrong version warning and then misleading
message "no objcopy cmd found"

However, part of that wrong version warning is this line:

WARNING: ignoring above objcopy command.

so in reality that "no objcopy cmd found" is just confirming
that we are indeed ignoring the objcopy cmd that we found...



2. Did you consider moving objcopy detection to configure?

No because this fix has to be backported to JDK8u and JDK7 since
we support FDS in those releases...

Of course, the build-infra team is always welcome to use a new
bug to evolve this code for JDK9 and newer.

Again, thanks for the review!

Dan




-Dmitry


On 2014-11-11 03:00, Daniel D. Daugherty wrote:

Greetings,

I have a Solaris Full Debug Symbols (FDS) fix ready for review.
Yes, it is a small fix, but it is in Makefiles so feel free to
run screaming from the room... :-)  On the plus side the fix does
delete two work around source files (Coleen would say that's a
Good Thing (TM)!)

The fix is to detect the version of GNU objcopy that is being
used on the machine and only enable Full Debug Symbols when that
version is 2.21.1 or newer. If you don't have the right version,
then the build drops back to pre-FDS build configs with a message
like this:

WARNING: /usr/sfw/bin/gobjcopy --version info:
WARNING: GNU objcopy 2.15
WARNING: an objcopy version of 2.21.1 or newer is needed to create valid
.debuginfo files.
WARNING: ignoring above objcopy command.
WARNING: patch 149063-01 or newer contains the correct Solaris 10 SPARC
version.
WARNING: patch 149064-01 or newer contains the correct Solaris 10 X86
version.
WARNING: Solaris 11 Update 1 contains the correct version.
INFO: no objcopy cmd found so cannot create .debuginfo files.
INFO: ENABLE_FULL_DEBUG_SYMBOLS=0

This work is being tracked by the following bug IDs:

  JDK-8033602 wrong stabs data in libjvm.debuginfo on JDK 8 - SPARC
  https://bugs.openjdk.java.net/browse/JDK-8033602

  JDK-8034005 cannot debug in synchronizer.o or objectMonitor.o on
Solaris X86
  https://bugs.openjdk.java.net/browse/JDK-8034005

Here is the webrev URL:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/0-jdk9-hs-rt/

Testing:

- JPRT test jobs to verify that the current JPRT Solaris hosts
are happy
- local builds on my Solaris 10 X86 machine to verify that the
wrong version of GNU objcopy is caught

Thanks, in advance, for any comments, questions or suggestions.

Dan

Re: RFR(S) Solaris Full Debug Symbols (FDS) fix for 8033602 and 8034005

[Daniel D. Daugherty]

Dmitry,

Thanks for the quick review!

Replies embedded below...


On 11/11/14 1:35 AM, Dmitry Samersoff wrote:

Dan,

1. defs.make:

It might be better to join obcopy version check and condition at ll.190


I looked at that... The seemingly natural place to put the version check
is actually in the else branch on line 194... However, if the version
check is bad, then you have to make a second check for a reset OBJCOPY
value (along with indenting all the code another level or two).

It just looked ugly... it seemed better to keep the version check
separate from the other logic.



otherwise the user will have a wrong version warning and then misleading
message "no objcopy cmd found"


However, part of that wrong version warning is this line:

WARNING: ignoring above objcopy command.

so in reality that "no objcopy cmd found" is just confirming
that we are indeed ignoring the objcopy cmd that we found...



2. Did you consider moving objcopy detection to configure?


No because this fix has to be backported to JDK8u and JDK7 since
we support FDS in those releases...

Of course, the build-infra team is always welcome to use a new
bug to evolve this code for JDK9 and newer.

Again, thanks for the review!

Dan





-Dmitry


On 2014-11-11 03:00, Daniel D. Daugherty wrote:

Greetings,

I have a Solaris Full Debug Symbols (FDS) fix ready for review.
Yes, it is a small fix, but it is in Makefiles so feel free to
run screaming from the room... :-)  On the plus side the fix does
delete two work around source files (Coleen would say that's a
Good Thing (TM)!)

The fix is to detect the version of GNU objcopy that is being
used on the machine and only enable Full Debug Symbols when that
version is 2.21.1 or newer. If you don't have the right version,
then the build drops back to pre-FDS build configs with a message
like this:

WARNING: /usr/sfw/bin/gobjcopy --version info:
WARNING: GNU objcopy 2.15
WARNING: an objcopy version of 2.21.1 or newer is needed to create valid
.debuginfo files.
WARNING: ignoring above objcopy command.
WARNING: patch 149063-01 or newer contains the correct Solaris 10 SPARC
version.
WARNING: patch 149064-01 or newer contains the correct Solaris 10 X86
version.
WARNING: Solaris 11 Update 1 contains the correct version.
INFO: no objcopy cmd found so cannot create .debuginfo files.
INFO: ENABLE_FULL_DEBUG_SYMBOLS=0

This work is being tracked by the following bug IDs:

 JDK-8033602 wrong stabs data in libjvm.debuginfo on JDK 8 - SPARC
 https://bugs.openjdk.java.net/browse/JDK-8033602

 JDK-8034005 cannot debug in synchronizer.o or objectMonitor.o on
Solaris X86
 https://bugs.openjdk.java.net/browse/JDK-8034005

Here is the webrev URL:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/0-jdk9-hs-rt/

Testing:

- JPRT test jobs to verify that the current JPRT Solaris hosts
   are happy
- local builds on my Solaris 10 X86 machine to verify that the
   wrong version of GNU objcopy is caught

Thanks, in advance, for any comments, questions or suggestions.

Dan

RFR(S) Solaris Full Debug Symbols (FDS) fix for 8033602 and 8034005

[Daniel D. Daugherty]

Greetings,

I have a Solaris Full Debug Symbols (FDS) fix ready for review.
Yes, it is a small fix, but it is in Makefiles so feel free to
run screaming from the room... :-)  On the plus side the fix does
delete two work around source files (Coleen would say that's a
Good Thing (TM)!)

The fix is to detect the version of GNU objcopy that is being
used on the machine and only enable Full Debug Symbols when that
version is 2.21.1 or newer. If you don't have the right version,
then the build drops back to pre-FDS build configs with a message
like this:

WARNING: /usr/sfw/bin/gobjcopy --version info:
WARNING: GNU objcopy 2.15
WARNING: an objcopy version of 2.21.1 or newer is needed to create valid 
.debuginfo files.

WARNING: ignoring above objcopy command.
WARNING: patch 149063-01 or newer contains the correct Solaris 10 SPARC 
version.
WARNING: patch 149064-01 or newer contains the correct Solaris 10 X86 
version.

WARNING: Solaris 11 Update 1 contains the correct version.
INFO: no objcopy cmd found so cannot create .debuginfo files.
INFO: ENABLE_FULL_DEBUG_SYMBOLS=0

This work is being tracked by the following bug IDs:

JDK-8033602 wrong stabs data in libjvm.debuginfo on JDK 8 - SPARC
https://bugs.openjdk.java.net/browse/JDK-8033602

JDK-8034005 cannot debug in synchronizer.o or objectMonitor.o on 
Solaris X86

https://bugs.openjdk.java.net/browse/JDK-8034005

Here is the webrev URL:

http://cr.openjdk.java.net/~dcubed/8033602-webrev/0-jdk9-hs-rt/

Testing:

- JPRT test jobs to verify that the current JPRT Solaris hosts
  are happy
- local builds on my Solaris 10 X86 machine to verify that the
  wrong version of GNU objcopy is caught

Thanks, in advance, for any comments, questions or suggestions.

Dan

Re: RFR: JDK-8056053: Disable HOTSPOT_BUILD_JOBS when building with configure

[Daniel D. Daugherty]

On 8/27/14 7:09 AM, Daniel D. Daugherty wrote:

On 8/27/14 6:53 AM, Erik Joelsson wrote:

Thanks David and Magnus for the reviews!

Now for pushing this. The changes in root and hotspot need to go 
together. They should not affect the standalone hotspot build, nor 
the resulting built bits. I think it is safe to push both to 
jdk9/hs-rt as it would not affect testing compatibility with the 
promoted jdk build. Is it ok to do so?


Not only OK, but also the preferred way to go! Alejandro prefers that
HotSpot makefile changes come up through HotSpot repos so we don't
get any surprises...


Forgot to say: Thumbs up! on the changes.

Dan




Dan




/Erik


On 2014-08-26 12:53, Erik Joelsson wrote:

Hello,

Please review this proposed fix for the Hotspot build.

In the new jdk9 build, we utilize the gnu make job server, which 
automatically makes sure the -j flag gets propagated and shared 
between all recursive make calls. In the hotspot build, this gets 
overridden by the HOTSPOT_BUILD_JOBS variable. Configure estimates a 
reasonable number of parallel make jobs into the JOBS variable, 
which gets propagated to the HOTSPOT_BUILD_JOBS variable. This used 
to work well enough, but in the new build, the hotspot build is 
happening concurrently with other parts of the build and the 
consequence is that the hotspot build gets JOBS number of jobs and 
the rest of the build also gets JOBS number of jobs, all of which 
are used at the same time. We would like the whole build to share in 
the same job pool.


To fix this, the setting of -j$(HOTSPOT_BUILD_JOBS) needs to be made 
conditional and we need to add .NOTPARALLEL: to a number of 
makefiles in hotspot that currently can't handle being executed in 
parallel. Lastly, the + sign must be added first to recipe lines 
that call make recursively but are not explicitly using the MAKE 
variable directly. The result will be that the active -j flag in the 
root makefiles will just automatically propagate down to the hotspot 
makefiles.


Bug: https://bugs.openjdk.java.net/browse/JDK-8056053
Webrev: http://cr.openjdk.java.net/~erikj/8056053/webrev.01/

/Erik

Re: RFR: JDK-8056053: Disable HOTSPOT_BUILD_JOBS when building with configure

[Daniel D. Daugherty]

On 8/27/14 6:53 AM, Erik Joelsson wrote:

Thanks David and Magnus for the reviews!

Now for pushing this. The changes in root and hotspot need to go 
together. They should not affect the standalone hotspot build, nor the 
resulting built bits. I think it is safe to push both to jdk9/hs-rt as 
it would not affect testing compatibility with the promoted jdk build. 
Is it ok to do so?


Not only OK, but also the preferred way to go! Alejandro prefers that
HotSpot makefile changes come up through HotSpot repos so we don't
get any surprises...

Dan




/Erik


On 2014-08-26 12:53, Erik Joelsson wrote:

Hello,

Please review this proposed fix for the Hotspot build.

In the new jdk9 build, we utilize the gnu make job server, which 
automatically makes sure the -j flag gets propagated and shared 
between all recursive make calls. In the hotspot build, this gets 
overridden by the HOTSPOT_BUILD_JOBS variable. Configure estimates a 
reasonable number of parallel make jobs into the JOBS variable, which 
gets propagated to the HOTSPOT_BUILD_JOBS variable. This used to work 
well enough, but in the new build, the hotspot build is happening 
concurrently with other parts of the build and the consequence is 
that the hotspot build gets JOBS number of jobs and the rest of the 
build also gets JOBS number of jobs, all of which are used at the 
same time. We would like the whole build to share in the same job pool.


To fix this, the setting of -j$(HOTSPOT_BUILD_JOBS) needs to be made 
conditional and we need to add .NOTPARALLEL: to a number of makefiles 
in hotspot that currently can't handle being executed in parallel. 
Lastly, the + sign must be added first to recipe lines that call make 
recursively but are not explicitly using the MAKE variable directly. 
The result will be that the active -j flag in the root makefiles will 
just automatically propagate down to the hotspot makefiles.


Bug: https://bugs.openjdk.java.net/browse/JDK-8056053
Webrev: http://cr.openjdk.java.net/~erikj/8056053/webrev.01/

/Erik

Re: RFR[P1]: 8055744 - 8u-dev nightly solaris builds failed on 08/20

[Daniel D. Daugherty]

> http://cr.openjdk.java.net/~jwilhelm/8055744/webrev.02/

make/solaris/Makefile
No comments.

make/solaris/makefiles/buildtree.make
No comments.

A product build with the above changes shows this:

$ cat solaris_amd64_compiler2/product/mapfile_ext

# Extended set of symbols.
JVM_SetVmMemoryPressure;

Looks right to me.

Dan


On 8/21/14 11:04 AM, Jesper Wilhelmsson wrote:

Thank you for the quick reply Dan!

A new webrev with your suggested change is available here:

http://cr.openjdk.java.net/~jwilhelm/8055744/webrev.02/

/Jesper


Daniel D. Daugherty skrev 21/8/14 18:42:

On 8/21/14 10:19 AM, Jesper Wilhelmsson wrote:

Hi,

On Solaris the HS_ALT_MAKE variable was not passed to vm.make when 
creating
the mapfiles which lead to mapfile-ext not being found and later a 
linker

error due to symbols declared in the extra mapfile not being found.

The hotspot makefiles are .. interesting .. yes.

The proposed solution is to include defs.make where HS_ALT_MAKE is 
set up into

vm.make on Solaris.


Webrev:
http://cr.openjdk.java.net/~jwilhelm/8055744/webrev/


Unfortunately, including defs.make in make/solaris/makefiles/vm.make
isn't the "right" way to get a top-level variable down into the
HotSpot build system. I cannot remember what breaks when you do that,
but it doesn't work right in all the ways that we build HotSpot.

For the HotSpot build system, you'll want to:

- update make/solaris/Makefile and add HS_ALT_MAKE to
   the BUILDTREE_VARS list:

   BUILDTREE_VARS += HS_ALT_MAKE=$(HS_ALT_MAKE)

- update make/solaris/makefiles/buildtree.make and add
   HS_ALT_MAKE to the rule that creates flags.make:

   [ -n "$(HS_ALT_MAKE)" ] && \
 echo && echo "HS_ALT_MAKE = $(HS_ALT_MAKE)"; \

The way I usually find all the right spots is I look for
where ZIPEXE is added to the above files and follow those
examples.

Dan






Bug:
https://bugs.openjdk.java.net/browse/JDK-8055744#comment-13542110

This is a P1 so if you feel comfortable with the hotspot makefiles, 
please

have a look.

Thanks!
/Jesper

Re: RFR[P1]: 8055744 - 8u-dev nightly solaris builds failed on 08/20

[Daniel D. Daugherty]

On 8/21/14 10:19 AM, Jesper Wilhelmsson wrote:

Hi,

On Solaris the HS_ALT_MAKE variable was not passed to vm.make when 
creating the mapfiles which lead to mapfile-ext not being found and 
later a linker error due to symbols declared in the extra mapfile not 
being found.


The hotspot makefiles are .. interesting .. yes.

The proposed solution is to include defs.make where HS_ALT_MAKE is set 
up into vm.make on Solaris.



Webrev:
http://cr.openjdk.java.net/~jwilhelm/8055744/webrev/


Unfortunately, including defs.make in make/solaris/makefiles/vm.make
isn't the "right" way to get a top-level variable down into the
HotSpot build system. I cannot remember what breaks when you do that,
but it doesn't work right in all the ways that we build HotSpot.

For the HotSpot build system, you'll want to:

- update make/solaris/Makefile and add HS_ALT_MAKE to
  the BUILDTREE_VARS list:

  BUILDTREE_VARS += HS_ALT_MAKE=$(HS_ALT_MAKE)

- update make/solaris/makefiles/buildtree.make and add
  HS_ALT_MAKE to the rule that creates flags.make:

  [ -n "$(HS_ALT_MAKE)" ] && \
echo && echo "HS_ALT_MAKE = $(HS_ALT_MAKE)"; \

The way I usually find all the right spots is I look for
where ZIPEXE is added to the above files and follow those
examples.

Dan






Bug:
https://bugs.openjdk.java.net/browse/JDK-8055744#comment-13542110

This is a P1 so if you feel comfortable with the hotspot makefiles, 
please have a look.


Thanks!
/Jesper

Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

[Daniel D. Daugherty]

Your call.

Dan


On 8/20/14 11:05 AM, Christian Tornqvist wrote:

I talked to Staffan Friberg (perf tech lead) about enabling this in a
release builds and this is his opinion:

"For /homeparams I think enable it now in fastdebug, let's use it for a
while and understand home much of a benefit it is for debugging. If it is a
huge help let's consider it for release, but that will require a lot of
performance testing before commiting"

I agree with Staffan and think we should only enable it in fastdebug builds
for now.

Thanks,
Christian
  
-Original Message-----

From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 12:02 PM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

On 8/20/14 9:48 AM, Christian Tornqvist wrote:

indicate that we shouldn't need this change in our debug/fastdebug

builds.

However, you looked at the generated prologue code before and after
turning on this option right?

Our fastdebug builds are compiled with /O2 which doesn't spill the
parameters onto the stack, our debug builds (with /Od) will do this so
passing /homeparams there is a no-op.

Here is a look at the prologue code for
ClassFileParser::parse_constant_pool_entries() before /homeparams:

mov dword ptr [rsp+10h],edx
pushrbp
pushrsi

and here is with /homeparams:

mov qword ptr [rsp+18h],r8
mov dword ptr [rsp+10h],edx
mov qword ptr [rsp+8],rcx
pushrbp
pushrsi

Cool. When we did the Full Debug Symbols (FDS) project, one of the things we
did was try to use the same optimization and debug options with
RELEASE/product builds as with fastdebug builds. Since we're generating
debug info for all builds configs, we thought this was a really good design
goal unless we ran into a performance issue.

During the FDS project, we saw no performance change when we switched the
optimization options and included the various generate-debug-info options.

A long way of saying:

I think you should enable /homeparams for RELEASE/product builds.

:-)

Dan



Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 11:17 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds
using /homeparams

On 8/20/14 8:49 AM, Christian Tornqvist wrote:

Do we have any idea how much of a change? Do we care? I'm presuming
the

increased debuggability is worth it.

It adds 0-4 additional stack movs in function prologue code which
should have a very low impact, it's only on debug/fastdebug builds.

I missed that this was a non-RELEASE build change in my original review.

These two blurbs from the MSDN note:

   > However, by default in a release build, the register parameters  >
will not be written to the stack, into the space that is already  >
provided for the parameters. This makes it difficult to debug an  >
optimized (release) build of your program.

and

   > In a debug build, the stack is always populated with parameters  >
passed in registers.

indicate that we shouldn't need this change in our debug/fastdebug builds.
However, you looked at the generated prologue code before and after
turning on this option right?

Does this mean that the MSDN note is not correct for the way that we
do debug and fastdebug builds? We might have some option enabled that
prevents the default /homeparams behavior from working...

Dan



The above block will also apply to an "ia64" build. We don't support
that

anymore, but I don't know if any licensees support it.

I've changed the checks in vm.make and WinGammaPlatformVC10.java


Do we need to do anything about the new, but unused  platform to
make

lint-like tools not squawk?

I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.

New webrev:
http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/

Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 9:48 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds
using /homeparams

> http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

General Comment

The MSDN note says:

> /homeparams does imply a performance disadvantage, because it  >
does require a cycle to load the register parameters on to the stack.

Do we have any idea how much of a change? Do we care? I'm presuming
the increased debuggability is worth it.

make/windows/makefiles/vm.make
line 38: !else
line 39: CXX_FLAGS=$(CXX_FLAGS) /D "ASSERT" /homeparams
line 40: !endif
The above block wi

Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

[Daniel D. Daugherty]

On 8/20/14 9:48 AM, Christian Tornqvist wrote:

indicate that we shouldn't need this change in our debug/fastdebug builds.

However, you looked at the generated prologue code before and after turning
on this option right?

Our fastdebug builds are compiled with /O2 which doesn't spill the
parameters onto the stack, our debug builds (with /Od) will do this so
passing /homeparams there is a no-op.

Here is a look at the prologue code for
ClassFileParser::parse_constant_pool_entries() before /homeparams:

mov dword ptr [rsp+10h],edx
pushrbp
pushrsi

and here is with /homeparams:

mov qword ptr [rsp+18h],r8
mov dword ptr [rsp+10h],edx
mov qword ptr [rsp+8],rcx
pushrbp
pushrsi


Cool. When we did the Full Debug Symbols (FDS) project, one of the
things we did was try to use the same optimization and debug options
with RELEASE/product builds as with fastdebug builds. Since we're
generating debug info for all builds configs, we thought this was
a really good design goal unless we ran into a performance issue.

During the FDS project, we saw no performance change when we
switched the optimization options and included the various
generate-debug-info options.

A long way of saying:

I think you should enable /homeparams for RELEASE/product builds.

:-)

Dan




Thanks,
Christian

-Original Message-----
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 11:17 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net; build-dev@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

On 8/20/14 8:49 AM, Christian Tornqvist wrote:

Do we have any idea how much of a change? Do we care? I'm presuming
the

increased debuggability is worth it.

It adds 0-4 additional stack movs in function prologue code which
should have a very low impact, it's only on debug/fastdebug builds.

I missed that this was a non-RELEASE build change in my original review.

These two blurbs from the MSDN note:

  > However, by default in a release build, the register parameters  > will
not be written to the stack, into the space that is already  > provided for
the parameters. This makes it difficult to debug an  > optimized (release)
build of your program.

and

  > In a debug build, the stack is always populated with parameters  > passed
in registers.

indicate that we shouldn't need this change in our debug/fastdebug builds.
However, you looked at the generated prologue code before and after turning
on this option right?

Does this mean that the MSDN note is not correct for the way that we do
debug and fastdebug builds? We might have some option enabled that prevents
the default /homeparams behavior from working...

Dan



The above block will also apply to an "ia64" build. We don't support
that

anymore, but I don't know if any licensees support it.

I've changed the checks in vm.make and WinGammaPlatformVC10.java


Do we need to do anything about the new, but unused  platform to make

lint-like tools not squawk?

I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.

New webrev:
http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/

Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 9:48 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds
using /homeparams

   > http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

General Comment

The MSDN note says:

   > /homeparams does imply a performance disadvantage, because it  >
does require a cycle to load the register parameters on to the stack.

Do we have any idea how much of a change? Do we care? I'm presuming
the increased debuggability is worth it.

make/windows/makefiles/vm.make
   line 38: !else
   line 39: CXX_FLAGS=$(CXX_FLAGS) /D "ASSERT" /homeparams
   line 40: !endif
   The above block will also apply to an "ia64" build.
   We don't support that anymore, but I don't know if
   any licensees support it.

src/share/tools/ProjectCreator/BuildConfig.java
   No comments.

src/share/tools/ProjectCreator/WinGammaPlatformVC10.java
   line 373: if(!platformName.equals("Win32")) {
   line 374: addAttr(rv, "AdditionalOptions", "/homeparams");
   The above block will also apply to an "ia64" build.

src/share/tools/ProjectCreator/WinGammaPlatformVC7.java
src/share/tools/ProjectCreator/WinGammaPlatformVC8.java
   Do we need to do anything about the new, but unused
   platform to make lint-like tools not squawk?

Thumbs up.

Dan


On 8/19/14 5:39 PM, Christian Tornqvist wrote:

Hi everyone,




This change adds /homeparams
(http://msdn.microsoft.com/

Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using /homeparams

[Daniel D. Daugherty]

On 8/20/14 8:49 AM, Christian Tornqvist wrote:

Do we have any idea how much of a change? Do we care? I'm presuming the

increased debuggability is worth it.

It adds 0-4 additional stack movs in function prologue code which should
have a very low impact, it's only on debug/fastdebug builds.


I missed that this was a non-RELEASE build change in my
original review.

These two blurbs from the MSDN note:

> However, by default in a release build, the register parameters
> will not be written to the stack, into the space that is already
> provided for the parameters. This makes it difficult to debug an
> optimized (release) build of your program.

and

> In a debug build, the stack is always populated with parameters
> passed in registers.

indicate that we shouldn't need this change in our debug/fastdebug
builds. However, you looked at the generated prologue code before
and after turning on this option right?

Does this mean that the MSDN note is not correct for the way that
we do debug and fastdebug builds? We might have some option enabled
that prevents the default /homeparams behavior from working...

Dan





The above block will also apply to an "ia64" build. We don't support that

anymore, but I don't know if any licensees support it.

I've changed the checks in vm.make and WinGammaPlatformVC10.java


Do we need to do anything about the new, but unused  platform to make

lint-like tools not squawk?

I'll open a new bug to clean up the VC7/8/9 files in ProjectCreator.

New webrev:
http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.01/

Thanks,
Christian

-Original Message-
From: Daniel D. Daugherty [mailto:daniel.daughe...@oracle.com]
Sent: Wednesday, August 20, 2014 9:48 AM
To: Christian Tornqvist
Cc: hotspot-...@openjdk.java.net
Subject: Re: RFR(S): 8027480 - Build Windows x64 fastdebug builds using
/homeparams

  > http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

General Comment

The MSDN note says:

  > /homeparams does imply a performance disadvantage, because it  > does
require a cycle to load the register parameters on to the stack.

Do we have any idea how much of a change? Do we care? I'm presuming the
increased debuggability is worth it.

make/windows/makefiles/vm.make
  line 38: !else
  line 39: CXX_FLAGS=$(CXX_FLAGS) /D "ASSERT" /homeparams
  line 40: !endif
  The above block will also apply to an "ia64" build.
  We don't support that anymore, but I don't know if
  any licensees support it.

src/share/tools/ProjectCreator/BuildConfig.java
  No comments.

src/share/tools/ProjectCreator/WinGammaPlatformVC10.java
  line 373: if(!platformName.equals("Win32")) {
  line 374: addAttr(rv, "AdditionalOptions", "/homeparams");
  The above block will also apply to an "ia64" build.

src/share/tools/ProjectCreator/WinGammaPlatformVC7.java
src/share/tools/ProjectCreator/WinGammaPlatformVC8.java
  Do we need to do anything about the new, but unused
  platform to make lint-like tools not squawk?

Thumbs up.

Dan


On 8/19/14 5:39 PM, Christian Tornqvist wrote:

Hi everyone,

   


This change adds /homeparams
(http://msdn.microsoft.com/en-us/library/6exwh0y6.aspx) to compiler
flags when building fastdebug on Windows x64. This causes the compiler
to generate code to spill the first 4 arguments to the stack (they're
normally only passed in registers), which should make it easier to debug.

   


I also changed the ProjectCreator to enable building with this using
Visual Studio. The size of jvm.dll increases with about 3% (about 504k

increase).
   


Verified that it builds correctly using Visual Studio and JPRT, the
generation of the spill code has been verified by comparing prologue
code for several functions in the JVM with/without using /homeparams.

   


Webrev:

http://cr.openjdk.java.net/~ctornqvi/webrev/8027480/webrev.00/

   


Bug:

https://bugs.openjdk.java.net/browse/JDK-8027480

   


Thanks,

Christian

   

Re: RFR: JDK-8044480: JDK image target overwrites lib/server/libjsig.dylib symlink with a copy of lib/libjsig.dylib

[Daniel D. Daugherty]

Erik,

Thanks for the very fast turn around on this.

I could not think of a good way to test this for unexpected side
effects other than to make the change and watch for fallout. I was
not able to envision a scenario where replacing a regular file
with a symlink to a copy of the same file would be functionally
different.

Thumbs up.

Dan

P.S.
Ron is on vacation this week so please don't wait for a code
review from him.


On 6/2/14 6:54 AM, Erik Joelsson wrote:

Hello,

Please review this small fix to images/bundle build on Macosx. In the 
macosx specific j2*-bundle, lib/server/libjsig.dylib is not a symlink 
to ../libjsig.dylib like on other platforms (or like in j2*-image). 
This behavior dates back to the original macosx port and was 
intentionally emulated in the new build in JDK 8.


Bug: https://bugs.openjdk.java.net/browse/JDK-8044480
The fix in JDK 9 inline:

diff --git a/make/Bundles.gmk b/make/Bundles.gmk
--- a/make/Bundles.gmk
+++ b/make/Bundles.gmk
@@ -74,19 +74,16 @@
   JDK_TARGET_LIST := $(subst 
$(JDK_IMAGE_DIR)/,$(JDK_BUNDLE_DIR)/Home/,$(JDK_FILE_LIST))
   JRE_TARGET_LIST := $(subst 
$(JRE_IMAGE_DIR)/,$(JRE_BUNDLE_DIR)/Home/,$(JRE_FILE_LIST))


-  # The old builds implementation of this did not preserve symlinks so
-  # make sure they are followed and the contents copied instead.
-  # To fix this, remove -L
   # Copy empty directories (jre/lib/applet).
   $(JDK_BUNDLE_DIR)/Home/%: $(JDK_IMAGE_DIR)/%
 $(ECHO) Copying $(patsubst $(OUTPUT_ROOT)/%,%,$@)
 $(MKDIR) -p $(@D)
-if [ -d "$<" ]; then $(MKDIR) -p $@; else $(CP) -f -R -L '$<' 
'$@'; fi
+if [ -d "$<" ]; then $(MKDIR) -p $@; else $(CP) -f -R -P '$<' 
'$@'; fi


   $(JRE_BUNDLE_DIR)/Home/%: $(JRE_IMAGE_DIR)/%
 $(ECHO) Copying $(patsubst $(OUTPUT_ROOT)/%,%,$@)
 $(MKDIR) -p $(@D)
-if [ -d "$<" ]; then $(MKDIR) -p $@; else $(CP) -f -R -L '$<' 
'$@'; fi
+if [ -d "$<" ]; then $(MKDIR) -p $@; else $(CP) -f -R -P '$<' 
'$@'; fi


   $(JDK_BUNDLE_DIR)/MacOS/libjli.dylib:
 $(ECHO) Creating link $(patsubst $(OUTPUT_ROOT)/%,%,$@)


/Erik

Re: RFR: 8033580: Old debug information in IMPORT_JDK is not removed

[Daniel D. Daugherty]

I'm fine if you leave your indentation as-is. This can be addressed
when the new build system style reaches the HotSpot universe...

Dan


On 3/25/14 3:41 AM, Erik Helin wrote:

Daniel, Erik

thanks for the reviews and sorry taking so long to respond.

About the indentation: I understand the convention used in the new 
build system and I think it makes a lot of sense, but the file 
hotspot/make/Makefile clearly does not use this convention :)


I would prefer to use the same kind of indentation as the rest of the 
file, even though as Dan noted, the file is not consistent with 
itself. It seems like most of the file is using 2 spaces for ifeq 
(something) and then just  or  for recipes 
depending on logical level.


Dan and Erik, are you fine with keeping the indentation as it is or do 
you prefer that I update the change to use the convention of the new 
build system?


Thanks,
Erik

On 02/17/2014 09:09 AM, Erik Joelsson wrote:

Hello,

The change looks good to me too.

Regarding indentation, for the rest of the JDK, we indent rules with
make ifeqs like this:

target: sources
<8 spaces>ifeq (something)
recipe line
<8 spaces>endif

Our reasoning is that it should still look like a recipe when glancing
over it quickly (so 8 characters base indentation). After that we apply
our standard 2 chars per logical level. Since make regards tab
characters as special, we have to use space for non recipe lines.

The rest of our guidelines can be found here:
http://openjdk.java.net/groups/build/doc/code-conventions.html

/Erik

On 2014-02-14 17:45, Daniel D. Daugherty wrote:

> http://cr.openjdk.java.net/~ehelin/8033580/webrev.03/

make/Makefile
No comments.

Makefile style question:
Since all this ifeq(...) logic is around Makefile rules,
should the rules themselves be indented by one tab or by
one tab and some number of spaces.

I scrolled around the Makefile and I don't really see
consistent indenting of the rule lines themselves...

If you change the indenting, I don't see a reason for
another round of review.

Dan


On 2/14/14 6:21 AM, Erik Helin wrote:

Dan,

On 2014-02-14 00:29, Daniel D. Daugherty wrote:

 > http://cr.openjdk.java.net/~ehelin/8033580/webrev.01/

make/Makefile

+  ifeq ($(OSNAME), bsd)
+$(RM) -rf $(EXPORT_SERVER_DIR)/libjvm.dylib.dSYM

The above needs to be:

ifeq ($(OS_VENDOR), Darwin)
  $(RM) -rf $(EXPORT_SERVER_DIR)/libjvm.dylib.dSYM

I don't think that BSD uses .dylib stuff; that's MacOS X specific.


You are right, thanks for the correction. Please see new webrev at:
http://cr.openjdk.java.net/~ehelin/8033580/webrev.03/

Again, same testing as for prior patches were run.

Thanks,
Erik


Dan


On 2/13/14 1:59 PM, Erik Helin wrote:

Hi Dan,

thanks for reviewing! See my replies inline.

On 02/13/2014 12:40 AM, Daniel D. Daugherty wrote:

 > http://cr.openjdk.java.net/~ehelin/8033580/webrev.00/

make/Makefile
 277   ifeq ($(ZIP_DEBUGINFO_FILES),1)
 278 ifeq ($(OSNAME), windows)
 279   $(RM) -f $(EXPORT_SERVER_DIR)/jvm.map
$(EXPORT_SERVER_DIR)/jvm.pdb
 280 else
 281   $(RM) -f $(EXPORT_SERVER_DIR)/libjvm.debuginfo
 282 endif

 On MacOS X, the debuginfo will be in libjvm.dylib.dSYM so
 you'll need a MacOS X specific rule. You don't need an
 update for the JVM_VARIANT_CLIENT version because MacOS X
 doesn't support the Client VM, but if it did...


Thanks for catching this! I've uploaded a new webrev at:
http://cr.openjdk.java.net/~ehelin/8033580/webrev.01/

The same testing was done as for the first patch.

On 02/13/2014 12:40 AM, Daniel D. Daugherty wrote:

So the above change handles libjvm, but what about the
other libraries exported by HotSpot? libjsig, libjvm_db,
and libjvm_dtrace come to mind...


As we discussed, I would like to implement this for libjvm first and
then take care of the other libraries in a separate patch.

Thanks,
Erik


Dan


On 2/12/14 8:03 AM, Erik Helin wrote:

Hi all,

this patch changes how old debug information copied from
IMPORT_JDK is
treated.

When running the copy_*_jdk target, HotSpot's makefiles copies the
entire IMPORT_JDK folder, including additional files (such as
unzipped
debug information). The export_*_jdk targets will then, via the
generic_export target, copy the build artifacts via implicit
rules to
the correct destination in hotspot/build/JDK_IMAGE_DIR.

The bug arises when IMPORT_JDK contains unzipped debug info
(libjvm.debuginfo) and the make target produces zipped debug info
(libjvm.diz), or vice versa. hotspot/build/JDK_IMAGE_DIR will then
contain both libjvm.debuginfo and libjvm.diz, but only one of them
will match libjvm.so.

Finally, the JPRT make targets jprt_build_* just zips
hotspot/build/$(JDK_IMAGE_DIR). The zipped JPRT bundle will end up
having different zipped and unzipped debug info, since the
IMPORT_JDK
in JPRT contains libjv

Re: JDK-8036003: Add variable not to separate debug information.

[Daniel D. Daugherty]

On 3/18/14 12:22 PM, Andrew Hughes wrote:

- Original Message -

On 3/17/14 7:19 PM, Andrew Hughes wrote:

- Original Message -

On 3/3/14 2:49 PM, Omair Majid wrote:

* David Holmes  [2014-02-28 18:48]:

There are three pieces to all of this:

1. Generating debug symbols in the binaries (via gcc -g or whatever)
2. Generating debuginfo files (zipped or not) (FDS)
3. Stripping debug symbols from the binaries (strip-policy)

It may be that we don't have clean separation between them, and if so
that should be fixed, but I don't see the current proposal as the way
forward.

Any chance we could clean up the names too? It's not obvious to me why
FDS means 'generating debuginfo files'.

FDS stands for Full Debug Symbols and is defined that way in
quite a few Makefiles... We just call it FDS for short...


At least to me, Full Debug Symbols suggests #1 (i.e. that symbols are
generated
in the binaries), not #2. That's why it sounded so odd to me when you
suggested
turning it off, when we discussed this before.

It's also not clear why you'd want a situation where #3 would be turned
off, but
not #2, as you end up with two copies of the debug symbols. That's the
problem
I believe we have with our builds; we can turn the stripping off, but then
we
end up with duplicate debug information.

Do we need more than just the following three alternatives?

#1. No debugging information at all.
#2. Debugging information left in the original binaries.
#3. Debugging information stripped from the binaries and zipped in separate
files.

The way Oracle does it is complete debug info in the
separate files and partial debug info left in the original
binaries so you get symbol names in stack traces for those
cases where the full debug info bundles are not available.

I'm not clear whether we need a 4th alternative or if #3
covers this case.


The intent was for #3 to cover this case (i.e. whatever Oracle does now)
and for #2 to be what the GNU/Linux distributions want (i.e. binaries with
all debuginfo generated and left intact so they can do their own stripping).


Very cool. Thanks for the clarification.

Dan





Dan



It sounds to me like Oracle want #3, while distros want #2 and I imagine
some
end users may just want #1 for a faster, smaller build.

Re: JDK-8036003: Add variable not to separate debug information.

[Daniel D. Daugherty]

On 3/17/14 7:19 PM, Andrew Hughes wrote:

- Original Message -

On 3/3/14 2:49 PM, Omair Majid wrote:

* David Holmes  [2014-02-28 18:48]:

There are three pieces to all of this:

1. Generating debug symbols in the binaries (via gcc -g or whatever)
2. Generating debuginfo files (zipped or not) (FDS)
3. Stripping debug symbols from the binaries (strip-policy)

It may be that we don't have clean separation between them, and if so
that should be fixed, but I don't see the current proposal as the way
forward.

Any chance we could clean up the names too? It's not obvious to me why
FDS means 'generating debuginfo files'.

FDS stands for Full Debug Symbols and is defined that way in
quite a few Makefiles... We just call it FDS for short...


At least to me, Full Debug Symbols suggests #1 (i.e. that symbols are generated
in the binaries), not #2. That's why it sounded so odd to me when you suggested
turning it off, when we discussed this before.

It's also not clear why you'd want a situation where #3 would be turned off, but
not #2, as you end up with two copies of the debug symbols. That's the problem
I believe we have with our builds; we can turn the stripping off, but then we
end up with duplicate debug information.

Do we need more than just the following three alternatives?

#1. No debugging information at all.
#2. Debugging information left in the original binaries.
#3. Debugging information stripped from the binaries and zipped in separate 
files.


The way Oracle does it is complete debug info in the
separate files and partial debug info left in the original
binaries so you get symbol names in stack traces for those
cases where the full debug info bundles are not available.

I'm not clear whether we need a 4th alternative or if #3
covers this case.

Dan




It sounds to me like Oracle want #3, while distros want #2 and I imagine some
end users may just want #1 for a faster, smaller build.

Re: JDK-8036003: Add variable not to separate debug information.

[Daniel D. Daugherty]

On 3/3/14 2:49 PM, Omair Majid wrote:

* David Holmes  [2014-02-28 18:48]:

There are three pieces to all of this:

1. Generating debug symbols in the binaries (via gcc -g or whatever)
2. Generating debuginfo files (zipped or not) (FDS)
3. Stripping debug symbols from the binaries (strip-policy)

It may be that we don't have clean separation between them, and if so
that should be fixed, but I don't see the current proposal as the way
forward.

Any chance we could clean up the names too? It's not obvious to me why
FDS means 'generating debuginfo files'.


FDS stands for Full Debug Symbols and is defined that way in
quite a few Makefiles... We just call it FDS for short...



What do folks think of modifying the OPENJDK build to use this as the
settings for the options above by default.

1. Yes
2. No
3. No


Whatever we do here has be architecturallyconsistent across the
various OpenJDK platforms. The above settings would not work well
on non-Linux platforms so I'm not in favor of doing that.

However, I don't have a problem with providing support for config'ing
a build to work with the downstream Linux repos...



Also there may well be differences between how things are handled on the
JDK side and hotspot side.

Is there any chance of this becoming consistent between hotspot and JDK?
It would be much nicer if concepts like strip-policy were consistent
between hotspot and the rest of the JDK.


When I did the Full Debug Symbols (FDS) project, I made it architecturally
consistent  between the HotSpot and JDK sides. Since the new build system
has arrived, something of the consistency has been broken and I haven't
been back on the JDK side to see what happened...

Dan




Thanks,
Omair

Re: JDK-8036003: Add variable not to separate debug information.

[Daniel D. Daugherty]

On 3/3/14 12:11 PM, Omair Majid wrote:

Hi,

* Daniel D. Daugherty  [2014-03-03 09:57]:

On 3/1/14 4:08 PM, Mike Duigou wrote:

  If we can put debuginfo into external files why would we ever want unstripped 
binaries?

We deliver minimal stripped binaries so that can have stack traces
with some amount of information in them when the debuginfo files
are not available. We have not yet figured out how to implement
minimal stripping on Windows so Windows hs_err_pid files have no
symbols in them when the debuginfo bundles are not present.

Am I reading it right that when tools (like RPM) strip the debug
information from these binaries, hotspot will break completely when it
tries to generate a stack trace on crash?


Not quite. If you follow this process:

- use gobjcopy to copy debug info from libjvm.so -> libjvm.debuginfo
- use gobjcopy to tell libjvm.so that debug info is found in 
libjvm.debuginfo

- strip libjvm.so (I don't remember the strip everything option)

then you can still get symbols in your crashes as long as you have
BOTH libjvm.so and libjvm.debuginfo. However, if you are in a
situation where you don't have libjvm.debuginfo, then you do not
get symbols in your crashes.

When I did the Full Debug Symbols (FDS) project, I copied complete
debug info from libjvm.so -> libjvm.debuginfo and left minimal
debug info in libjvm.so so that you could still get symbols on
crashes on Linux and Solaris when the libjvm.debuginfo files
were not present.

Dan




Thanks,
Omair

Re: JDK-8036003: Add variable not to separate debug information.

[Daniel D. Daugherty]

I think we're having a problem with not all replies making it
to all the aliases. Yasumasa's reply to David below that Mike
is replying to did not arrive on any of the aliases that I'm
on... Folks need to remember to reply to all of the aliases...


On 3/1/14 4:08 PM, Mike Duigou wrote:
> On Mar 1 2014, at 06:07 , Yasumasa Suenaga  wrote:
>
>> Hi David,
>>
>>> 1. Generating debug symbols in the binaries (via gcc -g or whatever)
>>> 2. Generating debuginfo files (zipped or not) (FDS)
>>> 3. Stripping debug symbols from the binaries (strip-policy)
>>>
>>> It may be that we don't have clean separation between them, and if so
>>> that should be fixed, but I don't see the current proposal as the way
>>> forward.
>> Currently, 1. depends 2. If FDS set to disable, debug symbols (-g) are not
>> generated.

Bit of history here. When FDS is disabled, the build system is
supposed to go back to what was there prior to the FDS project.
On Linux, one of the debug configs didn't actually have any
debugging flags enabled (debug or fastdebug, I don't remember
which). So when I implemented FDS, disabling FDS meant no
debug info. Bug in the original? You betcha.


>> SEPARATED_DEBUGINFO_FILES which my patch provides can separate them.
> I think David's list (kudos) is very helpful as I haven't seen the 
> requirements articulated this clearly anywhere else.
>
> Some comments:
>
> - Are there important tools that cannot deal with external debuginfo files?

Not that I'm aware of. There have been bugs in the way gobjcopy
is extracting the debuginfo into the separate files, but no
complete failures as far as I know...


>  If we can put debuginfo into external files why would we ever want 
> unstripped binaries?

We deliver minimal stripped binaries so that can have stack traces
with some amount of information in them when the debuginfo files
are not available. We have not yet figured out how to implement
minimal stripping on Windows so Windows hs_err_pid files have no
symbols in them when the debuginfo bundles are not present.


>  Is strip policy really necessary?

Yes. STRIP_POLICY allows folks to configure what they want
for the build that they are doing. It is also meant to map
to the Oracle policy where we were granted a waiver to
deliver minimal symbols in FDS project.


> Strip policy would seem to only be necessary if there are tools which can't 
> handle external debuginfo. Assuming that everything can use external 
> debuginfo then the policy would be to strip everything.

No you don't want to strip everything. See above aboutminimal
info in stack traces.


> Do I understand correctly that rpmbuild can only deal with unstripped 
> binaries and generates the stripped rpm package and debuginfo package. It 
> sounds kind of strange that find-debuginfo.sh wouldn't be able to use already 
> generated debuginfo files.
>
> - I would expect that the flow is something like an extended version of 
> https://blogs.oracle.com/dbx/entry/creating_separate_debug_info :
>   1.  Compile source files with some form of "-g"
>   2.  Create separate debug files for object files.
>   3.  Strip object files.
>   4.  Add gnu_debuglink to object files.
>   5.  Link library or executable which should carry along links to debuginfo.
>   6a. Debugging, testing, profiling, etc. by developers.
>   6b. Packaging for program bits and debuginfo.
>   7.  Testing and Use.
>
> Hopefully I didn't get any of those steps in the wrong order. What are the 
> "you-cant-get-there-from-here" gaps and breakdowns from implementing this 
> process?
>
> - Whether for the FDS debug symbols or RPM debuginfo packaging it seems that 
> the default debug level isn't quite right. I believe that in both cases 
> what's wanted is abbreviated debug info, mostly function names and line 
> number tables, for building stack traces. This is not the debug info that is 
> currently generated.

It's STRIP_POLICY=min_strip that leaves the minimal
debug info attached to the binaries and the more
complete debug info in left in the debuginfo file(s).
And I'll disagree about only wanting abbreviated debug
info in the separate debuginfo files.


> There are four basic alternatives for levels of debug info:
> 1. (-g0)  No debug info whatsoever.
>Only exported functions and globals will have names. 
> 2. (-g1)  Abbreviated debug info. 
>All functions have names and line number information. This seems to be 
> what is needed by both FDS and RPM debuginfo files to produce nice stack 
> traces. 
> 3. (-g)   Default debugging info. 
>Adds macro expansions and local variable names. Suitable for source level 
> debugging.
> 4. (-g3 or -gdwarf-4 -fvar-tracking-assignments). Full debugging info. 
>Most suitable for source level debugging including debugging of optimized 
> code. It is not clear that anyone would want to build the entire product with 
> this option.
>
> Compilations are currently done with a mix of -g, -g1, and -gstabs. It would 
> be good to rationalize this somew

Re: JDK-8036003: Add variable not to separate debug information.

[Daniel D. Daugherty]

The proper way to fix this is to disable FDS. We should not need
yet another option to control debug info.

Dan


On 2/28/14 4:13 AM, David Holmes wrote:
> Hi,
>
> As I put in the bug report this seems way too complicated. Seems to me
> all you need to do to get what you want is not use FDS and not strip the
> symbols from the binary. The former is trivial. The latter is more
> awkward as the strip policy stuff does not work as I would want it to
> work, but still doable.
>
> David
>
> On 28/02/2014 7:18 PM, Yasumasa Suenaga wrote:
>> Hi all,
>>
>>
>> Currently, configure script can accept --disable-debug-symbols and
>> --disable-zip-debug-info as controlling to generate debug information.
>> However, current makefiles cannot build ELF binaries which is contained
>> debug information with "images" target.
>>
>> Some Linux distros use RPM as package manager.
>> RPM is built by rpmbuild command, it strips symbols and debug information
>> during to generate rpm packages.
>> https://fedoraproject.org/wiki/Packaging:Debuginfo
>>
>> For example, OpenJDK8 in Fedora20 ships libjvm.so and libjvm.debuginfo .
>> libjvm.debuginfo is generated in OpenJDK's makefiles, however it does not
>> contain debug information. Actual debug information is shipped by OpenJDK
>> debuginfo package.
>> This packaging is important when we have to debug JVM/JNI libraries.
>> If we want to use debugging tools (GDB, SystemTap, etc...), they may requires
>> debuginfo package. Debuggee (e.g. libjvm.so) points libjvm.so.debug which is
>> located at sub directory in /usr/lib/debug . It is defined in ELF section
>> (.note.gnu.build-id) of libjvm.so . However libjvm.so.debug does not contain
>> valid debug information. We need to access libjvm.debuginfo.debug at same 
>> location.
>>
>>
>> When we think to build OpenJDK rpm packages, we have to build ELF binaries
>> which contain all debug information. we should not to separate debug 
>> information.
>>
>>
>> Thus I want to add a variable "SEPARATED_DEBUGINFO_FILES" .
>> If we pass "SEPARATED_DEBUGINFO_FILES=false" to make command, "make" does not
>> execute objcopy command for generating debuginfo binaries.
>>
>> If we build rpm packages, we also have to add "STRIP_POLICY=no_strip" to 
>> arguments
>> of make command. Or ELF binaries will be stripped.
>>
>>
>> I've uploaded webrev for this enhancement.
>> This is separated 3-part: top of forest, hotspot, jdk
>> http://cr.openjdk.java.net/~ysuenaga/JDK-8036003/
>>
>> Please review it and sponsoring!
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> P.S.
>>I tried to add SEPARATED_DEBUGINFO_FILES as a configure option like
>>"--disable-separated-debug-info" .
>>I ran configure which is located at jdk9/dev directory after I ran 
>> autoconf
>>and common/autoconf/autogen.sh. However it failed.
>>
>>I guess this is caused by changeset as below.
>>   JDK-8035495: Improvements in autoconf integration
>>   http://hg.openjdk.java.net/jdk9/dev/rev/6e29cd9ac2b4
>>
>>This changes add "CHECKME" option to configure script. However, this 
>> changes
>>affects "configure" only. It should change "configure.ac" .
>>

Re: RFR: 8033580: Old debug information in IMPORT_JDK is not removed

[Daniel D. Daugherty]

> http://cr.openjdk.java.net/~ehelin/8033580/webrev.03/

make/Makefile
No comments.

Makefile style question:
Since all this ifeq(...) logic is around Makefile rules,
should the rules themselves be indented by one tab or by
one tab and some number of spaces.

I scrolled around the Makefile and I don't really see
consistent indenting of the rule lines themselves...

If you change the indenting, I don't see a reason for
another round of review.

Dan


On 2/14/14 6:21 AM, Erik Helin wrote:

Dan,

On 2014-02-14 00:29, Daniel D. Daugherty wrote:

 > http://cr.openjdk.java.net/~ehelin/8033580/webrev.01/

make/Makefile

+  ifeq ($(OSNAME), bsd)
+$(RM) -rf $(EXPORT_SERVER_DIR)/libjvm.dylib.dSYM

The above needs to be:

ifeq ($(OS_VENDOR), Darwin)
  $(RM) -rf $(EXPORT_SERVER_DIR)/libjvm.dylib.dSYM

I don't think that BSD uses .dylib stuff; that's MacOS X specific.


You are right, thanks for the correction. Please see new webrev at:
http://cr.openjdk.java.net/~ehelin/8033580/webrev.03/

Again, same testing as for prior patches were run.

Thanks,
Erik


Dan


On 2/13/14 1:59 PM, Erik Helin wrote:

Hi Dan,

thanks for reviewing! See my replies inline.

On 02/13/2014 12:40 AM, Daniel D. Daugherty wrote:

 > http://cr.openjdk.java.net/~ehelin/8033580/webrev.00/

make/Makefile
 277   ifeq ($(ZIP_DEBUGINFO_FILES),1)
 278 ifeq ($(OSNAME), windows)
 279   $(RM) -f $(EXPORT_SERVER_DIR)/jvm.map
$(EXPORT_SERVER_DIR)/jvm.pdb
 280 else
 281   $(RM) -f $(EXPORT_SERVER_DIR)/libjvm.debuginfo
 282 endif

 On MacOS X, the debuginfo will be in libjvm.dylib.dSYM so
 you'll need a MacOS X specific rule. You don't need an
 update for the JVM_VARIANT_CLIENT version because MacOS X
 doesn't support the Client VM, but if it did...


Thanks for catching this! I've uploaded a new webrev at:
http://cr.openjdk.java.net/~ehelin/8033580/webrev.01/

The same testing was done as for the first patch.

On 02/13/2014 12:40 AM, Daniel D. Daugherty wrote:

So the above change handles libjvm, but what about the
other libraries exported by HotSpot? libjsig, libjvm_db,
and libjvm_dtrace come to mind...


As we discussed, I would like to implement this for libjvm first and
then take care of the other libraries in a separate patch.

Thanks,
Erik


Dan


On 2/12/14 8:03 AM, Erik Helin wrote:

Hi all,

this patch changes how old debug information copied from 
IMPORT_JDK is

treated.

When running the copy_*_jdk target, HotSpot's makefiles copies the
entire IMPORT_JDK folder, including additional files (such as 
unzipped

debug information). The export_*_jdk targets will then, via the
generic_export target, copy the build artifacts via implicit rules to
the correct destination in hotspot/build/JDK_IMAGE_DIR.

The bug arises when IMPORT_JDK contains unzipped debug info
(libjvm.debuginfo) and the make target produces zipped debug info
(libjvm.diz), or vice versa. hotspot/build/JDK_IMAGE_DIR will then
contain both libjvm.debuginfo and libjvm.diz, but only one of them
will match libjvm.so.

Finally, the JPRT make targets jprt_build_* just zips
hotspot/build/$(JDK_IMAGE_DIR). The zipped JPRT bundle will end up
having different zipped and unzipped debug info, since the IMPORT_JDK
in JPRT contains libjvm.debuginfo and we build libjvm.diz by default.

This patch removes the debug info that we did *not* build. If we 
build

libjvm.diz, then libjvm.debuginfo will be removed (if it exists).
Correspondingly, if we build libjvm.debuginfo, then libjvm.diz 
will be

removed (if it exists).

Patch:
http://cr.openjdk.java.net/~ehelin/8033580/webrev.00/

Bug:
https://bugs.openjdk.java.net/browse/JDK-8033580

Testing:
- Building in JPRT
- Building Linux 32-bit locally on a Linux 64-bit machine
- Building Linux 64-bit locally on a Linux 64-bit machine

For all of the above, verify that only the correct debug info is
present in the output.

Thanks,
Erik

Re: RFR: 8033580: Old debug information in IMPORT_JDK is not removed

[Daniel D. Daugherty]

> http://cr.openjdk.java.net/~ehelin/8033580/webrev.01/

make/Makefile

+  ifeq ($(OSNAME), bsd)
+$(RM) -rf $(EXPORT_SERVER_DIR)/libjvm.dylib.dSYM

The above needs to be:

   ifeq ($(OS_VENDOR), Darwin)
 $(RM) -rf $(EXPORT_SERVER_DIR)/libjvm.dylib.dSYM

I don't think that BSD uses .dylib stuff; that's MacOS X specific.

Dan


On 2/13/14 1:59 PM, Erik Helin wrote:

Hi Dan,

thanks for reviewing! See my replies inline.

On 02/13/2014 12:40 AM, Daniel D. Daugherty wrote:

 > http://cr.openjdk.java.net/~ehelin/8033580/webrev.00/

make/Makefile
 277   ifeq ($(ZIP_DEBUGINFO_FILES),1)
 278 ifeq ($(OSNAME), windows)
 279   $(RM) -f $(EXPORT_SERVER_DIR)/jvm.map
$(EXPORT_SERVER_DIR)/jvm.pdb
 280 else
 281   $(RM) -f $(EXPORT_SERVER_DIR)/libjvm.debuginfo
 282 endif

 On MacOS X, the debuginfo will be in libjvm.dylib.dSYM so
 you'll need a MacOS X specific rule. You don't need an
 update for the JVM_VARIANT_CLIENT version because MacOS X
 doesn't support the Client VM, but if it did...


Thanks for catching this! I've uploaded a new webrev at:
http://cr.openjdk.java.net/~ehelin/8033580/webrev.01/

The same testing was done as for the first patch.

On 02/13/2014 12:40 AM, Daniel D. Daugherty wrote:

So the above change handles libjvm, but what about the
other libraries exported by HotSpot? libjsig, libjvm_db,
and libjvm_dtrace come to mind...


As we discussed, I would like to implement this for libjvm first and 
then take care of the other libraries in a separate patch.


Thanks,
Erik


Dan


On 2/12/14 8:03 AM, Erik Helin wrote:

Hi all,

this patch changes how old debug information copied from IMPORT_JDK is
treated.

When running the copy_*_jdk target, HotSpot's makefiles copies the
entire IMPORT_JDK folder, including additional files (such as unzipped
debug information). The export_*_jdk targets will then, via the
generic_export target, copy the build artifacts via implicit rules to
the correct destination in hotspot/build/JDK_IMAGE_DIR.

The bug arises when IMPORT_JDK contains unzipped debug info
(libjvm.debuginfo) and the make target produces zipped debug info
(libjvm.diz), or vice versa. hotspot/build/JDK_IMAGE_DIR will then
contain both libjvm.debuginfo and libjvm.diz, but only one of them
will match libjvm.so.

Finally, the JPRT make targets jprt_build_* just zips
hotspot/build/$(JDK_IMAGE_DIR). The zipped JPRT bundle will end up
having different zipped and unzipped debug info, since the IMPORT_JDK
in JPRT contains libjvm.debuginfo and we build libjvm.diz by default.

This patch removes the debug info that we did *not* build. If we build
libjvm.diz, then libjvm.debuginfo will be removed (if it exists).
Correspondingly, if we build libjvm.debuginfo, then libjvm.diz will be
removed (if it exists).

Patch:
http://cr.openjdk.java.net/~ehelin/8033580/webrev.00/

Bug:
https://bugs.openjdk.java.net/browse/JDK-8033580

Testing:
- Building in JPRT
- Building Linux 32-bit locally on a Linux 64-bit machine
- Building Linux 64-bit locally on a Linux 64-bit machine

For all of the above, verify that only the correct debug info is
present in the output.

Thanks,
Erik

Re: RFR: 8033580: Old debug information in IMPORT_JDK is not removed

[Daniel D. Daugherty]

> http://cr.openjdk.java.net/~ehelin/8033580/webrev.00/

make/Makefile
277   ifeq ($(ZIP_DEBUGINFO_FILES),1)
278 ifeq ($(OSNAME), windows)
279   $(RM) -f $(EXPORT_SERVER_DIR)/jvm.map 
$(EXPORT_SERVER_DIR)/jvm.pdb

280 else
281   $(RM) -f $(EXPORT_SERVER_DIR)/libjvm.debuginfo
282 endif

On MacOS X, the debuginfo will be in libjvm.dylib.dSYM so
you'll need a MacOS X specific rule. You don't need an
update for the JVM_VARIANT_CLIENT version because MacOS X
doesn't support the Client VM, but if it did...

So the above change handles libjvm, but what about the
other libraries exported by HotSpot? libjsig, libjvm_db,
and libjvm_dtrace come to mind...

Dan


On 2/12/14 8:03 AM, Erik Helin wrote:

Hi all,

this patch changes how old debug information copied from IMPORT_JDK is 
treated.


When running the copy_*_jdk target, HotSpot's makefiles copies the 
entire IMPORT_JDK folder, including additional files (such as unzipped 
debug information). The export_*_jdk targets will then, via the 
generic_export target, copy the build artifacts via implicit rules to 
the correct destination in hotspot/build/JDK_IMAGE_DIR.


The bug arises when IMPORT_JDK contains unzipped debug info 
(libjvm.debuginfo) and the make target produces zipped debug info 
(libjvm.diz), or vice versa. hotspot/build/JDK_IMAGE_DIR will then 
contain both libjvm.debuginfo and libjvm.diz, but only one of them 
will match libjvm.so.


Finally, the JPRT make targets jprt_build_* just zips 
hotspot/build/$(JDK_IMAGE_DIR). The zipped JPRT bundle will end up 
having different zipped and unzipped debug info, since the IMPORT_JDK 
in JPRT contains libjvm.debuginfo and we build libjvm.diz by default.


This patch removes the debug info that we did *not* build. If we build 
libjvm.diz, then libjvm.debuginfo will be removed (if it exists). 
Correspondingly, if we build libjvm.debuginfo, then libjvm.diz will be 
removed (if it exists).


Patch:
http://cr.openjdk.java.net/~ehelin/8033580/webrev.00/

Bug:
https://bugs.openjdk.java.net/browse/JDK-8033580

Testing:
- Building in JPRT
- Building Linux 32-bit locally on a Linux 64-bit machine
- Building Linux 64-bit locally on a Linux 64-bit machine

For all of the above, verify that only the correct debug info is 
present in the output.


Thanks,
Erik

Re: code review round 0 for minor FDS makefile fix (8033714)

[Daniel D. Daugherty]

Thanks for the review!

Dan


On 2/6/14 9:53 AM, Tim Bell wrote:

On 02/ 6/14 08:32 AM, Daniel D. Daugherty wrote:

Looks good to me, Dan

Tim


On 6/02/2014 9:20 AM, Daniel D. Daugherty wrote:

This code review request is going to three different aliases.
Don't use Thunderbird's "reply to list" option since it will
pick just _one_ of the _three_ lists.


Greetings,

Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS)
makefile fix our way. Here are the bug and webrev URLs:

http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/ 



8033714 hotspot 'install_jvm' bld target broken with
ZIP_DEBUGINFO_FILES=0
https://bugs.openjdk.java.net/browse/JDK-8033714

As you might guess from the bug synopsis, this fix is needed when
building without ZIP'ing the debuginfo files 
(ZIP_DEBUGINFO_FILES=0).
Based on the Graal project fix, I've updated a few other places 
where

building with FDS disabled is affected.

As always, comments and suggestions are welcome.

Dan

Re: code review round 0 for minor FDS makefile fix (8033714)

[Daniel D. Daugherty]

On 2/6/14 9:29 AM, Doug Simon wrote:

Not sure if I’m being asked for a review, but if so, looks good.


Yes, I was looking for a review. In particular because I tweaked your
original fix...

Thanks for the review!

Dan




On Feb 6, 2014, at 5:07 PM, Tom Rodriguez  wrote:


Looks good to me too.  Thanks for fixing this.

tom

On Feb 6, 2014, at 6:07 AM, Daniel D. Daugherty  
wrote:


On 2/5/14 9:28 PM, David Holmes wrote:

Hi Dan,

Looks good to me.

Thanks for the review!



(I never run the install targets :( )

Neither do I and apparently neither does JPRT... That's how this
slipped through the cracks...

Dan



Thanks,
David

On 6/02/2014 9:20 AM, Daniel D. Daugherty wrote:

This code review request is going to three different aliases.
Don't use Thunderbird's "reply to list" option since it will
pick just _one_ of the _three_ lists.


Greetings,

Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS)
makefile fix our way. Here are the bug and webrev URLs:

http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/

8033714 hotspot 'install_jvm' bld target broken with
ZIP_DEBUGINFO_FILES=0
https://bugs.openjdk.java.net/browse/JDK-8033714

As you might guess from the bug synopsis, this fix is needed when
building without ZIP'ing the debuginfo files (ZIP_DEBUGINFO_FILES=0).
Based on the Graal project fix, I've updated a few other places where
building with FDS disabled is affected.

As always, comments and suggestions are welcome.

Dan

Re: code review round 0 for minor FDS makefile fix (8033714)

[Daniel D. Daugherty]

On 2/5/14 9:28 PM, David Holmes wrote:

Hi Dan,

Looks good to me.


Thanks for the review!



(I never run the install targets :( )


Neither do I and apparently neither does JPRT... That's how this
slipped through the cracks...

Dan




Thanks,
David

On 6/02/2014 9:20 AM, Daniel D. Daugherty wrote:

This code review request is going to three different aliases.
Don't use Thunderbird's "reply to list" option since it will
pick just _one_ of the _three_ lists.


Greetings,

Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS)
makefile fix our way. Here are the bug and webrev URLs:

http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/

 8033714 hotspot 'install_jvm' bld target broken with
ZIP_DEBUGINFO_FILES=0
 https://bugs.openjdk.java.net/browse/JDK-8033714

As you might guess from the bug synopsis, this fix is needed when
building without ZIP'ing the debuginfo files (ZIP_DEBUGINFO_FILES=0).
Based on the Graal project fix, I've updated a few other places where
building with FDS disabled is affected.

As always, comments and suggestions are welcome.

Dan

Re: code review round 0 for minor FDS makefile fix (8033714)

[Daniel D. Daugherty]

Ron,

Thanks for the review.

Dan


On 2/5/14 7:04 PM, Ron Durbin wrote:

Dan

The changes look good

Ron

-Original Message-
From: Daniel D. Daugherty
Sent: Wednesday, February 05, 2014 4:21 PM
To: hotspot-runtime-...@openjdk.java.net; serviceability-...@openjdk.java.net; 
build-dev;
Doug Simon; Tom Rodriguez
Subject: code review round 0 for minor FDS makefile fix (8033714)

This code review request is going to three different aliases.
Don't use Thunderbird's "reply to list" option since it will pick just _one_ of 
the _three_
lists.


Greetings,

Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS) makefile fix 
our way. Here
are the bug and webrev URLs:

http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/

  8033714 hotspot 'install_jvm' bld target broken with
ZIP_DEBUGINFO_FILES=0
  https://bugs.openjdk.java.net/browse/JDK-8033714

As you might guess from the bug synopsis, this fix is needed when building 
without ZIP'ing
the debuginfo files (ZIP_DEBUGINFO_FILES=0).
Based on the Graal project fix, I've updated a few other places where building 
with FDS
disabled is affected.

As always, comments and suggestions are welcome.

Dan

code review round 0 for minor FDS makefile fix (8033714)

[Daniel D. Daugherty]

This code review request is going to three different aliases.
Don't use Thunderbird's "reply to list" option since it will
pick just _one_ of the _three_ lists.


Greetings,

Doug Simon and Tom Rodriguez have sent a Full Debug Symbols (FDS)
makefile fix our way. Here are the bug and webrev URLs:

http://cr.openjdk.java.net/~dcubed/8033714-webrev/0-jdk9-hs-runtime/

8033714 hotspot 'install_jvm' bld target broken with 
ZIP_DEBUGINFO_FILES=0

https://bugs.openjdk.java.net/browse/JDK-8033714

As you might guess from the bug synopsis, this fix is needed when
building without ZIP'ing the debuginfo files (ZIP_DEBUGINFO_FILES=0).
Based on the Graal project fix, I've updated a few other places where
building with FDS disabled is affected.

As always, comments and suggestions are welcome.

Dan

Re: Build failure with gnu make 4.0 on Arch Linux

[Daniel D. Daugherty]

> The Gnu make version is 4.0,

I'm pretty sure the OpenJDK builds don't work on any version of
GNUMake newer than 3.81. Checkout this version:

$ ls -l /java/devtools/linux/bin/make381
-rwxrwxr-x   1 nobody   nobody635027 Sep 16  2009 
/java/devtools/linux/bin/make381


Dan


On 11/2/13 3:24 AM, David Holmes wrote:

Hi Henry,

Looks to me like the script that tries to hack the -j option has 
messed up:


-I /home/hjen/ws/tl/common/makefiles -f adlc.make -r  -rRs -I/home/h 
-j3 -en/ws/tl/common/makefiles


Note the -j3 which seems to have been inserted into the middle of 
/hjen/ws ...


David

On 2/11/2013 1:36 PM, Henry Jen wrote:

Hi,

I am trying to setup build environment on a new installation, and
encounter following build error.

I suspect this is because of missing some required tools and software,
however, the error message is not helpful.

Tried to echo the make commang used, but as you can see the output seems
to be scrambled. Is there a way to find out what exact command causing
problem? I tried to configure --with-jobs=1, that doesn't help.

The Gnu make version is 4.0, let me know what else information I can
collect to help diagnosis the problem.

Cheers,
Henry




INFO: ZIP_DEBUGINFO_FILES=1
/usr/bin/make -s VERBOSE=-s LOG_LEVEL=warn -R -I
/home/hjen/ws/tl/common/makefiles -f adlc.make -r  -rRs -I/home/h -j3
-en/ws/tl/common/makefiles -I/home/hjen/ws/tl/common/makefiles
-I/home/hjen/ws/tl/common/makefiles
-I/home/hjen/ws/tl/common/makefiles -I/home/hjen/ws/tl/common/makefiles
/usr/bin/make: invalid option -- '/'
/usr/bin/make: invalid option -- '/'
Usage: make [options] [target] ...
Options:
  -b, -m  Ignored for compatibility.
  -B, --always-make   Unconditionally make all targets.
  -C DIRECTORY, --directory=DIRECTORY
  Change to DIRECTORY before doing 
anything.

  -d  Print lots of debugging information.
  --debug[=FLAGS] Print various types of debugging
information.
  -e, --environment-overrides
  Environment variables override makefiles.
  --eval=STRING   Evaluate STRING as a makefile statement.
  -f FILE, --file=FILE, --makefile=FILE
  Read FILE as a makefile.
  -h, --help  Print this message and exit.
  -i, --ignore-errors Ignore errors from recipes.
  -I DIRECTORY, --include-dir=DIRECTORY
  Search DIRECTORY for included makefiles.
  -j [N], --jobs[=N]  Allow N jobs at once; infinite jobs with
no arg.
  -k, --keep-goingKeep going when some targets can't be 
made.

  -l [N], --load-average[=N], --max-load[=N]
  Don't start multiple jobs unless load is
below N.
  -L, --check-symlink-times   Use the latest mtime between symlinks
and target.
  -n, --just-print, --dry-run, --recon
  Don't actually run any recipe; just
print them.
  -o FILE, --old-file=FILE, --assume-old=FILE
  Consider FILE to be very old and don't
remake it.
  -O[TYPE], --output-sync[=TYPE]
  Synchronize output of parallel jobs by
TYPE.
  -p, --print-data-base   Print make's internal database.
  -q, --question  Run no recipe; exit status says if up to
date.
  -r, --no-builtin-rules  Disable the built-in implicit rules.
  -R, --no-builtin-variables  Disable the built-in variable settings.
  -s, --silent, --quiet   Don't echo recipes.
  -S, --no-keep-going, --stop
  Turns off -k.
  -t, --touch Touch targets instead of remaking them.
  --trace Print tracing information.
  -v, --version   Print the version number of make and 
exit.

  -w, --print-directory   Print the current directory.
  --no-print-directoryTurn off -w, even if it was turned on
implicitly.
  -W FILE, --what-if=FILE, --new-file=FILE, --assume-new=FILE
  Consider FILE to be infinitely new.
  --warn-undefined-variables  Warn when an undefined variable is
referenced.

This program built for x86_64-unknown-linux-gnu
Report bugs to 
make[5]: *** [ad_stuff] Error 2
/home/hjen/ws/tl/hotspot/make/linux/makefiles/top.make:91: recipe for
target 'ad_stuff' failed
make[4]: *** [product] Error 2
/home/hjen/ws/tl/hotspot/make/linux/Makefile:289: recipe for target
'product' failed
make[3]: *** [generic_build2] Error 2
Makefile:216: recipe for target 'generic_build2' failed
make[2]: *** [product] Error 2
Makefile:167: recipe for target 'product' failed
make[1]: ***
[/home/hjen/ws/tl/build/linux-x86_64-normal-server-release/hotspot/_hotspot.timestamp] 


Error 2
HotspotWrapper.gmk:44: recipe for target
'/home/hjen/ws/tl/build/linux-x86_64-normal-server-release/hotspot/_hotspot.timestamp' 


failed
/export/home/hjen/ws/tl//common/makefiles/Main.gmk:108: recipe for
target 'hotspot-only' failed
make

Re: RFR: JDK-8027298: broken link in jdk8b113 macosx binaries

[Daniel D. Daugherty]

Please review this simple fix for FDS on macosx. The actual debuginfo
suffix on mac is dylib.dSYM, compared to other platforms where the .so
is replaced with .debuginfo. This wasn't properly handled when
generating the (somewhat weird) libjsig.diz symlink file.

Bug:https://bugs.openjdk.java.net/browse/JDK-8027298

Webrev:http://cr.openjdk.java.net/~erikj/8027298/webrev.jdk.01/  


/Erik


makefiles/Import.gmk
No comments.

Thumbs up. Thanks for catching and fixing another MacOS X goof on my part.

Dan

Re: code review round 0 for MacOS X FDS whitespace/indent fixes (8027117)

[Daniel D. Daugherty]

On 10/24/13 2:12 AM, Magnus Ihse Bursie wrote:

On 2013-10-23 22:26, Daniel D. Daugherty wrote:

Greetings,

I have some MacOS X Full Debug Symbols whitespace/indent fixes.
Here is the JDK8/HSX-25 webrev URL:

OpenJDK: 
http://cr.openjdk.java.net/~dcubed/fds_revamp/8027117-webrev/0-jdk8/
Internal: 
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/8027117-webrev/0-jdk8/


Looks good. Thank you for taking the time to keep the indentation 
consistent!


/Magnus


Thanks for the review! And thank you for pointing me at the thread
that discussed the new makefile guidelines.

Dan

Re: code review round 0 for MacOS X FDS whitespace/indent fixes (8027117)

[Daniel D. Daugherty]

Erik,

Thanks for the review!

Dan


On 10/24/13 2:05 AM, Erik Joelsson wrote:

Daniel,

Looks good to me.

/Erik

On 2013-10-23 22:26, Daniel D. Daugherty wrote:

Greetings,

I have some MacOS X Full Debug Symbols whitespace/indent fixes.
Here is the JDK8/HSX-25 webrev URL:

OpenJDK: 
http://cr.openjdk.java.net/~dcubed/fds_revamp/8027117-webrev/0-jdk8/
Internal: 
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/8027117-webrev/0-jdk8/


Because these are whitespace/indent fixes, only the "Cdiffs" and
"Udiffs" links show all the changes.

This webrev includes changes for the follow repos:

jdk8
jdk8/jdk

Gory details are below...

As always, comments, questions and suggestions are welcome.

Dan


The fixes for the following two bugs hit JDK8 at the same time, but
came up via different sub-baselines:

7165611 implement Full Debug Symbols on MacOS X hotspot
https://bugs.openjdk.java.net/browse/JDK-7165611

8001931 build-infra: Apply consistent formatting/indentation to
new build-infra files
https://bugs.openjdk.java.net/browse/JDK-8001931

During the JDK8 merge process some of the changes from 7165611 were not
properly adjusted to meet the new whitespace/indent policy implemented
by 8001931. This changeset fixes those style issues.

Testing:
- JPRT JDK8 forest build and test on all platforms (in process)

Re: code review round 0 for MacOS X FDS whitespace/indent fixes (8027117)

[Daniel D. Daugherty]

Adding back the other two aliases...

Coleen,

Thanks for the fast review!

Dan


On 10/23/13 2:50 PM, Coleen Phillimore wrote:


Looks fine.
Coleen

On 10/23/2013 4:26 PM, Daniel D. Daugherty wrote:

Greetings,

I have some MacOS X Full Debug Symbols whitespace/indent fixes.
Here is the JDK8/HSX-25 webrev URL:

OpenJDK: 
http://cr.openjdk.java.net/~dcubed/fds_revamp/8027117-webrev/0-jdk8/
Internal: 
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/8027117-webrev/0-jdk8/


Because these are whitespace/indent fixes, only the "Cdiffs" and
"Udiffs" links show all the changes.

This webrev includes changes for the follow repos:

jdk8
jdk8/jdk

Gory details are below...

As always, comments, questions and suggestions are welcome.

Dan


The fixes for the following two bugs hit JDK8 at the same time, but
came up via different sub-baselines:

7165611 implement Full Debug Symbols on MacOS X hotspot
https://bugs.openjdk.java.net/browse/JDK-7165611

8001931 build-infra: Apply consistent formatting/indentation to
new build-infra files
https://bugs.openjdk.java.net/browse/JDK-8001931

During the JDK8 merge process some of the changes from 7165611 were not
properly adjusted to meet the new whitespace/indent policy implemented
by 8001931. This changeset fixes those style issues.

Testing:
- JPRT JDK8 forest build and test on all platforms (in process)

Re: code review round 0 for MacOS X FDS whitespace/indent fixes (8027117)

[Daniel D. Daugherty]

Adding back the other two aliases...

Dan


On 10/23/13 2:50 PM, Daniel D. Daugherty wrote:

Harold,

Thanks for the fast review!

Dan

On 10/23/13 2:36 PM, harold seigel wrote:

Hi Dan,

The whitespace/indents look good.

Harold

On 10/23/2013 4:26 PM, Daniel D. Daugherty wrote:

Greetings,

I have some MacOS X Full Debug Symbols whitespace/indent fixes.
Here is the JDK8/HSX-25 webrev URL:

OpenJDK: 
http://cr.openjdk.java.net/~dcubed/fds_revamp/8027117-webrev/0-jdk8/
Internal: 
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/8027117-webrev/0-jdk8/ 



Because these are whitespace/indent fixes, only the "Cdiffs" and
"Udiffs" links show all the changes.

This webrev includes changes for the follow repos:

jdk8
jdk8/jdk

Gory details are below...

As always, comments, questions and suggestions are welcome.

Dan


The fixes for the following two bugs hit JDK8 at the same time, but
came up via different sub-baselines:

7165611 implement Full Debug Symbols on MacOS X hotspot
https://bugs.openjdk.java.net/browse/JDK-7165611

8001931 build-infra: Apply consistent formatting/indentation to
new build-infra files
https://bugs.openjdk.java.net/browse/JDK-8001931

During the JDK8 merge process some of the changes from 7165611 were not
properly adjusted to meet the new whitespace/indent policy implemented
by 8001931. This changeset fixes those style issues.

Testing:
- JPRT JDK8 forest build and test on all platforms (in process)

Re: code review round 0 for MacOS X FDS whitespace/indent fixes (8027117)

[Daniel D. Daugherty]

Jerry,

Thanks for the fast review!


On 10/23/13 2:41 PM, Gerald Thornbrugh wrote:

Hi Dan,

Your changes look good but I have a question.

On source files that do not have an Oracle copyright like generated-configure.sh
is the correct method to leave the copyright as it is?


generated-configure.sh is a special case relative to the usual (Oracle)
copyright rules. In particular, this file (should) match the version
generated if you run 'configure' on the code base. The tools that
generate the generated-configure.sh file (correctly) do not include an
Oracle style copyright.

Dan




Thanks!

Jerry

- Original Message -
From: daniel.daughe...@oracle.com
To: hotspot-runtime-...@openjdk.java.net, serviceability-...@openjdk.java.net, 
build-dev@openjdk.java.net
Sent: Wednesday, October 23, 2013 2:28:54 PM GMT -07:00 US/Canada Mountain
Subject: code review round 0 for MacOS X FDS whitespace/indent fixes (8027117)

Greetings,

I have some MacOS X Full Debug Symbols whitespace/indent fixes.
Here is the JDK8/HSX-25 webrev URL:

OpenJDK:
http://cr.openjdk.java.net/~dcubed/fds_revamp/8027117-webrev/0-jdk8/
Internal:
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/8027117-webrev/0-jdk8/

Because these are whitespace/indent fixes, only the "Cdiffs" and
"Udiffs" links show all the changes.

This webrev includes changes for the follow repos:

  jdk8
  jdk8/jdk

Gory details are below...

As always, comments, questions and suggestions are welcome.

Dan


The fixes for the following two bugs hit JDK8 at the same time, but
came up via different sub-baselines:

  7165611 implement Full Debug Symbols on MacOS X hotspot
  https://bugs.openjdk.java.net/browse/JDK-7165611

  8001931 build-infra: Apply consistent formatting/indentation to
  new build-infra files
  https://bugs.openjdk.java.net/browse/JDK-8001931

During the JDK8 merge process some of the changes from 7165611 were not
properly adjusted to meet the new whitespace/indent policy implemented
by 8001931. This changeset fixes those style issues.

Testing:
- JPRT JDK8 forest build and test on all platforms (in process)

code review round 0 for MacOS X FDS whitespace/indent fixes (8027117)

[Daniel D. Daugherty]

Greetings,

I have some MacOS X Full Debug Symbols whitespace/indent fixes.
Here is the JDK8/HSX-25 webrev URL:

OpenJDK: 
http://cr.openjdk.java.net/~dcubed/fds_revamp/8027117-webrev/0-jdk8/
Internal: 
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/8027117-webrev/0-jdk8/


Because these are whitespace/indent fixes, only the "Cdiffs" and
"Udiffs" links show all the changes.

This webrev includes changes for the follow repos:

jdk8
jdk8/jdk

Gory details are below...

As always, comments, questions and suggestions are welcome.

Dan


The fixes for the following two bugs hit JDK8 at the same time, but
came up via different sub-baselines:

7165611 implement Full Debug Symbols on MacOS X hotspot
https://bugs.openjdk.java.net/browse/JDK-7165611

8001931 build-infra: Apply consistent formatting/indentation to
new build-infra files
https://bugs.openjdk.java.net/browse/JDK-8001931

During the JDK8 merge process some of the changes from 7165611 were not
properly adjusted to meet the new whitespace/indent policy implemented
by 8001931. This changeset fixes those style issues.

Testing:
- JPRT JDK8 forest build and test on all platforms (in process)

Re: Solaris fastdebug build: gobjcopy?

[Daniel D. Daugherty]

Running configure on a machine with gobjcopy and then running the build
on a machine without gobjcopy would explain the failure mode that you're
seeing. You need to file a bug with the build-infra folks. The old build
system wouldn't have had this problem because there was no configure step.
I'm not completely up to speed on the new build-infra stuff so pinging
them is your best bet.

Dan


On 10/21/13 6:28 AM, Weijun Wang wrote:
This is a full build, but I run configure on one machine and make from 
another. Doing both on the same machine without gobjcopy runs fine.


Thanks
Max

On 10/21/13 7:38 PM, David Holmes wrote:

If gobjcopy is
installed somewhere then ALT_OBJCOPY can be set to point to it. Was this
part of a full build? The configure process might also find gobjcopy and
set the appropriate variables.

David


Thanks
Max

Re: code review round 1 for Full Debug Symbols on MacOS X hotspot (7165611)

[Daniel D. Daugherty]

On 10/15/13 8:50 AM, Daniel D. Daugherty wrote:

On 10/15/13 6:21 AM, Magnus Ihse Bursie wrote:

On 2013-10-11 22:27, Daniel D. Daugherty wrote:

Greetings,

I'm ready for code review round 1 of the FDS on MacOS X hotspot 
changes.

Below is the original code review round 0 invite (slightly edited for
clarity). Working on FDS is like pulling a thread on a sweater... so
there are four additional changed files along with more changes to
many of the files from round 0.

Here is the code review round 1 JDK8/HSX-25 webrev URL:

OpenJDK: 
http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/1-jdk8/
Internal: 
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/1-jdk8/ 



New build system parts overall looks good. Just one note: Your webrev 
is based on the code before the whitespace/indentation cleanup. 
Please try to keep to the new indentation policy (see 
http://mail.openjdk.java.net/pipermail/build-dev/2013-October/010477.html) 
when merging. That means e.g. that code like:


ifneq ($(OPENJDK_TARGET_OS), macosx)   # OBJCOPY is not used 
on MacOS X

ifneq ($(OPENJDK_TARGET_OS), windows)  # nor on Windows


Magnus,

Thanks for the heads up on the style update. I will fix the above
two lines to meet the new guidelines.



shoud be properly indented and not on the same level.


I'm guessing that the logic from which I copied this style will be
fixed in the upcoming Makefile push from the build-infra team. I'm
in sync with JDK8-B111/HSX-25-B54 so I'm guessing that these changes
haven't been pushed yet.

Dan




/Magnus


Here's the diff:

$ diff common/makefiles/NativeCompilation.gmk.cr1 
common/makefiles/NativeCompilation.gmk

439c439
< ifneq ($(OPENJDK_TARGET_OS), windows)  # nor on Windows
---
>   ifneq ($(OPENJDK_TARGET_OS), windows)  # nor on Windows
463c463
< endif # !windows
---
>   endif # !windows
523c523
< ifneq ($(OPENJDK_TARGET_OS), windows)  # nor on Windows
---
>   ifneq ($(OPENJDK_TARGET_OS), windows)  # nor on Windows
547c547
< endif # !windows
---
>   endif # !windows


I took a quick scan of the rest of patch and I didn't see anything
jump out at me style-wise. If I missing something, we can follow up
with another bug.

Dan

PS
As a HotSpot guy, I'm happy to see the indent migrating from
4 spaces (or more) down to 2 space...

Re: code review round 1 for Full Debug Symbols on MacOS X hotspot (7165611)

[Daniel D. Daugherty]

On 10/15/13 6:21 AM, Magnus Ihse Bursie wrote:

On 2013-10-11 22:27, Daniel D. Daugherty wrote:

Greetings,

I'm ready for code review round 1 of the FDS on MacOS X hotspot changes.
Below is the original code review round 0 invite (slightly edited for
clarity). Working on FDS is like pulling a thread on a sweater... so
there are four additional changed files along with more changes to
many of the files from round 0.

Here is the code review round 1 JDK8/HSX-25 webrev URL:

OpenJDK: 
http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/1-jdk8/
Internal: 
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/1-jdk8/


New build system parts overall looks good. Just one note: Your webrev 
is based on the code before the whitespace/indentation cleanup. Please 
try to keep to the new indentation policy (see 
http://mail.openjdk.java.net/pipermail/build-dev/2013-October/010477.html) 
when merging. That means e.g. that code like:


ifneq ($(OPENJDK_TARGET_OS), macosx)   # OBJCOPY is not used 
on MacOS X

ifneq ($(OPENJDK_TARGET_OS), windows)  # nor on Windows


Magnus,

Thanks for the heads up on the style update. I will fix the above
two lines to meet the new guidelines.



shoud be properly indented and not on the same level.


I'm guessing that the logic from which I copied this style will be
fixed in the upcoming Makefile push from the build-infra team. I'm
in sync with JDK8-B111/HSX-25-B54 so I'm guessing that these changes
haven't been pushed yet.

Dan




/Magnus

Re: code review round 1 for Full Debug Symbols on MacOS X hotspot (7165611)

[Daniel D. Daugherty]

Thanks!

Dan


On 10/15/13 3:33 AM, Erik Joelsson wrote:

Daniel,
Looks good to me.

/Erik

On 2013-10-14 17:52, Daniel D. Daugherty wrote:

On 10/14/13 9:18 AM, Daniel D. Daugherty wrote:

On 10/13/13 7:57 PM, David Holmes wrote:

Hmm second comment - I don't see a .m4 file change that corresponds 
to the DSYMUTIL configure change ??


Yikes! I'll check into that shortly. Not sure what happened here.


I somehow missed/lost two 1-liner changes:

hg diff common/autoconf/basics.m4 common/autoconf/spec.gmk.in
diff -r 4faa09c7fe55 common/autoconf/basics.m4
--- a/common/autoconf/basics.m4 Wed Oct 02 15:08:03 2013 +0200
+++ b/common/autoconf/basics.m4 Mon Oct 14 08:49:37 2013 -0700
@@ -644,6 +644,7 @@ fi
 fi

 if test "x$OPENJDK_TARGET_OS" = "xmacosx"; then
+  BASIC_REQUIRE_PROG(DSYMUTIL, dsymutil)
   BASIC_REQUIRE_PROG(XATTR, xattr)
   AC_PATH_PROG(CODESIGN, codesign)
   if test "x$CODESIGN" != "x"; then
diff -r 4faa09c7fe55 common/autoconf/spec.gmk.in
--- a/common/autoconf/spec.gmk.in   Wed Oct 02 15:08:03 2013 +0200
+++ b/common/autoconf/spec.gmk.in   Mon Oct 14 08:49:37 2013 -0700
@@ -484,6 +484,7 @@ DATE:=@DATE@
 DATE:=@DATE@
 DIFF:=@DIFF@
 DIRNAME:=@DIRNAME@
+DSYMUTIL:=@DSYMUTIL@
 FIND:=@FIND@
 FIND_DELETE:=@FIND_DELETE@
 ECHO:=@ECHO@


I don't think that these two 1-liners are worth re-rolling the webrev.
Please let me know if you don't agree.

Dan




Dan




Thanks,
David

On 12/10/2013 6:27 AM, Daniel D. Daugherty wrote:

Greetings,

I'm ready for code review round 1 of the FDS on MacOS X hotspot 
changes.

Below is the original code review round 0 invite (slightly edited for
clarity). Working on FDS is like pulling a thread on a sweater... so
there are four additional changed files along with more changes to
many of the files from round 0.

Here is the code review round 1 JDK8/HSX-25 webrev URL:

OpenJDK:
http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/1-jdk8/
Internal:
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/1-jdk8/ 



Changes from code review round 0 to round 1:

   root repo:
 - add configure support for DSYMUTIL macro

   hotspot repo:
 - drop Minimal1 config support for FDS on MacOS X
 - use DSYMUTIL macro instead of literal 'dsymutil'
 - fix FASTDEBUG_CFLAGS typo and add USE_CLANG support
 - STRIP is not used on MacOS X
 - fix $(UNIVERSAL_COPY_LIST) rule
 - refine install-dir function

   jdk repo:
 - add prep-target-dir, install-import-dir, and
install-import-debuginfo
   functions
 - support importing .dSYM directories from hotspot
 - create libjsig.dSYM symlink from VM specific directory to
   the real location

Files changed from code review round 0 to round 1:

 common/autoconf/generated-configure.sh
 hotspot/make/Makefile
 hotspot/make/bsd/makefiles/defs.make
 hotspot/make/bsd/makefiles/dtrace.make
 hotspot/make/bsd/makefiles/gcc.make
 hotspot/make/bsd/makefiles/jsig.make
 hotspot/make/bsd/makefiles/saproc.make
 hotspot/make/bsd/makefiles/universal.gmk
 hotspot/make/bsd/makefiles/vm.make
 hotspot/make/defs.make

New files changed in code review round 1:

 hotspot/make/bsd/makefiles/product.make
 jdk/make/common/Defs.gmk
 jdk/make/java/redist/Makefile
 jdk/makefiles/Import.gmk

Additional bugs filed based on code review comments, testing and
just plain looking at the Makefiles:

 8026280 implement Full Debug Symbols on MacOS X jdk
 8026281 hotspot 'jdk6_or_earlier logic' for FDS needs to be 
removed

 8026282 the '64' subdir logic in Makefiles might be removable
 8026283 literal 'lipo' cmd uses in HotSpot makefiles
 8026284 Minimal1 references/support should be removed from 
BSD/MacOS X

 8026285 JPRT -buildenv ZIP_DEBUGINFO_FILES=0 no longer works

As always, comments, questions and suggestions are welcome.

Dan

 > 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
 >
 > 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' 

Re: code review round 1 for Full Debug Symbols on MacOS X hotspot (7165611)

[Daniel D. Daugherty]

Nothing like a willing victim^H^H^H^H^H^Htest user of your changes!

Thanks!

Dan


On 10/15/13 12:30 AM, Staffan Larsen wrote:

I've been using this patch while debugging other issues and I now get full 
symbols in the debugger without a lot of manual work. So it's a go!

Thanks,
/Staffan

On 11 okt 2013, at 22:27, Daniel D. Daugherty  
wrote:


Greetings,

I'm ready for code review round 1 of the FDS on MacOS X hotspot changes.
Below is the original code review round 0 invite (slightly edited for
clarity). Working on FDS is like pulling a thread on a sweater... so
there are four additional changed files along with more changes to
many of the files from round 0.

Here is the code review round 1 JDK8/HSX-25 webrev URL:

OpenJDK: http://cr.openjdk.java.net/~dcubed/fds_revamp/7165611-webrev/1-jdk8/
Internal: 
http://javaweb.us.oracle.com/~ddaugher/fds_revamp/7165611-webrev/1-jdk8/

Changes from code review round 0 to round 1:

  root repo:
- add configure support for DSYMUTIL macro

  hotspot repo:
- drop Minimal1 config support for FDS on MacOS X
- use DSYMUTIL macro instead of literal 'dsymutil'
- fix FASTDEBUG_CFLAGS typo and add USE_CLANG support
- STRIP is not used on MacOS X
- fix $(UNIVERSAL_COPY_LIST) rule
- refine install-dir function

  jdk repo:
- add prep-target-dir, install-import-dir, and install-import-debuginfo
  functions
- support importing .dSYM directories from hotspot
- create libjsig.dSYM symlink from VM specific directory to
  the real location

Files changed from code review round 0 to round 1:

common/autoconf/generated-configure.sh
hotspot/make/Makefile
hotspot/make/bsd/makefiles/defs.make
hotspot/make/bsd/makefiles/dtrace.make
hotspot/make/bsd/makefiles/gcc.make
hotspot/make/bsd/makefiles/jsig.make
hotspot/make/bsd/makefiles/saproc.make
hotspot/make/bsd/makefiles/universal.gmk
hotspot/make/bsd/makefiles/vm.make
hotspot/make/defs.make

New files changed in code review round 1:

hotspot/make/bsd/makefiles/product.make
jdk/make/common/Defs.gmk
jdk/make/java/redist/Makefile
jdk/makefiles/Import.gmk

Additional bugs filed based on code review comments, testing and
just plain looking at the Makefiles:

8026280 implement Full Debug Symbols on MacOS X jdk
8026281 hotspot 'jdk6_or_earlier logic' for FDS needs to be removed
8026282 the '64' subdir logic in Makefiles might be removable
8026283 literal 'lipo' cmd uses in HotSpot makefiles
8026284 Minimal1 references/support should be removed from BSD/MacOS X
8026285 JPRT -buildenv ZIP_DEBUGINFO_FILES=0 no longer works

As always, comments, questions and suggestions are welcome.

Dan


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

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