Hi Chris,
I do not have [diff] section both ~/.hgrc and <jdk src>/.hg/hgrc.
In particular I've not edited .hg/hgrc (except defpath).
It seems to be nature at least hg of OpenJDK.
For example, [1] moves (and make some changes) BaseFileManager, but the
changeset removes old file and adds new one.
Yasumasa
[1] http://hg.openjdk.java.net/jdk10/master/rev/8f8e54a1fa20
On 2019/11/05 13:50, Chris Plummer wrote:
On 11/4/19 8:47 PM, Chris Plummer wrote:
On 11/4/19 5:19 PM, Yasumasa Suenaga wrote:
Hi Chris,
On 2019/11/05 2:28, Chris Plummer wrote:
On 11/2/19 5:33 AM, Yasumasa Suenaga wrote:
Hi Chris,
On 2019/11/02 7:07, Chris Plummer wrote:
Hi Yasumasa,
I can't comment on the build changes. I don't know how the build works well
enough.
I ensured it on submit repo
(mach5-one-ysuenaga-JDK-8233285-1-20191101-0746-6336009).
This change affects for Linux only. So I changed toolchain if SA would be built
for Linux only.
LinuxDebuggerLocal.cpp should show up as a rename + diffs, not as a new file.
I uploaded it:
http://cr.openjdk.java.net/~ysuenaga/JDK-8233285/webrev.00/LinuxDebuggerLocal.patch
If we can use Git, it can show "rename + diffs"...
I tried to use "hg rename" and "hg move", but the result did not change.
Hi Yasumasa,
I just did an "hg mv" and then modified the file, and webrev did what it is
suppose to do and showed just the diffs, but also indicated that the file was moved.
Which version of webrev are you using?
I'm using wevrev changeset: 24:8cd091802cd0 with Mercurial 4.5.3 on Ubuntu
18.04 LTS.
What do "hg diff" and "hg status" show?
For example, rename Makefile to Makefile.orig:
```
$ hg mv Makefile Makefile.orig
$ hg status
A Makefile.orig
R Makefile
This part looks correct.
$ hg diff
diff -r c41d1303a87c Makefile
--- a/Makefile Mon Nov 04 13:02:40 2019 -0800
+++ /dev/null Thu Jan 01 00:00:00 1970 +0000
@@ -1,64 +0,0 @@
-#
-# Copyright (c) 2012, 2015, Oracle and/or its affiliates. All rights reserved.
-# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
:
(snip)
```
This part is odd. Not sure why it says "diff -r". Mine looks like:
$ hg status
A test/hotspot/jtreg/serviceability/sa/CDSJMapClstats2.java
R test/hotspot/jtreg/serviceability/sa/CDSJMapClstats.java
$ hg diff
diff --git a/test/hotspot/jtreg/serviceability/sa/CDSJMapClstats.java
b/test/hotspot/jtreg/serviceability/sa/CDSJMapClstats2.java
rename from test/hotspot/jtreg/serviceability/sa/CDSJMapClstats.java
rename to test/hotspot/jtreg/serviceability/sa/CDSJMapClstats2.java
--- a/test/hotspot/jtreg/serviceability/sa/CDSJMapClstats.java
+++ b/test/hotspot/jtreg/serviceability/sa/CDSJMapClstats2.java
@@ -40,7 +40,7 @@
<snip>
..and the actual diff follows the above, which is just a one line edit. Do you have an
override of "diff" in your .hgrc?
I should have mention what's in my .hgrc. because I just noticed something:
diff =
[diff]
git=1
nodates=1
Don't ask me why I have this. I cloned someones .hgrc when openjdk first moved to Mercurial, and
have never touched this part of it. At the very least the "git=1" would explain why my
diff output says "diff --git ...".
Chris
BTW, my Mercurial is 3.6.
Chris
It seems to be a problem in hg instead of webrev.
Thanks,
Yasumasa
thanks,
Chris
The rest of the changes look fine.
Thanks!
Yasumasa
thanks,
Chris
On 11/1/19 1: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:
0x00007ff255a8fa4c
_ZN9JavaCalls11call_helperEP9JavaValueRK12methodHandleP17JavaCallArgumentsP6Thread
+ 0x6ac
0x00007ff255a8cc1d
_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:
0x00007ff1aba20a4c JavaCalls::call_helper(JavaValue*, methodHandle const&,
JavaCallArguments*, Thread*) + 0x6ac
0x00007ff1aba1dc1d 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
