Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v6]
On Wed, 9 Jun 2021 19:00:39 GMT, Leonid Mesnik wrote: >> EFH is improved to process cores and get mixed stack traces with jhsdb and >> native stack traces with gdb/lldb. It might be useful because hs_err doesn't >> contain info about all threads, sometimes it is even not generated. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > fxies Marked as reviewed by iignatyev (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v6]
> EFH is improved to process cores and get mixed stack traces with jhsdb and > native stack traces with gdb/lldb. It might be useful because hs_err doesn't > contain info about all threads, sometimes it is even not generated. Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision: fxies - Changes: - all: https://git.openjdk.java.net/jdk/pull/4234/files - new: https://git.openjdk.java.net/jdk/pull/4234/files/e70518bc..67b61d01 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4234=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4234=04-05 Stats: 7 lines in 4 files changed: 2 ins; 5 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4234/head:pull/4234 PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
On Wed, 2 Jun 2021 01:00:53 GMT, Leonid Mesnik wrote: >> EFH is improved to process cores and get mixed stack traces with jhsdb and >> native stack traces with gdb/lldb. It might be useful because hs_err doesn't >> contain info about all threads, sometimes it is even not generated. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > spaces updated. updated diff - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
On Wed, 9 Jun 2021 08:42:13 GMT, Igor Ignatyev wrote: >> Leonid Mesnik has updated the pull request incrementally with one additional >> commit since the last revision: >> >> spaces updated. > > test/failure_handler/src/share/classes/jdk/test/failurehandler/GathererFactory.java > line 32: > >> 30: import java.io.FileWriter; >> 31: import java.io.PrintWriter; >> 32: import java.nio.file.Files; > > I don't see why we need these 3 new imports. fixed > test/failure_handler/src/share/classes/jdk/test/failurehandler/ToolKit.java > line 28: > >> 26: import jdk.test.failurehandler.action.ActionSet; >> 27: import jdk.test.failurehandler.action.ActionHelper; >> 28: import jdk.test.failurehandler.action.PatternAction; > > redundant import fixed > test/failure_handler/src/share/conf/mac.properties line 71: > >> 69: native.lldb.app=lldb >> 70: native.lldb.delimiter=\0 >> 71: native.lldb.args=--core\0%p\0%java\0-o\0thread backtrace all\0-o\0quit > > could you please add a comment similar to the one in common.properties file? fixed > test/failure_handler/src/share/conf/mac.properties line 72: > >> 70: native.lldb.delimiter=\0 >> 71: native.lldb.args=--core\0%p\0%java\0-o\0thread backtrace all\0-o\0quit >> 72: native.lldb.params.timeout=360 > > why does `lldb` require an increases timeout, but `gdb` and `jhsdb` do not? Not sure I remember if there is any reason. I remove it. Let increase it later if it actually needed. - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
On Wed, 2 Jun 2021 01:00:53 GMT, Leonid Mesnik wrote: >> EFH is improved to process cores and get mixed stack traces with jhsdb and >> native stack traces with gdb/lldb. It might be useful because hs_err doesn't >> contain info about all threads, sometimes it is even not generated. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > spaces updated. Changes requested by iignatyev (Reviewer). test/failure_handler/src/share/classes/jdk/test/failurehandler/GathererFactory.java line 32: > 30: import java.io.FileWriter; > 31: import java.io.PrintWriter; > 32: import java.nio.file.Files; I don't see why we need these 3 new imports. test/failure_handler/src/share/classes/jdk/test/failurehandler/ToolKit.java line 28: > 26: import jdk.test.failurehandler.action.ActionSet; > 27: import jdk.test.failurehandler.action.ActionHelper; > 28: import jdk.test.failurehandler.action.PatternAction; redundant import test/failure_handler/src/share/conf/linux.properties line 62: > 60: cores=native.gdb > 61: native.gdb.app=gdb > 62: native.gdb.args=%java\0-c\0%p\0-batch\0-ex\0thread apply all backtrace could you please add a comment similar to the one in `common.properties` file? test/failure_handler/src/share/conf/mac.properties line 71: > 69: native.lldb.app=lldb > 70: native.lldb.delimiter=\0 > 71: native.lldb.args=--core\0%p\0%java\0-o\0thread backtrace all\0-o\0quit could you please add a comment similar to the one in common.properties file? test/failure_handler/src/share/conf/mac.properties line 72: > 70: native.lldb.delimiter=\0 > 71: native.lldb.args=--core\0%p\0%java\0-o\0thread backtrace all\0-o\0quit > 72: native.lldb.params.timeout=360 why does `lldb` require an increases timeout, but `gdb` and `jhsdb` do not? - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
On Fri, 28 May 2021 02:14:45 GMT, Igor Ignatyev wrote:
>> Leonid Mesnik has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> spaces updated.
>
> test/failure_handler/src/share/classes/jdk/test/failurehandler/action/PatternAction.java
> line 63:
>
>> 61: }
>> 62: for (int i = 0, n = args.length; i < n; ++i) {
>> 63: args[i] = args[i].replace("%java",
>> helper.findApp("java").getAbsolutePath());
>
> are we sure that `java` from `PATH` (which is what `findApp` returns) is the
> right `java`?
The paths contain testJdk and compileJdk before PATH. We use it to find any of
our tools.
-
PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes
On Fri, 28 May 2021 00:54:21 GMT, Leonid Mesnik wrote: >> Hi Leonid, >> >> What is EFH? Please update the bug and PR to explain. >> >> Thanks, >> David > >> Hi Leonid, >> >> What is EFH? Please update the bug and PR to explain. >> >> Thanks, >> David > > Updated summary to "Improve jtreg test failure handler do get native/mixed > stack traces for cores and live processes". > @lmesnik , how has this been tested? I used it in the loom for several weeks. - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
On Fri, 28 May 2021 02:25:59 GMT, Igor Ignatyev wrote: >> Leonid Mesnik has updated the pull request incrementally with one additional >> commit since the last revision: >> >> spaces updated. > > @lmesnik , how has this been tested? @iignatev, thank you for your comments. I updated all of them. - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
On Fri, 28 May 2021 02:20:04 GMT, Igor Ignatyev wrote: >> Leonid Mesnik has updated the pull request incrementally with one additional >> commit since the last revision: >> >> spaces updated. > > test/failure_handler/Makefile line 41: > >> 39: CONF_DIR = src/share/conf >> 40: >> 41: JAVA_RELEASE = 7 > > could you please update `DEPENDENCES` section in > `test/failure_handler/README`? done - PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v5]
> EFH is improved to process cores and get mixed stack traces with jhsdb and > native stack traces with gdb/lldb. It might be useful because hs_err doesn't > contain info about all threads, sometimes it is even not generated. Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision: spaces updated. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4234/files - new: https://git.openjdk.java.net/jdk/pull/4234/files/57d76163..e70518bc Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4234=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4234=03-04 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4234/head:pull/4234 PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v4]
> EFH is improved to process cores and get mixed stack traces with jhsdb and > native stack traces with gdb/lldb. It might be useful because hs_err doesn't > contain info about all threads, sometimes it is even not generated. Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision: remove unused code. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4234/files - new: https://git.openjdk.java.net/jdk/pull/4234/files/c48542b5..57d76163 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4234=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4234=02-03 Stats: 4 lines in 1 file changed: 0 ins; 3 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4234/head:pull/4234 PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v3]
> EFH is improved to process cores and get mixed stack traces with jhsdb and > native stack traces with gdb/lldb. It might be useful because hs_err doesn't > contain info about all threads, sometimes it is even not generated. Leonid Mesnik has updated the pull request incrementally with one additional commit since the last revision: README updated. - Changes: - all: https://git.openjdk.java.net/jdk/pull/4234/files - new: https://git.openjdk.java.net/jdk/pull/4234/files/cb1eb944..c48542b5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4234=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4234=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4234/head:pull/4234 PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes [v2]
> EFH is improved to process cores and get mixed stack traces with jhsdb and > native stack traces with gdb/lldb. It might be useful because hs_err doesn't > contain info about all threads, sometimes it is even not generated. Leonid Mesnik has updated the pull request incrementally with two additional commits since the last revision: - Merge branch 'efh' of https://github.com/lmesnik/jdk into efh - updated after comments from Igor - Changes: - all: https://git.openjdk.java.net/jdk/pull/4234/files - new: https://git.openjdk.java.net/jdk/pull/4234/files/68fd69d9..cb1eb944 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4234=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4234=00-01 Stats: 47 lines in 7 files changed: 9 ins; 31 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/4234.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4234/head:pull/4234 PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes
On Thu, 27 May 2021 22:05:55 GMT, Leonid Mesnik wrote:
> EFH is improved to process cores and get mixed stack traces with jhsdb and
> native stack traces with gdb/lldb. It might be useful because hs_err doesn't
> contain info about all threads, sometimes it is even not generated.
@lmesnik , how has this been tested?
test/failure_handler/Makefile line 41:
> 39: CONF_DIR = src/share/conf
> 40:
> 41: JAVA_RELEASE = 7
could you please update `DEPENDENCES` section in `test/failure_handler/README`?
test/failure_handler/src/share/classes/jdk/test/failurehandler/ToolKit.java
line 69:
> 67: }
> 68: } catch (IOException ioe) {
> 69: // just silently skip gzipped core processing
we don't silently ignore exceptions in `failure_handler`, we tend to print them
so we at least have some echoes of what happened.
test/failure_handler/src/share/classes/jdk/test/failurehandler/ToolKit.java
line 71:
> 69: // just silently skip gzipped core processing
> 70: }
> 71: unpackedCore.toFile().delete();
Suggestion:
Paths.delete(unpackedCore);
``` ?
test/failure_handler/src/share/classes/jdk/test/failurehandler/Utils.java line
73:
> 71: InputStream stream =
> Utils.class.getResourceAsStream(resourceName);
> 72:
> 73: // EFH_CONF_DIR might re-defined to load custom configs for
> development purposes
this also seems to be unrelated to the subject and does require documentation
in `test/failure_handler/README`.
test/failure_handler/src/share/classes/jdk/test/failurehandler/action/PatternAction.java
line 63:
> 61: }
> 62: for (int i = 0, n = args.length; i < n; ++i) {
> 63: args[i] = args[i].replace("%java",
> helper.findApp("java").getAbsolutePath());
are we sure that `java` from `PATH` (which is what `findApp` returns) is the
right `java`?
test/failure_handler/src/share/conf/common.properties line 38:
> 36: jcmd.vm.system_properties \
> 37: jcmd.gc.heap_info jcmd.gc.class_histogram jcmd.gc.finalizer_info \
> 38: jcmd.vm.info \
this seems to be unrelated to the RFE you are working on.
test/failure_handler/src/share/conf/common.properties line 67:
> 65: cores=jhsdb.jstack
> 66: jhsdb.jstack.app=jhsdb
> 67: jhsdb.jstack.args=jstack --mixed --core %p --exe %java
strictly speaking, we can't guarantee that the executable is always `bin/java`,
but it's the most common and the most interesting case, so this assumption is
good enough for our pourposes.
could you please add a comment explaining this so the further reading won't be
puzzled?
test/failure_handler/src/share/conf/mac.properties line 68:
> 66: native.jhsdb.app=bash
> 67: native.jhsdb.jstack.delimiter=\0
> 68: native.jhsdb.jstack.args=-c\0DevToolsSecurity --status | grep -q enabled
> && jhsdb jstack --mixed --pid %p
AFAICS `jhsdb` does check "Developer mode" on macos before trying to attach and
will just report 'lack of privileges' (as opposed to hanging with a modal
window asking for permission), so you can move `jshdb.jstack` to
`common.properties`.
-
Changes requested by iignatyev (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4234
Re: RFR: 8267893: Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes
On Thu, 27 May 2021 23:38:48 GMT, David Holmes wrote: > Hi Leonid, > > What is EFH? Please update the bug and PR to explain. > > Thanks, > David Updated summary to "Improve jtreg test failure handler do get native/mixed stack traces for cores and live processes". - PR: https://git.openjdk.java.net/jdk/pull/4234
