Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]

[Erik Gahlin]

On Wed, 11 May 2022 12:59:49 GMT, Magnus Ihse Bursie  wrote:

>> Adam Sotona has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   8244681: Add a warning for possibly lossy conversion in compound 
>> assignments
>>   recommended correction of the warning description
>
> make/modules/jdk.jfr/Java.gmk line 26:
> 
>> 24: #
>> 25: 
>> 26: DISABLED_WARNINGS_java += exports lossy-conversions
> 
> Note that with the fix of JDK-8286392 (and JDK-8286396) the 
> `lossy-conversions` warning should not be disabled for the JFR code. 
> 
> In general, you need to check which of the subtasks of JDK-8286374 that has 
> been fixed, and adjust the makefiles accordingly, before pushing this fix.
> 
> (In the future, it might be easier to push the fix which disables the 
> warnings first, and then file follow-up bugs on aa per-component basis, and 
> remind them to remove the disabling in the makefile. That way there won't be 
> a race between individual fixes and a "master" bug like this.)

I agree, but if it doesn't happen, I can follow up with a separate PR where I 
remove the disablement.

-

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

Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]

[Erik Gahlin]

On Wed, 11 May 2022 07:45:39 GMT, Adam Sotona  wrote:

>> Please review this patch adding new lint option, **lossy-conversions**, to 
>> javac to warn about type casts in compound assignments with possible lossy 
>> conversions.
>> 
>> The new lint warning is shown if the type of the right-hand operand of a 
>> compound assignment is not assignment compatible with the type of the 
>> variable.
>> 
>> The implementation of the warning is based on similar check performed to 
>> emit "possible lossy conversion" compilation error for simple assignments. 
>> 
>> Proposed patch also include complex matrix-style test with positive and 
>> negative test cases of lossy conversions in compound assignments.
>> 
>> Proposed patch also disables this new lint option in all affected JDK 
>> modules and libraries to allow smooth JDK build. Individual cases to address 
>> possibly lossy conversions warnings in JDK are already addressed in a 
>> separate umbrella issue and its sub-tasks.
>> 
>> Thanks for your review,
>> Adam
>
> Adam Sotona has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8244681: Add a warning for possibly lossy conversion in compound assignments
>   recommended correction of the warning description

Lossy conversion issues for jdk.jfr and jdk.management.jfr. have been fixed.

-

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

Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

[Erik Gahlin]

On Fri, 29 Apr 2022 18:27:27 GMT, Mikhailo Seledtsov  
wrote:

>> Alan Bateman has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e
>
> test/jdk/jdk/jfr/api/consumer/TestManyRecordings.java line 57:
> 
>> 55: int classLoaderCount = Integer.parseInt(args[0]);
>> 56: int classCount = Integer.parseInt(args[1]);
>> 57: for (int i = 0; i  
> Did you mean classLoaderCount here instead of classCount? Also, please make 
> sure there is a space between < and classLoaderCount.

The JFR "tests" look strange. I would expect a test called TestManyRecording to 
start recordings, not created a lot of classes, similar to TestManyClasses. How 
is this related to Loom?  Could this be a merge gone bad?

-

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

Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

[Erik Gahlin]

On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

Marked as reviewed by egahlin (Reviewer).

-

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

Re: RFR: 8284161: Implementation of Virtual Threads (Preview) [v8]

[Erik Gahlin]

On Wed, 27 Apr 2022 14:24:20 GMT, Alan Bateman  wrote:

>> This is the implementation of JEP 425: Virtual Threads (Preview); TBD which 
>> JDK version to target.
>> 
>> We will refresh this PR periodically to pick up changes and fixes from the 
>> loom repo.
>> 
>> Most of the new mechanisms in the HotSpot VM are disabled by default and 
>> require running with `--enable-preview` to enable.
>> 
>> The patch has support for x64 and aarch64 on the usual operating systems 
>> (Linux, macOS, and Windows). There are stubs (calling Unimplemented) for 
>> zero and some of the other ports. Additional ports can be contributed via 
>> PRs against the fibers branch in the loom repo.
>> 
>> There are changes in many areas. To reduce notifications/mails, the labels 
>> have been trimmed down for now to hotspot, serviceability and core-libs. 
>> We'll add the complete set of labels when the PR is further along.
>> 
>> The changes include a refresh of java.util.concurrent and ForkJoinPool from 
>> Doug Lea's CVS. These changes will probably be proposed and integrated in 
>> advance of this PR.
>> 
>> The changes include some non-exposed and low-level infrastructure to support 
>> the (in draft) JEPs for Structured Concurrency and Scope Locals. This is to 
>> make life a bit easier and avoid having to separate VM changes and juggle 
>> branches at this time.
>
> Alan Bateman has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Refresh 7965cc6b168e567ac2596f2fbc3b00a7d99b7e1e

src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp line 168:

> 166:   assert(!tl->is_excluded(), "invariant");
> 167:   if (virtual_thread) {
> 168: // TODO: blob cache for virtual threads

Fix this now or after integration?

src/hotspot/share/jfr/metadata/metadata.xml line 121:

> 119: thread="true" stackTrace="true">
> 120:  description="Thread enlisted as a carrier" />
> 121:  description="Class of the continuation" />

"Continuation class" = >" Continuation Class"
numFrames = frames
numRefs = references
"Number of interpreted frames" => "Interpreted Frames"
"Number of references" => "References"
"Stack size in bytes" => "Stack Size" contentType"bytes"

src/hotspot/share/jfr/metadata/metadata.xml line 130:

> 128: thread="true" stackTrace="true">
> 129:  description="Thread enlisted as a carrier" />
> 130:  description="Class of the continuation" />

contClass => continuationClass
"Continuation class" => "Continuation Class"
"Class of the continuation" Remove (not needed)
"Number of interpreted frames" => "Interpreted Frames"
numFrames => frames
"Number of references" => "References"
numRefs => references
"Stack size in bytes" => "Stack Size" contentType="bytes"

src/hotspot/share/jfr/metadata/metadata.xml line 138:

> 136:category="Java Virtual Machine" label="Continuation Freeze Young" 
> thread="true" stackTrace="false" startTime="false">
> 137: 
> 138: 

"Allocated new" => "Allocated New"

src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp line 94:

> 92: static const size_t global_buffer_size = 512 * K;
> 93: 
> 94: static const size_t thread_local_buffer_prealloc_count = 32;

Why is more memory needed? Moore's law or something specific to virtual threads?

src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointWriter.cpp line 81:

> 79:   be_writer.write(size);
> 80:   be_writer.write(time);
> 81:   be_writer.write(JfrTicks::now().value() - time);

Why is this changed?

src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadEndEvent.java line 35:

> 33: 
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread end")

"Virtual thread end" => "Virtual Thread End"
Remove description.

src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadPinnedEvent.java line 35:

> 33: 
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread pinned")

"Virtual thread pinned" => "Virtual Thread Pinned"
Remove description

src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadStartEvent.java line 35:

> 33: 
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread start")

"virtual thread start" => "Virtual Thread Start"
Remove description

src/jdk.jfr/share/classes/jdk/jfr/events/VirtualThreadSubmitFailedEvent.java 
line 35:

> 33: 
> 34: @Category({"Java Runtime"})
> 35: @Label("Virtual thread submit task failed")

The label is a bit a long, would "Virtual Thread Submit Failed" work?

-

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

Re: RFR: 8280035: Use Class.isInstance instead of Class.isAssignableFrom where applicable

[Erik Gahlin]

On Thu, 13 Jan 2022 08:39:04 GMT, Andrey Turbanov  wrote:

>> Method `Class.isAssignableFrom` is often used in form of:
>> 
>> if (clazz.isAssignableFrom(obj.getClass())) {
>> Such condition could be simplified to more shorter and performarnt code
>> 
>> if (clazz.isInstance(obj)) {
>> 
>> Replacement is equivalent if it's known that `obj != null`.
>> In JDK codebase there are many such places.
>> 
>> I tried to measure performance via JMH
>> 
>> Class integerClass = Integer.class;
>> Class numberClass = Number.class;
>> 
>> Object integerObject = 45666;
>> Object doubleObject = 8777d;
>> 
>> @Benchmark
>> public boolean integerInteger_isInstance() {
>> return integerClass.isInstance(integerObject);
>> }
>> 
>> @Benchmark
>> public boolean integerInteger_isAssignableFrom() {
>> return integerClass.isAssignableFrom(integerObject.getClass());
>> }
>> 
>> @Benchmark
>> public boolean integerDouble_isInstance() {
>> return integerClass.isInstance(doubleObject);
>> }
>> 
>> @Benchmark
>> public boolean integerDouble_isAssignableFrom() {
>> return integerClass.isAssignableFrom(doubleObject.getClass());
>> }
>> 
>> @Benchmark
>> public boolean numberDouble_isInstance() {
>> return numberClass.isInstance(doubleObject);
>> }
>> 
>> @Benchmark
>> public boolean numberDouble_isAssignableFrom() {
>> return numberClass.isAssignableFrom(doubleObject.getClass());
>> }
>> 
>> @Benchmark
>> public boolean numberInteger_isInstance() {
>> return numberClass.isInstance(integerObject);
>> }
>> 
>> @Benchmark
>> public boolean numberInteger_isAssignableFrom() {
>> return numberClass.isAssignableFrom(integerObject.getClass());
>> }
>> 
>> @Benchmark
>> public void numberIntegerDouble_isInstance(Blackhole bh) {
>> bh.consume(numberClass.isInstance(integerObject));
>> bh.consume(numberClass.isInstance(doubleObject));
>> }
>> 
>> @Benchmark
>> public void integerIntegerDouble_isAssignableFrom(Blackhole bh) {
>> bh.consume(integerClass.isAssignableFrom(integerObject.getClass()));
>> bh.consume(integerClass.isAssignableFrom(doubleObject.getClass()));
>> }
>> 
>> `isInstance` is a bit faster than `isAssignableFrom`
>> 
>> Benchmark  Mode  Cnt  Score   Error  Units
>> integerDouble_isAssignableFrom avgt5  1,173 ± 0,026  ns/op
>> integerDouble_isInstance   avgt5  0,939 ± 0,038  ns/op
>> integerIntegerDouble_isAssignableFrom  avgt5  2,106 ± 0,068  ns/op
>> numberIntegerDouble_isInstance avgt5  1,516 ± 0,046  ns/op
>> integerInteger_isAssignableFromavgt5  1,175 ± 0,029  ns/op
>> integerInteger_isInstance  avgt5  0,886 ± 0,017  ns/op
>> numberDouble_isAssignableFrom  avgt5  1,172 ± 0,007  ns/op
>> numberDouble_isInstanceavgt5  0,891 ± 0,030  ns/op
>> numberInteger_isAssignableFrom avgt5  1,169 ± 0,014  ns/op
>> numberInteger_isInstance   avgt5  0,887 ± 0,016  ns/op
>
> src/jdk.jfr/share/classes/jdk/jfr/internal/TypeLibrary.java line 405:
> 
>> 403: Object res = m.invoke(a, new Object[0]);
>> 404: if (res instanceof Annotation[]) {
>> 405: for (Annotation rep : (Annotation[]) 
>> m.invoke(a, new Object[0])) {
> 
> BTW it looks suspicious to have `m.invoke(a, new Object[0])` called again 
> here. Shouldn't it just reuse `res` variable instead?

It looks unnecessary. Please feel free to fix.

-

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

Re: RFR: 8279918: Fix various doc typos [v2]

[Erik Gahlin]

On Thu, 13 Jan 2022 14:01:04 GMT, Pavel Rappo  wrote:

>> - Most of the typos are of a trivial kind: missing whitespace.
>> - If any of the typos should be fixed in the upstream projects instead, 
>> please say so; I will drop those typos from the patch.
>> - As I understand it, ` ` in ImageInputStream and DataInput is an irrelevant 
>> formatting artefact and thus can be removed.
>> - `` is an apostrophe, which does not require to be encoded.
>
> Pavel Rappo has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Additional typos

Marked as reviewed by egahlin (Reviewer).

The JFR related changes looks OK.

-

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

Re: RFR: 8275445: RunThese30M.java failed "assert(ZAddress::is_marked(addr)) failed: Should be marked" [v2]

[Erik Gahlin]

On Tue, 19 Oct 2021 10:05:15 GMT, Markus Grönlund  wrote:

>> Greetings,
>> 
>> This fixes the issue seen in testing when accessing an oop as part of 
>> unloading (introduced with 
>> [JDK-8266936](https://bugs.openjdk.java.net/browse/JDK-8266936)).
>> 
>> Instead, oop accesses will be done outside of unloading and the result, i.e 
>> the codesource attribute, will be cached and reused in the FinalizerEntry.
>> 
>> Testing: tier1-3, jdk_jfr
>> 
>> Thanks
>> Markus
>> 
>> PS one effect of this is that classes that unload before they have allocated 
>> anything will not have a codesource attribute. This can be fixed by letting 
>> classes register with the table as part of class loading, instead of during 
>> allocation. I will follow-up with a separate change for that.
>
> Markus Grönlund has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   header

Marked as reviewed by egahlin (Reviewer).

-

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

Re: RFR: 8266936: Add a finalization JFR event [v10]

[Erik Gahlin]

On Fri, 27 Aug 2021 15:23:35 GMT, Markus Grönlund  wrote:

>> Greetings,
>> 
>> Object.finalize() was deprecated in JDK9. There is an ongoing effort to 
>> replace and mitigate Object.finalize() uses in the JDK libraries; please see 
>> https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. 
>> 
>> We also like to assist users in replacing and mitigating uses in non-JDK 
>> code.
>> 
>> Hence, this changeset adds a periodic JFR event to help identify which 
>> classes are overriding Object.finalize().
>> 
>> Thanks
>> Markus
>
> Markus Grönlund has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - localize
>  - cleanup
>  - FinalizerStatistics

Marked as reviewed by egahlin (Reviewer).

src/java.base/share/classes/java/lang/ref/Finalizer.java line 71:

> 69: }
> 70: 
> 71: 

extraneous whitespace?

test/jdk/jdk/jfr/event/runtime/TestFinalizerStatisticsEvent.java line 98:

> 96:   case TEST_CLASS_NAME: {
> 97:   
> Asserts.assertTrue(event.getString("codeSource").startsWith("file://"));
> 98:   foundTestClassName = true;

Could we (sanity) check "objects" and "totalFinalzersRun" fields as well?

-

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

Re: RFR: 8275259: Add support for Java level DCmd

[Erik Gahlin]

On Thu, 14 Oct 2021 05:42:09 GMT, Denghui Dong  wrote:

> I proposed to extend DCmd to allow Java developers to customize their own 
> diagnostic commands last week.
> 
> At present, I have implemented a preliminary version.
> 
> In the current implementation, I provided some simple APIs in the Java layer 
> (under sun.management.cmd) for defining and registering commands.
> 
> - Executable interface
>   Java diagnostic commands need to implement this interface, the interface 
> only contains a simple method:
> 
> /**
>  * @param output the output when this executable is running
>  */
> void execute(PrintWriter output);
> 
> 
> - Add two annotations (@Command and @Parameter) to describe the command meta 
> info
> 
> - Use Factory API to register command, the following forms are supported
> 
> @Command(name = "My.Echo", description = "Echo description")
> class Echo implements Executable {
> 
> @Parameter(name = "text", ordinal=0, isMandatory = true)
> String text;
> 
> @Parameter(name = "repeat", isMandatory = true, defaultValue = "1")
> int repeat;
> 
> @Override
> public void execute(PrintWriter out) {
> for (int i = 0 ; i < repeat; i++) {
> out.println(text);
> }
> }
> }
> 
> Factory.register(Echo.class);
> 
> 
> 
> Factory.register("My.Date", output -> {
> output.println(new Date());
> });
> 
> 
> - When the command is running, the VM will call `Executor.executeCommand` to 
> execute the command. In the implementation of Executor, I introduced a simple 
> timeout mechanism to prevent the command channel from being blocked.
> 
> At the VM layer, I extended the existing DCmd framework(implemented JavaDCmd 
> and JavaDCmdFactoryImpl) to be compatible with existing functions (jmx, help, 
> etc.).
> 
> In terms of security, considering that the SecurityManager will be 
> deprecated, there are not too many considerations for the time being.
> 
> Any input is appreciated.
> 
> Thanks,
> Denghui

To understand what is needed (namespacing, error handling, safety, lifecycle 
etc.) and if this is actually useful for Java, I think you need to write 2-3 
commands for a few popular frameworks, for example Spring.

If we can't come up with 5-10 commands where this functionality would solve 
real problems users face, it's unlikely somebody will use it. It will just be a 
maintenance burden in the JDK. Once an API is added, it's hard to remove.

The scope of this seem to be JEP level.

(JFR is not a valid use case, since we don't want to add a dependency on 
annotation classes in the jdk.jcmd module).

-

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

Re: Extend jcmd to java application level

[Erik Gahlin]

To make an application command usable, it must provide metadata (name and 
description of the command, its options’ data types, units, default values, if 
it is mandatory etc.) and error handling. This will make the API surface larger 
and trickier to get right.

(Not a full overlap, but we are thinking of adding more interactive 
capabilities to JFR now that we have event streaming. The design hasn’t 
started, but one could imagine a command to see object allocation events 
aggregated in a histogram. Such a command may be able to operate on 
user-defined events as well, including requestable/periodic events.)

Erik

That seems an ideal solution. I think there are some potential code 
consolidation work further. With this change, some existing C++ JFR Jcmd 
structure definitions(and other Jcmd commands) in VM level can also be lifted 
to Java level because they simply forward request to Java level by 
JavaCalls::call_static.

--
From:Ioi Lam 
Send Time:2021 Oct. 8 (Fri.) 15:22
To:David Holmes ; dong denghui 
; serviceability-dev 
; hotspot-runtime-...@openjdk.java.net 

Subject:Re: Extend jcmd to java application level



On 10/7/21 6:25 PM, David Holmes wrote:
> Hi Denghui,
>
> On 7/10/2021 11:58 pm, Denghui Dong wrote:
>> Hi team,
>>
>> The `jcmd` command can be used to call some built-in diagnostic commands in 
>> vm.
>>
>> Can we consider extending it to the java layer like perf data, so that Java 
>> developers can
>>
>> customize their diagnostic commands and then call them through `jcmd`?
>>
>> One application scenario I can think of for this extension is that some 
>> statistical information
>>
>> may be collected in a java application. Triggering the output of this 
>> statistical information through
>>
>> the `jcmd` command seems to me relative to other mechanisms that trigger 
>> output (such as through
>>
>> an HTTP service, or periodic Printing) is more convenient.
>>
>> Any input is appreciated.
>
> If the intent is that you could issue a jcmd:
>
> jcmd  MyClass.foo
>
> to have it run/use a Java thread to execute arbitrary code then I
> think a lot of careful consideration would need to be given to making
> this facility safe and secure. I can easily imagine that the thread
> used, and the timing, could cause failures. Executing arbitrary code
> may be far too general, so it might mean we need a new "service"
> interface defined that the target class has to implement.
>
> It might well be useful but will need some deep thought IMO.
>

If I understood correctly, the app would need to call an API like:


JcmdProvider.register("mycmd1", new JcmdHandler() {
public void handleCommand(String args[], PrintStream out) {
out.print("my response is ");

  ... }
});

and then the user can issue:

jcmd  mycmd1 args .

which will reach the handleCommand() method provided by the app.

Thanks
- Ioi






> Cheers,
> David
>
>> Thanks,
>> Denghui Dong

Re: Monitoring Java Safepoint Time in JDK16+

[Erik Gahlin]

It's possible to access safepoint information using JFR, for example:

try (RecordingStream r = new RecordingStream()) {
  r.enable("jdk.SafepointBegin");
  r.enable("jdk.SafepointEnd");
  r.onEvent("jdk.SafepointBegin", e -> System.out.println("begin: " + 
e.getEndTime()));
  r.onEvent("jdk.SafepointEnd", e -> System.out.println("end: " + 
e.getEndTime()));
  r.start();
}

Erik

On 2021-06-16, 21:12, "serviceability-dev on behalf of Carter Kozak" 
 wrote:

As java 16 and beyond lock down access to internal components by default, 
it can be difficult to produce Prometheus-style metrics describing application 
safepoints. I’ve been monitoring these metrics so that I can be alerted when an 
application spends more than ~10% of time in safepoints for some duration, 
because it means that something has gone wrong: Most often GC spirals, however 
excessive thread dumps, deadlock checks, etc can contribute. This has been one 
of the most meaningful tools in my arsenal to detect general JVM badness, 
however there doesn’t seem to be a way to access the data in newer JREs without 
allowing access to internal components.

Previously I was able to use something along these lines, which required 
legacy sun.management component access.

sun.management.HotspotRuntimeMBean hotspotRuntimeManagementBean = 
sun.management.ManagementFactoryHelper.getHotspotRuntimeMBean();
long totalSafepointTimeMillis = 
hotspotRuntimeManagementBean.getTotalSafepointTime();

Before I get ahead of myself, I’d like to confirm that I haven’t missed a 
supported pathway to access safepoint time. If my read is correct and there’s 
no way to access this information from inside the running JVM, would it be a a 
reasonable addition to the public API?

Thanks,
Carter Kozak

Integrated: 8265036: JFR: Remove use of -XX:StartFlightRecording= and -XX:FlightRecorderOptions=

[Erik Gahlin]

On Sun, 18 Apr 2021 15:17:35 GMT, Erik Gahlin  wrote:

> Hi,
> 
> Could I have a review of fix that removes the use of "=" together with 
> -XX:StartFlightRecording and -XX:FlightRecorderOptions. It's been possible to 
> use "-XX:StartFlightRecording:" and "-XX:FlightRecorderOption:" since JFR was 
> introduced into OpenJDK (JDK 11), so this is not a change of the 
> specification, just an update to make the use consistent in tests, comments, 
> documentation etc.
> 
> I also removed the use of -XX:+FlightRecorder, which is not needed, and has 
> been deprecated since JDK 13. 
> 
> Testing: jdk/jdk/jfr, tier 1-4.
> 
> Thanks
> Erik

This pull request has now been integrated.

Changeset: 4dcaac1f
Author:Erik Gahlin 
URL:   https://git.openjdk.java.net/jdk/commit/4dcaac1f
Stats: 104 lines in 40 files changed: 0 ins; 0 del; 104 mod

8265036: JFR: Remove use of -XX:StartFlightRecording= and 
-XX:FlightRecorderOptions=

Reviewed-by: cjplummer

-

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

Re: RFR: 8265036: JFR: Remove use of -XX:StartFlightRecording= and -XX:FlightRecorderOptions= [v2]

[Erik Gahlin]

On Mon, 19 Apr 2021 20:16:59 GMT, Chris Plummer  wrote:

> The changes look good. Have you considered doing a test run where the use of 
> `=` and `XX:+FlightRecorder` are disallowed just to make sure you caught them 
> all?

It's a good idea, but it requires changes outside OpenJDK which I rather not do 
now. I'm sure somebody will reintroduce '=', so this is not so much about 
getting rid of them all, but to avoid them being propagated, when code is copy 
pasted for new tests etc.

-

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

Re: RFR: 8265036: JFR: Remove use of -XX:StartFlightRecording= and -XX:FlightRecorderOptions= [v2]

[Erik Gahlin]

> Hi,
> 
> Could I have a review of fix that removes the use of "=" together with 
> -XX:StartFlightRecording and -XX:FlightRecorderOptions. It's been possible to 
> use "-XX:StartFlightRecording:" and "-XX:FlightRecorderOption:" since JFR was 
> introduced into OpenJDK (JDK 11), so this is not a change of the 
> specification, just an update to make the use consistent in tests, comments, 
> documentation etc.
> 
> I also removed the use of -XX:+FlightRecorder, which is not needed, and has 
> been deprecated since JDK 13. 
> 
> Testing: jdk/jdk/jfr, tier 1-4.
> 
> Thanks
> Erik

Erik Gahlin has updated the pull request incrementally with one additional 
commit since the last revision:

  Replace '=' in jfrOptionsSet.cpp

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3561/files
  - new: https://git.openjdk.java.net/jdk/pull/3561/files/4ce08939..138bac16

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3561=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3561=00-01

  Stats: 7 lines in 1 file changed: 0 ins; 0 del; 7 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3561.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3561/head:pull/3561

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

RFR: 8265036: JFR: Remove use of -XX:StartFlightRecording= and -XX:FlightRecorderOptions=

[Erik Gahlin]

Hi,

Could I have a review of fix that removes the use of "=" together with 
-XX:StartFlightRecording and -XX:FlightRecorderOptions. It's been possible to 
use "-XX:StartFlightRecording:" and "-XX:FlightRecorderOption:" since JFR was 
introduced into OpenJDK (JDK 11), so this is not a change of the specification, 
just an update to make the use consistent in tests, comments, documentation etc.

I also removed the use of -XX:+FlightRecorder, which is not needed, and has 
been deprecated since JDK 13. 

Testing: jdk/jdk/jfr, tier 1-4.

Thanks
Erik

-

Commit messages:
 - Initial

Changes: https://git.openjdk.java.net/jdk/pull/3561/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3561=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8265036
  Stats: 97 lines in 39 files changed: 0 ins; 0 del; 97 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3561.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3561/head:pull/3561

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

Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect and corresponsing logic seems to be broken

[Erik Gahlin]

Hi Fairoz,

What I think you need to do is something like this:

    if (className.equals("java.lang.Thread")) {
    return !isJfrInitialized();
    }

...

    private static boolean isJfrInitialized() {
    try {
    Class clazz = Class.forName("jdk.jfr.FlightRecorder");
    Method method = clazz.getDeclaredMethod("isInitialized", 
new Class[0]);

    return (boolean) method.invoke(null, new Object[0]);
    } catch (Exception e) {
    return false;
    }
    }

Erik

On 2020-06-01 12:30, Fairoz Matte wrote:

Hi Erik,

Thanks for your quick response,
Below is the updated webrev to handle if jfr module is not present
http://cr.openjdk.java.net/~fmatte/8243451/webrev.01/

Thanks,
Fairoz


-----Original Message-
From: Erik Gahlin
Sent: Monday, June 1, 2020 2:31 PM
To: Fairoz Matte 
Cc: serviceability-dev@openjdk.java.net
Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect
and corresponsing logic seems to be broken

Hi Fairoz,

If the test needs to run with builds where the JFR module is not present(?), you
need to do the check using reflection.

If not, looks good.

Erik


On 1 Jun 2020, at 10:27, Fairoz Matte  wrote:

Hi,

Please review this small test infra change to identify at runtime the JFR is

active or not.

JBS - https://bugs.openjdk.java.net/browse/JDK-8243451
Webrev - http://cr.openjdk.java.net/~fmatte/8243451/webrev.00/

Thanks,
Fairoz

Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is incorrect and corresponsing logic seems to be broken

[Erik Gahlin]

Hi Fairoz,

If the test needs to run with builds where the JFR module is not present(?), 
you need to do the check using reflection. 

If not, looks good.

Erik

> On 1 Jun 2020, at 10:27, Fairoz Matte  wrote:
> 
> Hi,
> 
> Please review this small test infra change to identify at runtime the JFR is 
> active or not.
> 
> JBS - https://bugs.openjdk.java.net/browse/JDK-8243451
> Webrev - http://cr.openjdk.java.net/~fmatte/8243451/webrev.00/
> 
> Thanks,
> Fairoz

Re: 8233197(S): Invert JvmtiExport::post_vm_initialized() and Jfr:on_vm_start() start-up order for correct option parsing

[Erik Gahlin]

Looks good.

Erik

On 2019-11-20 21:54, Markus Gronlund wrote:


"It does not look as a good idea to change the JVMTI phase like above.

  If you need the ONLOAD phase just to enable capabilities then it is 
better to do it in the real ONLOAD phase.


  Do I miss anything important here?

  Please, ask questions if you have any problems with it."

Yes, so the reason for the phase transition is not so much to do with 
capabilities, but that an agent can only register, i.e. call GetEnv(), 
in phases JVMTI_PHASE_ONLOAD and JVMTI_PHASE_LIVE.


create_vm_init_agents() is where the (temporary) 
JVMTI_PHASE_PRIMORDIAL to JVMTI_PHASE_ONLOAD happens during the 
callouts to Agent_OnLoad(), and then the state is returned to 
JVMTI_PHASE_PRIMORDIAL. It is hard to find an unconditional hook point 
there since create_vm_init_agents() is made conditional on 
Arguments::init_agents_at_startup(), with a listing populated from 
"real agents" (on command-line).


The JFR JVMTI agent itself is also conditional, installed only if JFR 
is actively started (i.e. a starting a recording). Hence, the phase 
transition mechanism merely replicates the state changes in 
create_vm_init_agents() to have the agent register properly. This is a 
moot point now however as I have taken another pass. I now found a way 
to only have the agent register during the JVMTI_PHASE_LIVE phase, so 
the phase transition mechanism is not needed.


"The Jfr::on_vm_init() is confusing as there is a mismatch with the 
JVMTI phases order.


  It fills like it means JFR init event (not VM init) or something 
like this.


  Or maybe it denotes the VM initialization start. :)

  I'll be happy if you could explain it a little bit."

Yes, this is confusing, I agree. Of course, JFR has a tight relation 
to the JVMTI phases, but only in so far as to coordinate agent 
registration. The JFR calls are not intended to reflect the JVMTI 
phases per se but a more general initialization order state 
description, like you say "VM initialization start and completion". 
However, it is very hard to encode proper semantics into the JFR calls 
in Threads::create_vm() to reflect the concepts of "stages"; they are 
simply not well-defined. In addition, there are so many of them J. For 
example, I always get confused that VM initialization is reflected in 
JVMTI by the VMStart event and the completion by the VMInit event 
(representing VM initialization complete). At the same time, the 
DTRACE macros have both HOTSPOT_VM_INIT_BEGIN() HOTSPOT_VM_INIT_END() 
placed before both...


I abandoned the attempt to encode anything meaningful into the JFR 
calls trying to represent a certain "VM initialization stage".


Instead, I will just have syntactic JFR calls reflecting some relative 
order (on_create_vm_1(), on_create_vm_2(),.. _3()) etc. Looks like 
there are precedents of this style.


“Not sure, if your agent needs to enable these capabilities 
(introduced in JDK 9 with modules):

  can_generate_early_vmstart
  can_generate_early_class_hook_events”

Thanks for the suggestion Serguei, but these capabilities are not yet 
needed.


Here is the updated webrev: 
http://cr.openjdk.java.net/~mgronlun/8233197/webrev02/


Thanks again

Markus

*From:*Serguei Spitsyn
*Sent:* den 20 november 2019 04:10
*To:* Markus Gronlund ; hotspot-jfr-dev 
; 
hotspot-runtime-...@openjdk.java.net; serviceability-dev@openjdk.java.net
*Subject:* Re: 8233197(S): Invert JvmtiExport::post_vm_initialized() 
and Jfr:on_vm_start() start-up order for correct option parsing


Hi Marcus,

It looks good in general.

A couple of comments though.

http://cr.openjdk.java.net/~mgronlun/8233197/webrev01/src/hotspot/share/jfr/instrumentation/jfrJvmtiAgent.cpp.frames.html

258 class JvmtiPhaseTransition {
259  private:
260   bool _transition;
261  public:
262   JvmtiPhaseTransition() : _transition(JvmtiEnvBase::get_phase() 
== JVMTI_PHASE_PRIMORDIAL) {

263 if (_transition) {
264   JvmtiEnvBase::set_phase(JVMTI_PHASE_ONLOAD);
265 }
266   }
267   ~JvmtiPhaseTransition() {
268 if (_transition) {
269   assert(JvmtiEnvBase::get_phase() == JVMTI_PHASE_ONLOAD, 
"invariant");

270   JvmtiEnvBase::set_phase(JVMTI_PHASE_PRIMORDIAL);
271 }
272   }
273 };
274
  275 static bool initialize() {
  276   JavaThread* const jt = current_java_thread();
  277   assert(jt != NULL, "invariant");
  278   assert(jt->thread_state() == _thread_in_vm, "invariant");
  279   DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(jt));
*280   JvmtiPhaseTransition jvmti_phase_transition;*
  281   ThreadToNativeFromVM transition(jt);
  282   if (create_jvmti_env(jt) != JNI_OK) {
  283 assert(jfr_jvmti_env == NULL, "invariant");
  284 return false;
  285   }
  286   assert(jfr_jvmti_env != NULL, "invariant");
287   if (!register_capabilities(jt)) {
  288 return false;
  289   }
290   if (!register_callbacks(jt)) {
  291 return false;
  292   }
293   return update_class_file_load_hook_event(JVMTI_ENABLE);
  294 }

Re: DumpJFR tool

[Erik Gahlin]

On 2019-10-24 16:52, Yasumasa Suenaga wrote:

On 2019/10/24 22:50, Erik Gahlin wrote:

Hi Yasumasa,

The DumpJFR tool relied on SA, which was too much of a burden to 
maintain (slowing down the development of new JFR features).


I think tool was removed in Oracle JDK 9, at least it was completely 
broken at that time.


Functionality has been added that allows JFR to dump an emergency 
dump if the JVM crashes. This reduces the need for a separate tool.


SA is powerful tool for hung up and for postmortem analysis.
For example, SA is only way to gather any information even if Attach 
Listener does not respond.


There are always trade-offs, and with the crash dump support in place, 
we came to the conclusion that the cost of maintaining the tool and SA 
support outweighed the benefits.



JFR emergency dump is not enough.
I commented in JDK-8213435, crash which is caused by 
-XX:+CrashOnOutOfMemoryError does not contain

recently old object sampling.
Duplicating logic in the SA tool to calculate shortest path GC, 
information about root objects etc. would be a much larger effort to 
implement and maintain than fixing JDK-8213435.


In addition, there are some cases not to be called crash handler (e.g. 
native stack overflow).



The Event Streaming JEP, targeted for JDK 14, should also help out 
here, as it ensures the latest file in the disk repository, 
previously known as the .part file, can always read. With a default 
flush interval of 1 second, data will be available very close to the 
point of the crash.


Is flight record flushed to the disk without any programming for Event 
Streaming?

I guess it is just about Event Stream API.

I hope to get flight record with some configuration - not any 
programming.


Streaming will be always on. No programming or configuration will be needed.

You can just open the latest file in JMC.

Best regards,
Erik




Thanks,

Yasumasa



Best regards,
Erik


Hi all,

DumpJFR tool is introduced in JDK 8u60 [1], however it is not 
available on OpenJDK (JDK 11 or later).

Why it is not open sourced?

I think DumpJFR is very useful not only for postmortem analysis but 
also analyzing process hung up.
If DumpJFR is available on OpenJDK, I believe JFR will be more 
useful for troubleshooting.



Thanks,

Yasumasa


[1] 
https://www.oracle.com/technetwork/java/javase/8u60-relnotes-2620227.html

Re: DumpJFR tool

[Erik Gahlin]

Hi Yasumasa,

The DumpJFR tool relied on SA, which was too much of a burden to 
maintain (slowing down the development of new JFR features).


I think tool was removed in Oracle JDK 9, at least it was completely 
broken at that time.


Functionality has been added that allows JFR to dump an emergency dump 
if the JVM crashes. This reduces the need for a separate tool.


The Event Streaming JEP, targeted for JDK 14, should also help out here, 
as it ensures the latest file in the disk repository, previously known 
as the .part file, can always read. With a default flush interval of 1 
second, data will be available very close to the point of the crash.


Best regards,
Erik


Hi all,

DumpJFR tool is introduced in JDK 8u60 [1], however it is not 
available on OpenJDK (JDK 11 or later).

Why it is not open sourced?

I think DumpJFR is very useful not only for postmortem analysis but 
also analyzing process hung up.
If DumpJFR is available on OpenJDK, I believe JFR will be more useful 
for troubleshooting.



Thanks,

Yasumasa


[1] 
https://www.oracle.com/technetwork/java/javase/8u60-relnotes-2620227.html

Re: Protocol version of Attach API

[Erik Gahlin]

On 2019-02-26 07:47, Thomas Stüfe wrote:

Hi David, Yasumasa,



> Do we support connection to later VMs from earlier JDK tools?

I could not find the spec about this.
So I asked to serviceability folks before filing this to JBS :-)


Just to chime in on that, I do not know if it is specified but it is 
certainly very handy in daily use. I often use old jcmd tools to 
connect to newer VMs. I always thought that was a neat design.


I agree.

The tool was designed to be dumb, so it can connect to JVMs regardless 
of release. If something has changed so that is no longer true, it 
should be fixed (and backported if needed).


Erik



..Thomas

Re: [RFR]: Per thread IO statistics in JFR

[Erik Gahlin]

Hi Gunter, 

Sounds like a useful enhancement!

Do you have an idea of what the overhead might be? 

Does it work on Mac OS X?

I haven't done a proper review, but you could change the contentType to 
"bits-per-second”. That is what we use for the Network Utilization event which 
reports statistics per interface.

Thanks
Erik

> Hi All,
> 
> Could I please have a review and possibly some opinions on the following 
> enhancement to JFR and the JDK? 
> 
> At SAP we have a per thread IO statistic among our supportability 
> enhancements which proved to be very helpful for our support engineers. It 
> might be beneficial for JFR as well and would certainly help us to drive 
> adoption of OpenJDK.
> 
> The basic idea is simple, we have added fields to the thread class where the 
> number of bytes read and written from/to file and network are counted in. The 
> newly created JFR events are written periodically as for example the 
> ThreadAllocationStatistics event already is.
> 
> In order to collect the data, we have added hooks to the JDK C coding calling 
> back into the VM.
> 
> I have opened a bug here:
> https://bugs.openjdk.java.net/browse/JDK-8216981 
> 
> 
> Here is a webrev:
> http://cr.openjdk.java.net/~ghaug/webrevs/8216981 
> 
> 
> There are no tests yet and the code be a bit nicer in places. We will work on 
> this if/when this feature is deemed acceptable.
> 
> Finally, we have an API in our SAP version of the JDK to access this data 
> from a Java application. This is used by many SAP applications and I think we 
> could add an MXBean in a second step, to provide similar functionality.
> 
> Thanks in advance,
> Gunter
> 
> 
> 
> 

Re: 8215534: [testbug] some jfr test don't check @requires vm.hasJFR

[Erik Gahlin]

Looks good.

Erik


Hi,

These tests lack @requires vm.hasJFR, thus they are failing on AIX.
http://cr.openjdk.java.net/~goetz/wr18/8215334-JFR_requires/01/

Please review.
I will push this to jdk12 as it is a testbug if I miss  the RDP deadline.

Best regards,
   Goetz.

Re: JDK-8171311 Current state

[Erik Gahlin]

The reason to put this into the JDK is to standardize the protocol.

If you want to build a client today, you must build one for every 
adapter because they have different ways to represent URLs etc.


The JDK is in unique position to set the standard, since the 
implementation comes by default.


Erik


Thanks,

I just found that JEP searching for an simple way to attach to a non 
application server VM avoiding the hassle for setting up Firewall 
Rules for RMI and that JEP was the first in the list followed by the 
Jolokia that seems not jet ready for Java 11...


I will look into the Jolokia library and will try to find out, what 
the exact issues with Java 11 are.


Besides that it would really make sense to see if there would be a 
better way for starting the JMX services as Alan pointed out.


-Patrick




On 2018-12-07 10:59, Raghavan Puranam wrote:

My apologies Patrick...I should have added the comment first before
closing.  I have added it now.

Regards,
Raga

-Original Message-
From: Alan Bateman
Sent: Friday, December 7, 2018 2:52 PM
To: Patrick Reinhart
Cc: serviceability-dev@openjdk.java.net
Subject: Re: JDK-8171311 Current state

On 07/12/2018 08:36, Patrick Reinhart wrote:

It's a bit disturbing that just at the time of my question this JEP
has been closed (without any further comment why)

I suspect your inquiry prompted Raghavan to close it as there isn't (to
my knowledge anyway) anyone actively working on it. I agree a comment is
needed when closing issues.



I think that it still would be worth while looking into supporting
a REST based implementation in favour of the existing RMI based
solution just by the fact of the troubles just one can have with
firewalls.

Right, and I think there is some interest. In addition to the REST
adapters that you found then I think some of the app servers have
support too. The big question for features like this is whether it is
something that the JDK  has to include or not (the batteries included
vs. batteries available discussion).  If you look at Harsha's prototype
(linked from the JEP) then you'll you see it can be mostly developed in
its own project, the only JDK piece is integrating it with the JMX agent
and existing -Dcom.sun.management options for starting the JMX agent. I
think this is an area that could be improved to make it easier to deploy
JMX adapters that aren't in the JDK.

-Alan

Re: jcmd - executing multiple commands at the same safepoint?

[Erik Gahlin]

Not sure if this will have an impact, but we have plans to add 
parameterization to JFR.start.


It will work like the recording wizard in JMC. All the the widgets you 
see in the dialog will be available from command line. For instance, you 
will be able to do something like this (syntax and naming have not been 
decided)


jcmd  JFR.start gc-level=detailed exceptions=true 
method-sampling=10ms classloading=true


The parameters are not fixed at compile time, but instead depends on the 
.jfc file in use. This will simplify usage of JFR as you will no longer 
need to export a .jfc file from JMC if you want to configure events. The 
parameterization will also be available from -XX:StartFlightRecording, 
which uses a comma-separated list.


Furthermore, JFR also supports custom settings, which can have arbitrary 
syntax and semantics. For instance, it will be possible to add a setting 
that will filter out events for a particular thread and have it exposed 
to jcmd as a parameter, i.e.


jcmd  JFR.start threads={Thread1,Thread2, .*Pool.*}

The diagnostic commands for JFR are implemented in Java, so they will 
not be able to execute during a safepoint, so I don't expect there to be 
an issue, but I thought I should mention it.


Erik


Hi Thomas,

very positive suggestion.

One observation:
There already seems to be some different interpretation of the command 
line in different Java versions:
For instance when I try to dump a flight recording in 1.8.0_152-ea-b05 
: I can use:

jcmd 33014 JFR.dump filename="recording1.jfr" recording=1
but in build 9+181 , the same command must be:
jcmd 33014 JFR.dump filename="recording1.jfr",recording=1
(the comma to separate sub-options you mentioned earlier)

Therefore the suggestion without curly brackets, giving several 
commands after one another seems good with regard to backwards 
compatibility.


Motivation: hawt.io  uses the 
MBean com.sun.management:type=DiagnosticCommand to access the same 
functionality as jcmd. Variations in option syntax across Java 
versions is already an issue (only affecting sub-options though, as 
each command is a separate JMX operation). So syntax compatibility is 
highly appreciated :-)


Martin

lør. 12. mai 2018 kl. 20:11 skrev Kirk Pepperdine 
>:



> On May 10, 2018, at 11:26 AM, Thomas Stüfe
> wrote:
>
> On Thu, May 10, 2018 at 9:13 AM, Kirk Pepperdine
> >
wrote:
>> The stacking at the safe point would be a huge win. Right now
thread dump consistency can really confuse people as the tooling
will show two threads owning the same lock at seemingly the same
time. Reality, it’s just that people didn’t realize you get a safe
point for each thread therefor there is motion in the system as
you’re collecting the data.
>
> I am a bit confused. What tooling are you talking about?

jstack. I’ve not seen it with jcmd. But I often see 2 threads
holding the same lock at the “same time” which is of course
non-sense. I can dig one up for you if you’re still confused.


>
>>
>> As an aside, it’s amazing how many dev’s aren’t aware of jcmd.
Just yesterday after my session at Devoxx I had someone ask me
about using jfr from the command line, many in that session had
not seen jcmd before. The feedback was, wow, this is very useful
and I wished I had of known about it earlier.
>
> Yes, jcmd is quite useful. I also really like the simple design,
which
> is by default up- and downward compatible (so you can talk to
any VM,
> new, old, it should not matter). That is just good design. We -
Sap -
> work to extend jcmd, to help our support folks. E.g. the whole new
> VM.metaspace command chain.

And simple it is….well done!!!

— Kirk

Re: RFR : JDK-8196744 : JMX: Not enough JDP packets received before timeout

[Erik Gahlin]

Looks OK.

Erik


Hi All,

Please find the fix below for the Jdp test-case.

issue: https://bugs.openjdk.java.net/browse/JDK-8196028
webrev : http://cr.openjdk.java.net/~hb/8196028/webrev.00/

Fix details : The test was receiving JDP packets from other VM and 
hence the multi-cast socket was not timing-out. The default timeout 
handler was causing test to fail. Added a shutdown method that passes 
the test in case of timeout.


Thanks
Harsha

Re: RFR: 8041626: [Event Request] Shutdown reason

[Erik Gahlin]

Hi Robin,

I can sponsor this.

I wonder if we should change the name of the event to "Shutdown" 
instead? It will give us flexibility to add other shutdown related 
fields in the future.


Could you change the label and field to "Status Code" and statusCode.  
Event fields should have headline-style capitalization and statusCode 
allows us to add other status related information in the future.


Thanks
Erik


Hi all,

Please review the following change that adds an event-based tracing 
event that is generated when the VM shuts down. The intent is to 
capture shutdowns that occur after the VM has been properly 
initialized (as initialization problems would most likely mean that 
the tracing framework hasn’t been properly started either).


Issue: https://bugs.openjdk.java.net/browse/JDK-8041626
Webrev: http://cr.openjdk.java.net/~rwestberg/8041626/webrev.00/ 


Testing: hs-tier1,hs-tier2,jdk-tier1,jdk-tier2

Best regards,
Robin

Re: RFR: JDK-8187498: Add a -Xmanagement flag as syntactic sugar for -Dcom.sun.management.jmxremote.* properties

[Erik Gahlin]

Hi Harsha,

Very nice to see progress on this!

Before reviewing, the minimal command line to start up the default 
management server now becomes


-Xmanagement:ssl=false,authenticate=false

and if you use a property that doesn't exist, or that is mandatory, you 
will get an error message stating what is wrong?


Could we reduce the command line further, so only a single property is 
needed:


-Xmanagement:secure=false

or perhaps:

-Xmanagement:unsecure

which would set ssl=false,authenticate=false, because that is what you 
want 99% of the time.


Thanks
Erik


Hi,

Please review the changes for above enhancement having webrev at,

http://cr.openjdk.java.net/~hb/8187498/webrev.00/

Thanks
Harsha

Re: Question about DiagnosticCommand.jfrCheck

[Erik Gahlin]

Hi Martin,

There will be a JEP outlining how things will work in OpenJDK.

When it comes to Oracle JDK, it is too early to say and should not be 
discussed on this mailing list, but feel free to mail me when there is a 
JEP.


Cheers!
Erik


Hi

I have a question about plans for 
com.sun.management.DiagnosticCommand.jfrCheck :


If I run this command in JRE 9+181 i get this output:

"Java Flight Recorder not enabled.

Use VM.unlock_commercial_features to enable."

Will the output change in future versions when the flight recorder 
goes open source?

Will it work differently in Oracle and OpenJDK JRE ?

My background for asking this question: I do some work on flight 
recorder support in hawt.io  and would like to give 
the user proper warning before attempting to unlock commercial 
features at runtime.


Cheers!

Martin Skarsaune

RFR (XS): 8179083: Uninitialized notifier in Java Monitor Wait tracing event

[Erik Gahlin]

Hi,

Could I have fix of this small change.

Webrev:
http://cr.openjdk.java.net/~egahlin/8179083/

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

Thanks
Erik

Re: RFR(S): 8189368: Add information on current bias holder for BiasedLockRevocation event

[Erik Gahlin]

Looks good.

Erik

Hi all,

Please review the following changes which add a new field to the 
BiasedLockRevocation event, previousOwner, containing information on which 
thread (if any) owned the bias before it was revoked.

Issue:
https://bugs.openjdk.java.net/browse/JDK-8189368

Webrev:
http://cr.openjdk.java.net/~egahlin/8189368_1/

Best regards,
Robin

Re: RFR(S): 8189425: Minor updates in support of closed changes

[Erik Gahlin]

Thanks for the review,  David and Markus!

Erik

Hi Erik,

Looks good.

Thanks
Markus

-Original Message-
From: Erik Gahlin
Sent: den 18 oktober 2017 22:05
To: David Holmes; serviceability-dev@openjdk.java.net
Subject: Re: RFR(S): 8189425: Minor updates in support of closed changes

Hi David,


Hi Erik,

On 18/10/2017 12:23 PM, Erik Gahlin wrote:

Hi,

Could I have a review of this change that will adjust an assertion
and

Can you explain the adjustment please.

We have closed code that modifies the mark word and then changes it back during 
a safepoint. When the mark word is modified, we reuse GC infrastructure that 
run into the assert. If we change the assert to ignore checking that the mark 
word is NULL, we don't run into the problem.


remove a lock associated with JFR.

I forgot to modify the header file, see updated webrev.

http://cr.openjdk.java.net/~egahlin/8189425_1/

I  also made a change to GrowableArray, the insert_sorted method now takes a 
const.

Thanks
Erik


That bit is fine :)

Thanks,
David


Webrev:
http://cr.openjdk.java.net/~egahlin/8189425_0

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

Thanks
Erik

Re: RFR(S): 8189425: Minor updates in support of closed changes

[Erik Gahlin]

Hi David,


Hi Erik,

On 18/10/2017 12:23 PM, Erik Gahlin wrote:

Hi,

Could I have a review of this change that will adjust an assertion and 


Can you explain the adjustment please.
We have closed code that modifies the mark word and then changes it back 
during a safepoint. When the mark word is modified, we reuse GC 
infrastructure that run into the assert. If we change the assert to 
ignore checking that the mark word is NULL, we don't run into the problem.





remove a lock associated with JFR.




I forgot to modify the header file, see updated webrev.

http://cr.openjdk.java.net/~egahlin/8189425_1/

I  also made a change to GrowableArray, the insert_sorted method now 
takes a const.


Thanks
Erik


That bit is fine :)

Thanks,
David


Webrev:
http://cr.openjdk.java.net/~egahlin/8189425_0

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

Thanks
Erik

Re: RFR: 8189440: Event tracing macros for allocation and weak oops processing

[Erik Gahlin]

Hi David,


Hi Erik,

On 18/10/2017 12:34 PM, Erik Gahlin wrote:

Hi,

Could I have a review of a change that adds two macros to be used 
with event-based JVM tracing.


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

Webrev:
http://cr.openjdk.java.net/~egahlin/8189440_0


Reviewed - though all somewhat mysterious in isolation :(


Thanks for the review!

Sorry about the mysterious part.


My only real query is in jniHandles.cpp:

   JvmtiExport::weak_oops_do(is_alive, f);
+  TRACE_WEAK_OOPS_DO(is_alive, f);

Can't/shouldn't the tracing be done inside weak_oops_do?

Stefan Karlsson has a change out for review and if integrated before 
this one, I will move the TRACE_WEAK_OOPS_DO into weak_oops_do in 
weakProcessor.cpp.


Thanks
Erik

RFR: 8189440: Event tracing macros for allocation and weak oops processing

[Erik Gahlin]

Hi,

Could I have a review of a change that adds two macros to be used with 
event-based JVM tracing.


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

Webrev:
http://cr.openjdk.java.net/~egahlin/8189440_0

Thanks
Erik

RFR(S): 8189425: Minor updates in support of closed changes

[Erik Gahlin]

Hi,

Could I have a review of this change that will adjust an assertion and 
remove a lock associated with JFR.


Webrev:
http://cr.openjdk.java.net/~egahlin/8189425_0

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

Thanks
Erik

Re: RFR(XS): 8173917: Safepoint ID is not consistent across event-based tracing events

[Erik Gahlin]

Look good.

Should I push this change before the new events?

Thanks
Erik

> On 13 Oct 2017, at 16:54, Robin Westberg  wrote:
> 
> Hi all,
> 
> Please review the following change that aligns the safepointId value for the 
> SafepointStateSynchronization event to be consistent with the other related 
> events committed later.
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8173917 
> 
> 
> Webrev:
> http://cr.openjdk.java.net/~egahlin/8173917_0/ 
> 
> 
> Best regards,
> Robin

RFR(XS): 8189274: Allow cutoff attribute for event based tracing

[Erik Gahlin]

Hi,

Could I have a review of this small change that will add the capability 
to use a cutoff attribute on an event.


Webrev:
http://cr.openjdk.java.net/~egahlin/8189274/

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

Thanks
Erik

Re: RFR: 8187042: Events to show which objects are associated with biased object revocations

[Erik Gahlin]

Looks good!

Erik


> On 6 Oct 2017, at 12:22, Robin Westberg <robin.westb...@oracle.com> wrote:
> 
> Hi all,
> 
> Please review this change to add event-based tracing events for biased lock 
> revocations:
> 
> Issue: https://bugs.openjdk.java.net/browse/JDK-8187042 
> <https://bugs.openjdk.java.net/browse/JDK-8187042>
> Webrev (courtesy of Erik Gahlin): 
> http://cr.openjdk.java.net/~egahlin/8187042/ 
> <http://cr.openjdk.java.net/~egahlin/8187042/>
> 
> Best regards,
> Robin

Re: jmx-dev JEP review : JDK-8171311 - REST APIs for JMX

[Erik Gahlin]

hana B 
<harsha.wardhan...@oracle.com 
<mailto:harsha.wardhan...@oracle.com>> wrote:


Hi Erik,


On Monday 11 September 2017 07:14 PM, Erik Gahlin wrote:

Hi Harsha,

I haven't looked at Jolokia, or know what is the most reasonable 
approach in this case, but as a principle, I think we should 
strive for the best possible design rather than trying to be 
compatible with third party tools.
Agreed. That will always be the first priority. That is the reason 
HTTP GET interfaces will not be changed. I am undecided if the 
POST payloads need to be changed (without compromising the REST 
design principles) to increase adoption of this feature.


How will the command line look like to start the agent with the 
rest adapter?


In the past there have been discussions about adding syntactic 
sugar for -Dcom.sun.management, i.e.


-Xmanagement:ssl=false,port=7091,authenticate=false

instead of

-Dcom.sun.management.jmxremote.ssl=false
-Dcom.sun.management.jmxremote.port=7091
-Dcom.sun.management.jmxremote.authenticate=false

which is hard to remember, cumbersome to write and error prone 
since the parameters are not validated. If we are adding support 
for REST, it could perhaps be default, i.e.


-Xmanagement:ssl=false,authenticate=false,port=80

If you want to use JMX over RMI you would specify protocol:

-Xmanagement:ssl=false,port=7091,authenticate=false,protocol=rmi
Yes. There is an enhancement request to add the -Xmanagemet:* 
syntatic sugar for -Dcom.sun.management.jmxremote.* flags. REST 
adapter will use one of the above flags though I haven't thought 
of the exact name for it yet. I will update the JEP with the 
details of the flag shortly.


Has there been any thoughts about JMX notifications?

Notifications will not be supported in this JEP.

  * MBean Notifications are not a widely used feature and will not
be supported via the REST adapter.



I know it is outside the scope of the JEP, but I think we should 
take it into consideration when doing the design, so the 
functionality could be added on later without too much difficulty.
Notifications can be added without modifying the current design 
too much. If required, it will be worked upon via an enhancement 
request.


Thanks
Erik


Thanks
Harsha


Hi Martin,

In my opinion, the interfaces exposed by current JEP are lot 
closer to REST style than the interfaces exposed by Jolokia.


For instance, HTTP GET by default should be used to read 
resources, but it is made part of URL in Jolokia interfaces.


/read///

I would wait on opinions from more people before considering 
changing the current interfaces.


Thanks
-Harsha

On Wednesday 06 September 2017 11:40 AM, Martin Skarsaune wrote:

Hello

Would one at least consider adopting the same URL paths and 
payloads as Jolokia? This could make life a lot easier for 
third party tools that connect to it.


Best Regards

Martin Skarsaune

ons. 6. sep. 2017 kl. 07:04 skrev Harsha Wardhana B 
<harsha.wardhan...@oracle.com 
<mailto:harsha.wardhan...@oracle.com>>:


Hi Kirk,

Yes. Jolokia was considered and is listed as an alternative
in the JEP.

  * Jolokia can serve as a viable alternative but can be
bulky. We are looking for simple and lightweight solution.


-Harsha

On Wednesday 06 September 2017 10:21 AM, Kirk Pepperdine wrote:

Hi,

Have you run into this project?https://jolokia.org <https://jolokia.org/>. 
Unfortunately it’s not exactly a drop in replacement for the standard RMI based JMX 
connector but it’s not far off.

Kind regards,
Kirk


On Sep 5, 2017, at 6:30 PM, Erik Gahlin<erik.gah...@oracle.com> 
<mailto:erik.gah...@oracle.com>  wrote:

Hi Harsha,

Looping in jmx-dev.


byte[], short[], int[], float[], double[]

Should long[] be included there as well?


The REST adapter will come with a simple and lightweight JSON parser.

Is this an internal parser or will it be exposed as an API?

If so, how does it relate to JEP 198: Light-Weight JSON API?
http://openjdk.java.net/jeps/198

Will com.sun.net.httpserver.HttpServer be used to serve the requests?

Thanks
Erik


Hi All,

Please review the JEP for REST APIs for JMX :
https://bugs.openjdk.java.net/browse/JDK-8171311

The JEP aims at providing RESTful web interfaces to MBeans.

Access to MBeans registered in a MBeanServer running inside a JVM requires 
a Java client. Language-agnostic access to MBeans will require spawning a Java 
client which can be cumbersome. The proposed JEP allows MBeans to be accessed 
in a language/platform-independent, ubiquitous and seamless manner.

Thanks
-Harsha

Re: jmx-dev JEP review : JDK-8171311 - REST APIs for JMX

[Erik Gahlin]

Hi Harsha,

I haven't looked at Jolokia, or know what is the most reasonable 
approach in this case, but as a principle, I think we should strive for 
the best possible design rather than trying to be compatible with third 
party tools.


How will the command line look like to start the agent with the rest 
adapter?


In the past there have been discussions about adding syntactic sugar for 
-Dcom.sun.management, i.e.


-Xmanagement:ssl=false,port=7091,authenticate=false

instead of

-Dcom.sun.management.jmxremote.ssl=false
-Dcom.sun.management.jmxremote.port=7091
-Dcom.sun.management.jmxremote.authenticate=false

which is hard to remember, cumbersome to write and error prone since the 
parameters are not validated. If we are adding support for REST, it 
could perhaps be default, i.e.


-Xmanagement:ssl=false,authenticate=false,port=80

If you want to use JMX over RMI you would specify protocol:

-Xmanagement:ssl=false,port=7091,authenticate=false,protocol=rmi

Has there been any thoughts about JMX notifications?

I know it is outside the scope of the JEP, but I think we should take it 
into consideration when doing the design, so the functionality could be 
added on later without too much difficulty.


Thanks
Erik


Hi Martin,

In my opinion, the interfaces exposed by current JEP are lot closer to 
REST style than the interfaces exposed by Jolokia.


For instance, HTTP GET by default should be used to read resources, 
but it is made part of URL in Jolokia interfaces.


/read///

I would wait on opinions from more people before considering changing 
the current interfaces.


Thanks
-Harsha

On Wednesday 06 September 2017 11:40 AM, Martin Skarsaune wrote:

Hello

Would one at least consider adopting the same URL paths and payloads 
as Jolokia? This could make life a lot easier for third party tools 
that connect to it.


Best Regards

Martin Skarsaune

ons. 6. sep. 2017 kl. 07:04 skrev Harsha Wardhana B 
<harsha.wardhan...@oracle.com <mailto:harsha.wardhan...@oracle.com>>:


Hi Kirk,

Yes. Jolokia was considered and is listed as an alternative in
the JEP.

  * Jolokia can serve as a viable alternative but can be bulky.
We are looking for simple and lightweight solution.


-Harsha

On Wednesday 06 September 2017 10:21 AM, Kirk Pepperdine wrote:

Hi,

Have you run into this project?https://jolokia.org. Unfortunately it’s not 
exactly a drop in replacement for the standard RMI based JMX connector but it’s 
not far off.

Kind regards,
Kirk


On Sep 5, 2017, at 6:30 PM, Erik Gahlin<erik.gah...@oracle.com> 
<mailto:erik.gah...@oracle.com>  wrote:

Hi Harsha,

Looping in jmx-dev.


byte[], short[], int[], float[], double[]

Should long[] be included there as well?


The REST adapter will come with a simple and lightweight JSON parser.

Is this an internal parser or will it be exposed as an API?

If so, how does it relate to JEP 198: Light-Weight JSON API?
http://openjdk.java.net/jeps/198

Will com.sun.net.httpserver.HttpServer be used to serve the requests?

Thanks
Erik


Hi All,

Please review the JEP for REST APIs for JMX :
https://bugs.openjdk.java.net/browse/JDK-8171311

The JEP aims at providing RESTful web interfaces to MBeans.

Access to MBeans registered in a MBeanServer running inside a JVM requires 
a Java client. Language-agnostic access to MBeans will require spawning a Java 
client which can be cumbersome. The proposed JEP allows MBeans to be accessed 
in a language/platform-independent, ubiquitous and seamless manner.

Thanks
-Harsha

Re: JEP review : JDK-8171311 - REST APIs for JMX

[Erik Gahlin]

Hi Harsha,

Looping in jmx-dev.

> byte[], short[], int[], float[], double[]

Should long[] be included there as well?

> The REST adapter will come with a simple and lightweight JSON parser.

Is this an internal parser or will it be exposed as an API?

If so, how does it relate to JEP 198: Light-Weight JSON API?
http://openjdk.java.net/jeps/198

Will com.sun.net.httpserver.HttpServer be used to serve the requests?

Thanks
Erik


Hi All,

Please review the JEP for REST APIs for JMX :
https://bugs.openjdk.java.net/browse/JDK-8171311

The JEP aims at providing RESTful web interfaces to MBeans.

Access to MBeans registered in a MBeanServer running inside a JVM 
requires a Java client. Language-agnostic access to MBeans will 
require spawning a Java client which can be cumbersome. The proposed 
JEP allows MBeans to be accessed in a language/platform-independent, 
ubiquitous and seamless manner.


Thanks
-Harsha

Re: RFR: JDK-8185003 JMX: Add a version of ThreadMXBean.dumpAllThreads with a maxDepth argument

[Erik Gahlin]

Looks good, not a reviewer.

Nit, saw that spaces before commas were missing in some method calls, 
i.e. ",-1" instead of ", -1"


Erik


Hi,

kindly review the changes made.

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

webrev: 
http://cr.openjdk.java.net/~uvangapally/webrev/2017/8185003/webrev.00/


CSR: https://bugs.openjdk.java.net/browse/JDK-8185705

Thanks,

Ujwal.

Re: [PATCH] attach in linux should be relative to /proc/pid/root and namespace aware

[Erik Gahlin]

I noticed thatgetNamespacePid throws IOException and 
InvalidPathException. Perhaps we want to catch those, so we can default 
to the original pid if there is a I/O related problem.


Erik

Hey,

I’ve attached a version rebased on jdk10, it also (currently) applies cleanly 
to jdk9.

While there is no supplied test or harness for this patch, how I built and 
tested is
available at https://github.com/tjfontaine/jdkbuild (there’s also a preview of 
my
follow on patch for pathmap_open as well).

Thanks!

TJ

On 4/28/17, 3:47 PM, "serviceability-dev on behalf of TJ Fontaine" 
 wrote:

 I had no doubt we’d end up on the conversation of 10 -> 9 -> 8u, I started 
with 8u merely because it was representative of today’s customer pain. I’ll be sure 
to work on retargeting it as well.
 
 Thanks!
 
 TJ
 
 On 4/28/17, 3:42 PM, "David Holmes"  wrote:
 
 Hi TJ,
 
 Thanks for the patch (I haven't looked at it yet). FYI at the moment,

 unless this is considered a high priority bug for JDK 9 it has to be
 targeted to JDK 10, and then possibly backported to 9 and 8u.
 
 Cheers,

 David
 
 On 29/04/2017 8:23 AM, TJ Fontaine wrote:

 > I have attached a patch that allows jcmd to work against a java 
process running
 > inside a Docker container. Apologies if this is not in the correct 
format. It was
 > built against jdk8u. I couldn’t seem to find an existing JIRA for it.
 >
 > Diagnostic commands (i.e. jcmd, jstack, etc) fail to attach to a 
target JVM
 > that is inside a container (e.g. Docker).
 >
 > A Linux container often isolates a process in a PID and Mount 
namespace that is
 > separate from the "root container" (analogous to the hypervisor/dom0 
in
 > hardware virtualization environments, or the global zone on 
Solaris). A target
 > JVM that is isolated in either a PID namespace, or a Mount namespace 
will fail
 > the attach sequence.
 >
 > When the target JVM is in its own PID namespace the pid of the 
process is
 > distinct from what the real pid of the process as it relates to the 
root
 > container. For example, in the root container you can observe a JVM 
with a pid
 > of 17734, however if that JVM is running inside a Docker container 
the pid
 > inside its PID namespace is likely 1. So when the target JVM 
receives the
 > SIGQUIT it looks in /proc/self/cwd/ for .attach_pid1 however the 
external
 > attaching JVM has created the file /proc/17734/cwd/.attach_pid17734. 
Given this
 > discrepancy the target JVM will output to stderr thread status, since
 > /proc/self/cwd/.attach_pid1 doesn't exist and won't continue with 
the attach
 > sequence.
 >
 > The solution is to parse /proc/pid/status for the field NSpid 
(available since
 > Linux 4.1) which contains a list of pids, where the last entry is the 
"inner
 > most" PID namespace value. (Namespaces can be stacked, unlike 
Solaris Zones
 > which have a virtualization depth of 1)
 >
 > The rest of the Linux attach sequence assumes a shared mount 
namespace by
 > waiting for /tmp/.java_pid17734 to appear. But if the attaching 
process is in a
 > separate namespace because the target JVM is in a mount namepsace 
(or in a
 > chroot as well) the unix domain socket for attaching won't appear.
 >
 > Instead the attach sequence should resolve file names relative to
 > /proc/17734/root which has a materialized view of the rootfs for the 
target.
 >
 > Thanks!
 >
 > TJ
 >
 
 
 
 

Re: [PATCH] attach in linux should be relative to /proc/pid/root and namespace aware

[Erik Gahlin]

I am not a (R)eviewer, so I can't give it my blessings, but I can 
sponsor it.


I will create a webrev so it easier to review.

Erik


I have attached a patch that allows jcmd to work against a java process running
inside a Docker container. Apologies if this is not in the correct format. It 
was
built against jdk8u. I couldn’t seem to find an existing JIRA for it.

Diagnostic commands (i.e. jcmd, jstack, etc) fail to attach to a target JVM
that is inside a container (e.g. Docker).

A Linux container often isolates a process in a PID and Mount namespace that is
separate from the "root container" (analogous to the hypervisor/dom0 in
hardware virtualization environments, or the global zone on Solaris). A target
JVM that is isolated in either a PID namespace, or a Mount namespace will fail
the attach sequence.

When the target JVM is in its own PID namespace the pid of the process is
distinct from what the real pid of the process as it relates to the root
container. For example, in the root container you can observe a JVM with a pid
of 17734, however if that JVM is running inside a Docker container the pid
inside its PID namespace is likely 1. So when the target JVM receives the
SIGQUIT it looks in /proc/self/cwd/ for .attach_pid1 however the external
attaching JVM has created the file /proc/17734/cwd/.attach_pid17734. Given this
discrepancy the target JVM will output to stderr thread status, since
/proc/self/cwd/.attach_pid1 doesn't exist and won't continue with the attach
sequence.

The solution is to parse /proc/pid/status for the field NSpid (available since
Linux 4.1) which contains a list of pids, where the last entry is the "inner
most" PID namespace value. (Namespaces can be stacked, unlike Solaris Zones
which have a virtualization depth of 1)

The rest of the Linux attach sequence assumes a shared mount namespace by
waiting for /tmp/.java_pid17734 to appear. But if the attaching process is in a
separate namespace because the target JVM is in a mount namepsace (or in a
chroot as well) the unix domain socket for attaching won't appear.

Instead the attach sequence should resolve file names relative to
/proc/17734/root which has a materialized view of the rootfs for the target.

Thanks!

TJ

Re: RFR - [RFE] JDK-8007141 : Introduce Dynamic MBean exposing the perf counters

[Erik Gahlin]

If somebody writes a tool that plots the values that the perf counters 
provide, which is bound to happen if it is exposed via JMX, but then the 
internals of the JVM is changed and the perf counter value is removed or 
it is incorrect.


Erik


Hi Erik,

I am not sure if Perf Counters could be disabled. If yes, then 
PerfCounter MBean must throw appropriate exception while getting 
MBeanInfo.


I don't know about tests for PerfCounter. The webrev contains tests 
for PerfCounter MBean.


I am not sure how we will be breaking other's code. Could you please 
elaborate?


Thanks
Harsha

On Monday 27 February 2017 07:39 PM, Erik Gahlin wrote:

Hi Harsha,

How will perf counters be supported?

Can they be removed at anytime, or only at major releases etc. Do we 
have tests for them? I think it would be good know, and perhaps state 
somewhere so we don't break people's code.


Erik


Hi All,

Please review the below RFE,

JDK-8007141 <http://JDK-8007141> : Introduce Dynamic MBean exposing 
the perf counters.


with webrev at,

http://cr.openjdk.java.net/~hb/8007141/webrev.00/

Appreciate if I can get inputs for below.

1. Location of HotSpotPerfCounterMBean. It is located at 
src/jdk.management/share/classes/com/sun/management/internal. It can 
be moved to src/jdk.management/share/classes/com/sun/management if 
required.


2. Javadoc comments for HotSpotPerfCounterMBean. Not sure if it is 
adequate.


3. Description for each attribute of MBean. I am using getUnits(), 
getVariability(), and getFlags() for description. I am not sure if 
that is the right description. Appreciate inputs from someone who 
knows perf counters well.


Thanks

Harsha

Re: RFR - [RFE] JDK-8007141 : Introduce Dynamic MBean exposing the perf counters

[Erik Gahlin]

Hi Harsha,

How will perf counters be supported?

Can they be removed at anytime, or only at major releases etc. Do we 
have tests for them? I think it would be good know, and perhaps state 
somewhere so we don't break people's code.


Erik


Hi All,

Please review the below RFE,

JDK-8007141  : Introduce Dynamic MBean exposing 
the perf counters.


with webrev at,

http://cr.openjdk.java.net/~hb/8007141/webrev.00/

Appreciate if I can get inputs for below.

1. Location of HotSpotPerfCounterMBean. It is located at 
src/jdk.management/share/classes/com/sun/management/internal. It can 
be moved to src/jdk.management/share/classes/com/sun/management if 
required.


2. Javadoc comments for HotSpotPerfCounterMBean. Not sure if it is 
adequate.


3. Description for each attribute of MBean. I am using getUnits(), 
getVariability(), and getFlags() for description. I am not sure if 
that is the right description. Appreciate inputs from someone who 
knows perf counters well.


Thanks

Harsha

Re: RFR : JDK-7132577 - javax/management/monitor/MultiMonitorTest.java fails in JDK8-B22

[Erik Gahlin]

Looks good.

Erik


Hello,

Please review this test bug fix which eliminates test case’s own 
 timeout mechanism to default jtreg timeout.


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

Webrev : 
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-7132577/webrev.00/ 



Thanks,

Amit

Re: RFR : JDK-8174196 : sun/management/jdp tests are not running properly

[Erik Gahlin]

Looks good.

Erik


Hello,

Please review following test case fix

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

Webrev : 
http://cr.openjdk.java.net/~asapre/webrev/2017/JDK-8174196/webrev.00/ 



Thanks,

Amit

Re: RFR(S): 8170672: Event-based tracing to support classloader instances

[Erik Gahlin]

Looks good.

Erik


Greetings,

Kindly asking for reviews for the following changeset:

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

Webrev: http://cr.openjdk.java.net/~mgronlun/8170672/webrev01/ 



(this work is covered by an FC exception)

Summary:

Event-based tracing previously had little information about class 
loaders; a class loader was essentially treated no different than from 
a regular class (type information only).


With JDK9, supported has been added to java.lang.ClassLoader to 
associate a name with an individual class loader instance.


This changeset will allow the name information of individual class 
loader instances to be provided by the event-based tracing framework.


Aux info:

Some folding of the numerous macros completed as well.

Thanks in advance
Markus

Re: [9] Review Request: 8143077 Deprecate InputEvent._MASK in favor of InputEvent._DOWN_MASK

[Erik Gahlin]

Looks good.

Erik


On Sep 30, 2016, at 8:45 AM, Sergey Bylokhov  wrote:

Hello.

Please review the fix for jdk9.

This is request to deprecate the obsolete flags inside InputEvent. This 
constants were marked as obsolete in jdk1.4 and were replaced by the new one. 
In jdk1.4 the documentation were update with notion that the new constants 
should be used. And this bug is about official deprecation of them.

We can replace old constants by the new one in the whole jdk, but I updated 
only the code in InputEvent to make change smaller and safer. So at least the 
new code in jdk and the code in applications will start to use the new 
constants.

The changes in jconsole are necessary to fix deprecation warning.

jprt build passed, no new issues were found by jck/jtreg tests.


Bug: https://bugs.openjdk.java.net/browse/JDK-8143077
Webrev can be found at: http://cr.openjdk.java.net/~serb/8143077/webrev.00

--
Best regards, Sergey.

Re: RFR: JDK-8166202: Tracefile gensrc cannot handle closed src dir in different location

[Erik Gahlin]

Looks good.

Erik

The xml/xsl source files used to generate sources for trace in hotspot 
have hard coded relative paths in them that make assumptions on where 
the oracle closed files are located. The source files should not make 
these kind of assumptions since that makes it difficult to change the 
repository layout structure.


While this is mostly an oracle internal change, there are some light 
changes in the open makefile too. The end result should be unchanged.


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

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

/Erik

Re: RFR: JDK-8165493: SA: Add method in GrowableArray.java to be able to access the 'data' field

[Erik Gahlin]

Could you change to:

return dataField.getValue(getAddress());

Otherwise it looks good. No need to upload new webrev. Not a (R)eviewer.

Thanks
Erik

On 2016-09-08 23:40, Poonam Bajaj Parhar wrote:

Hello,

Please review this small change that adds an accessor method to 
GrowableArray class for its 'data' field.


Bug: https://bugs.openjdk.java.net/browse/JDK-8165493
Webrev: http://cr.openjdk.java.net/~poonam/8165493/webrev/

This change is required for closed flight recorder related changes on 
the SA side.


Thanks,
Poonam

8164523: Clean up metadata for event based tracing

[Erik Gahlin]

Hi,

Could I have review of this fix to the event tracing metadata. The patch 
touches many files, but it doesn't change the control flow, only class 
and field names.


Webrev:
http://cr.openjdk.java.net/~egahlin/8164523/

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

Thanks
Erik

Re: RFR(XXS): 8162945: HotspotDiagnosticMXBean getFlags erroneously reports OutOfMemory

[Erik Gahlin]

Looks ok (not a reviewer)

Erik

On 2016-08-02 14:58, Markus Gronlund wrote:


Greetings,

Please review this small fix to address some new issues seen in 
testing where OOM is erroneously being reported:


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

Changeset:

# HG changeset patch

# User mgronlun

# Date 1470141649 -7200

#  Tue Aug 02 14:40:49 2016 +0200

# Node ID e06e6474ff5f5798f9249aeaa6b33ec6aa021563

# Parent 9672159305d72f5dd430a3edd4b97c4e5bc816e0

[mq]: 8162945

diff --git a/src/jdk.management/share/native/libmanagement_ext/Flag.c 
b/src/jdk.management/share/native/libmanagement_ext/Flag.c


--- a/src/jdk.management/share/native/libmanagement_ext/Flag.c

+++ b/src/jdk.management/share/native/libmanagement_ext/Flag.c

@@ -142,7 +142,7 @@

 continue;

 }

-if (valueObj == NULL) {

+if (valueObj == NULL && !(globals[i].type == 
JMM_VMGLOBAL_TYPE_JSTRING)) {


free(globals);

JNU_ThrowOutOfMemoryError(env, 0);

 return 0;

Summary:

OOM’s have manifested after the following change was integrated:

8162524: src/jdk.management/share/native/libmanagement_ext/Flag.c 
doesn't handle JNI exceptions


The following check was then added to 
jdk\src\jdk.management\share\native\libmanagement_ext\flags.c:


if (valueObj == NULL) {
free(globals);
JNU_ThrowOutOfMemoryError(env, 0);
return 0;
}

However, valueObj is not always the direct result of a failed JNI 
allocation, for example:


case JMM_VMGLOBAL_TYPE_JSTRING:
valueObj = globals[i].value.l;

The valueObj here (a JSTRING) is coming a VM allocation, more 
specifically from :


services/management.cpp jmm_GetVMGlobals()
...
add_global_entry()

} else if (flag->is_ccstr()) {
Handle str = java_lang_String::create_from_str(flag->get_ccstr(), 
CHECK_false); <<--

global->value.l = (jobject)JNIHandles::make_local(env, str());
global->type = JMM_VMGLOBAL_TYPE_JSTRING;

For certain ccstr flags, such as  the "HeapDumpFlag" for example, the 
ccstr() is NULL (i.e. not set).


So returning a NULL is fine here, the JNI NULL check validation needs 
to take this into account.


Thanks

Markus

Re: RFR(XS): 8158033: notify_tracing() misplaced for intended purpose

[Erik Gahlin]

Looks good!

Not a (R)eviewer

Erik

On 2016-05-27 11:33, Markus Gronlund wrote:


Greetings,

Please review this small fix:

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

Webrev: http://cr.openjdk.java.net/~mgronlun/8158033/webrev/ 



Description:

The intent when putting in the notify_tracing() hook into debug.cpp 
(report_java_out_of_memory()) was to intercept a state believed to be 
a VM termination state, especially when OOME is thrown. Since it is 
totally valid that Java code catches OOME, and this location actually 
goes back to Java, this is the wrong location for this hook.


In addition, the hook should not be typed for OOME only, but generic 
for any exit condition (normal / OOME / crash).
This should instead have been put into java.cpp (before_exit()) and in 
VMError.cpp (report_vm_die()).


Thanks

Markus

RFR (XS): 8154794,,Add support for experimental fields/events to event-based tracing

[Erik Gahlin]

Hi,

Could I have a review of this small fix that adds support for 
experimental events and experimental fields for event based tracing.


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

Webrev:
http://cr.openjdk.java.net/~egahlin/8154794/

Thanks
Erik

Re: RFR: 8149790: NegativeArraySizeException with hprof

[Erik Gahlin]

Looks good, not a Reviewer.

Do you really need curly braces in the switch clauses?

Erik

On 2016-04-15 16:40, Andreas Eriksson wrote:

Hi,

Please review this test fix for 8149790: NegativeArraySizeException 
with hprof


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

Changes are to the hprof verifier, which now will pass heap dump 
content around as JavaThing arrays instead of byte arrays, since the 
latter cannot be guaranteed to be able to hold all the elements of 
large arrays.


There is still a problem where the test will timeout on machines with 
lots of memory (seen on machines with 200+GB of memory) because the 
verification takes a long time. I'll file a new bug for that problem.


Regards,
Andreas

Re: RFR(XS): 8152119: Event-based tracing to allow for tracing Klass definition

[Erik Gahlin]

Looks good. Not a reviewer.

Erik

On 2016-03-17 19:43, Markus Gronlund wrote:


Greetings,

Kindly asking for reviews for the following change to allow for 
tracing Klass definitions.


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

Webrev: http://cr.openjdk.java.net/~mgronlun/8152119/webrev01/ 



This change complements the change associated with Klass creation 
which is already integrated:


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

Thanks

Markus

Re: RFR(XS) serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java failing because expects HPROF JAVA PROFILE 1.0.1 file format

[Erik Gahlin]

Thanks Dan,

I missed that Andreas has started working on the bug and sent out a review.

Sorry about that. I will reassign back to Andreas.

Erik

On 2016-03-03 00:14, Daniel D. Daugherty wrote:

Just caught up on the bug updates so now I see that you (Erik G)
have taken over the bug...

I'm good with this fix also...

Dan


On 3/2/16 4:08 PM, Daniel D. Daugherty wrote:

Hi Erik,

I reviewed a different fix for the same bug from Andreas E.
earlier today.

Dan

On 3/2/16 3:18 PM, Markus Gronlund wrote:

Hi Erik,

I think this looks good.

Thanks for fixing.

Markus

-Original Message-
From: Erik Gahlin
Sent: den 2 mars 2016 23:08
To: serviceability-dev@openjdk.java.net
Subject: RFR(XS) 
serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java failing 
because expects HPROF JAVA PROFILE 1.0.1 file format


Hi,

Could I have a review of a fix for

"8150986: serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
failing because expects HPROF JAVA PROFILE 1.0.1 file format"
https://bugs.openjdk.java.net/browse/JDK-8150986

It's a test update to accommodate the removal of 1.0.1 HPROF file 
format support that happened with

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

Thanks
Erik

diff --git
a/test/serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
b/test/serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
--- a/test/serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
+++ b/test/serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
@@ -54,7 +54,6 @@

   public class JMapHProfLargeHeapTest {
   private static final String HEAP_DUMP_FILE_NAME = "heap.hprof";
-private static final String HPROF_HEADER_1_0_1 = "JAVA PROFILE 
1.0.1";
   private static final String HPROF_HEADER_1_0_2 = "JAVA 
PROFILE 1.0.2";

   private static final long M = 1024L;
   private static final long G = 1024L * M; @@ -79,17 +78,7 @@
   }
   }

-// Small heap 22 megabytes, should create 1.0.1 file format
-testHProfFileFormat("-Xmx1g", 22 * M, HPROF_HEADER_1_0_1);
-
-/**
- * This test was deliberately commented out since the test
system lacks
- * support to handle the requirements for this kind of heap
size in a
- * good way. If or when it becomes possible to run this 
kind of

tests in
- * the test environment the test should be enabled again.
- * */
-// Large heap 2,2 gigabytes, should create 1.0.2 file format
-// testHProfFileFormat("-Xmx4g", 2 * G + 2 * M,
HPROF_HEADER_1_0_2);
+testHProfFileFormat("-Xmx1g", 22 * M, HPROF_HEADER_1_0_2);
   }

   private static void testHProfFileFormat(String vmArgs, long 
heapSize,

RFR(XS) serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java failing because expects HPROF JAVA PROFILE 1.0.1 file format

[Erik Gahlin]

Hi,

Could I have a review of a fix for

"8150986: serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java 
failing because expects HPROF JAVA PROFILE 1.0.1 file format"

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

It's a test update to accommodate the removal of 1.0.1 HPROF file format 
support that happened with

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

Thanks
Erik

diff --git 
a/test/serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java 
b/test/serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java

--- a/test/serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
+++ b/test/serviceability/sa/jmap-hprof/JMapHProfLargeHeapTest.java
@@ -54,7 +54,6 @@

 public class JMapHProfLargeHeapTest {
 private static final String HEAP_DUMP_FILE_NAME = "heap.hprof";
-private static final String HPROF_HEADER_1_0_1 = "JAVA PROFILE 1.0.1";
 private static final String HPROF_HEADER_1_0_2 = "JAVA PROFILE 1.0.2";
 private static final long M = 1024L;
 private static final long G = 1024L * M;
@@ -79,17 +78,7 @@
 }
 }

-// Small heap 22 megabytes, should create 1.0.1 file format
-testHProfFileFormat("-Xmx1g", 22 * M, HPROF_HEADER_1_0_1);
-
-/**
- * This test was deliberately commented out since the test 
system lacks
- * support to handle the requirements for this kind of heap 
size in a
- * good way. If or when it becomes possible to run this kind of 
tests in

- * the test environment the test should be enabled again.
- * */
-// Large heap 2,2 gigabytes, should create 1.0.2 file format
-// testHProfFileFormat("-Xmx4g", 2 * G + 2 * M, 
HPROF_HEADER_1_0_2);

+testHProfFileFormat("-Xmx1g", 22 * M, HPROF_HEADER_1_0_2);
 }

 private static void testHProfFileFormat(String vmArgs, long heapSize,

Re: RFR(XXS): 8151053: com/sun/jdi/StepTest.java fails in 2016-03-01 JDK9-hs-rt nightly

[Erik Gahlin]

Looks good. Not a reviewer.

Erik

On 2016-03-02 20:49, Markus Gronlund wrote:


Greetings,

Could a please ask for reviews for the following simple fix to resolve 
a test issue associated with com/sun/jdi/StepTest.java


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

Comment:

Need to filter out some classes that has moved to another package

Patch is attached.

Thanks in advance

Markus

Re: RFR(xs): 8147442: Event-based tracing to allow for tracing Klass creation

[Erik Gahlin]

Looks good!

Erik

On 2016-01-15 10:52, Markus Gronlund wrote:


Greetings,

Please review this small change in order to allow for the Event-based 
tracing framework to trace the creation of Klass’es.


Bug:

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

Webrev:

http://cr.openjdk.java.net/~mgronlun/8147442/webrev01/ 



Thanks

Markus

Re: RFR: JDK-8145788: JVM crashes with -XX:+EnableTracing

[Erik Gahlin]

Sorry for the delay, we are looking into it.

Erik

On 2016-01-06 07:36, David Holmes wrote:
> Pinging the serviceability tracing experts please!
>
> David
>
> On 24/12/2015 12:25 AM, Yasumasa Suenaga wrote:
>> Hi David,
>>
  1. Initialize JavaThread before calling apply_ergo() in create_vm().
>>> That is not likely to be an option - it would likely be far too
>>> disruptive to the initialization sequence.
>> Agree. Thus I choose 2.
>>
>>> We will have to wait for the tracing experts to have a good look at
>>> this.
>> I'm waiting that the tracing experts join this discussion.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> On 2015/12/23 13:20, David Holmes wrote:
>>> Hi Yasumasa,
>>>
>>> On 23/12/2015 11:49 AM, Yasumasa Suenaga wrote:
 Hi David,

 I've added callstack when JVM crashed:
  
 https://bugs.openjdk.java.net/browse/JDK-8145788?focusedCommentId=13880225=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13880225
>>> Thanks for that.
>>>
 This crash occurrs in Arguments::apply_ergo() in Threads::create_vm().
 apply_ergo() calls before JavaThread initialization.

 I think that TraceEvent can avoid crash with two approach:

  1. Initialize JavaThread before calling apply_ergo() in create_vm().
>>> That is not likely to be an option - it would likely be far too
>>> disruptive to the initialization sequence.
>>>
  2. Avoid crash at TraceEvent when it is called before JavaThread 
 initialization.
>>> Or don't call it at all.
>>>
>>> We will have to wait for the tracing experts to have a good look at
>>> this. We end up in code that is not expecting to be run before more of
>>> the VM is initialized, so we have to look at what else may be being
>>> assumed and then decide whether to handle the situation or avoid it.
>>>
>>> Thanks,
>>> David
>>>
 Thanks,

 Yasumasa


 On 2015/12/22 21:19, David Holmes wrote:
> On 19/12/2015 1:50 AM, Yasumasa Suenaga wrote:
>> Hi all,
>>
>> I encountered JVM crash when I passed -XX:+EnableTracing.
>>
>>   I checked core image, it crashed in 
>> EventBooleanFlagChanged::writeEvent()
>>   which is called by Arguments::apply_ergo() because thread had not 
>> been
>>   initialized. (JVM seems to log changing GC algorithm to G1.)
> This seems like a logic error to me - something is trying to happen too
> early during VM initialization. We need to look at exactly what we are
> trying to do here, not just work around the crash.
>
> David
> -
>
>>   writeEvent() uses ResourceMark. Default constructor of 
>> ResourceMark uses
>>   ResourceArea in current thread. So ResourceMark in writeEvent() 
>> should
>>   pass valid ResourceArea.
>>
>> I think this issue is in traceEventClasses.xsl .
>> However, my environment (GCC 5.3.1 on Fedora23) cannot build it because
>> -Werror=maybe-uninitialized was occurred.
>> So I also fixed them together.
>>
>> I've uploaded webrev. Could you review it?
>>http://cr.openjdk.java.net/~ysuenaga/JDK-8145788/webrev.00/
>>
>> I'm jdk9 committer, however I cannot access JPRT.
>> So I need a sponsor.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>

Re: RFR(XS): 8066814: Reduce accessibility in TraceEvent

[Erik Gahlin]

Looks good, not a reviewer.

Erik

On 2014-12-17 15:45, Markus Grönlund wrote:


Greetings,

Kindly asking for reviews for the following changeset:

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

Webrev: http://cr.openjdk.java.net/~mgronlun/8066814/webrev01/ 



Description:

TraceEvent currently exposes internals unnecessarily.

Therefore:

Remove unnecessarily exposed methods.
Add assert for not committing a cancelled event.
Add method stubs for !INCLUDE_TRACE

Thanks in advance

Markus

Re: RFR(XS): 8143235: Remove libjfr mapfile

[Erik Gahlin]

Looks good, not a reviewer.

Erik

On 2015-11-18 21:09, Markus Gronlund wrote:


Greetings,

Kindly asking for reviews for the following:

Bug:

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

Summary:

Remove libjfr mapfile (cleanup) + adds “jdk.management.jfr” to boot 
modules


Patch is attached and also unrolled here:

# HG changeset patch

# User mgronlun

# Date 1447765226 -3600

#  Tue Nov 17 14:00:26 2015 +0100

# Node ID 13e7746e92f9b9651c0f77ed04057f382cc9e477

# Parent 85c9ddeda1c3fe13ef64ce1005c09af8e0bf1f42

[mq]: jdk_src_open_remove_mapfile

diff --git a/make/mapfiles/libjfr/mapfile-vers 
b/make/mapfiles/libjfr/mapfile-vers


deleted file mode 100644

--- a/make/mapfiles/libjfr/mapfile-vers

+++ /dev/null

@@ -1,45 +0,0 @@

-#

-# Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights 
reserved.


-# ORACLE PROPRIETARY/CONFIDENTIAL. Use is subject to license terms.

-#

-

-# Define library interface.

-

-SUNWprivate_1.1 {

-  global:

- Java_oracle_jrockit_jfr_Process_getpid;

- Java_oracle_jrockit_jfr_Timing_counterTime;

- Java_oracle_jrockit_jfr_Timing_init;

- Java_oracle_jrockit_jfr_Logger_output0;

- Java_oracle_jrockit_jfr_JFR_isCommercialFeaturesUnlocked;

- Java_oracle_jrockit_jfr_JFR_isStarted;

- Java_oracle_jrockit_jfr_JFR_isSupportedInVM;

- Java_oracle_jrockit_jfr_JFR_startFlightRecorder;

- Java_oracle_jrockit_jfr_JFR_isDisabledOnCommandLine;

- Java_oracle_jrockit_jfr_JFR_isEnabled;

- Java_oracle_jrockit_jfr_VMJFR_options;

- Java_oracle_jrockit_jfr_VMJFR_init;

- Java_oracle_jrockit_jfr_VMJFR_addConstPool;

- Java_oracle_jrockit_jfr_VMJFR_removeConstPool;

- Java_oracle_jrockit_jfr_VMJFR_storeConstPool;

- Java_oracle_jrockit_jfr_VMJFR_classID0;

- Java_oracle_jrockit_jfr_VMJFR_stackTraceID;

- Java_oracle_jrockit_jfr_VMJFR_threadID;

- Java_oracle_jrockit_jfr_VMJFR_rotate;

- Java_oracle_jrockit_jfr_VMJFR_shutdown;

- Java_oracle_jrockit_jfr_VMJFR_start;

- Java_oracle_jrockit_jfr_VMJFR_stop;

- Java_oracle_jrockit_jfr_VMJFR_buffer;

- Java_oracle_jrockit_jfr_VMJFR_flush;

- Java_oracle_jrockit_jfr_VMJFR_write;

- Java_oracle_jrockit_jfr_VMJFR_add;

- Java_oracle_jrockit_jfr_VMJFR_remove;

- Java_oracle_jrockit_jfr_VMJFR_setThreshold;

- Java_oracle_jrockit_jfr_VMJFR_setPeriod;

- Java_oracle_jrockit_jfr_VMJFR_getPeriod;

- Java_oracle_jrockit_jfr_VMJFR_descriptors;

- Java_oracle_jrockit_jfr_VMJFR_retransformClasses0;

-  JNI_OnLoad;

-  local:

-  *;

-};

diff --git a/make/src/classes/build/tools/module/boot.modules 
b/make/src/classes/build/tools/module/boot.modules


--- a/make/src/classes/build/tools/module/boot.modules

+++ b/make/src/classes/build/tools/module/boot.modules

@@ -23,6 +23,7 @@

jdk.jfr

jdk.management

jdk.management.cmm

+jdk.management.jfr

jdk.management.resource

jdk.naming.rmi

jdk.sctp

Re: 8143226 (M): Minor updates to Event Based tracing

[Erik Gahlin]

Looks good, not a reviewer.

Erik

On 2015-11-18 14:30, Markus Gronlund wrote:


Greetings,

Kindly asking for reviews for the following bug:

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

Webrev:

http://cr.openjdk.java.net/~mgronlun/8143226/webrev01/ 



Summary:


- Remove obsoleted files from make system
- Consolidate to a single Thread abstraction
- Expand relational keys to u8's to introduce uniqueness property
- Tracing hook to be notified on out of memory
- Tracing hook in early vm startup
- Event "starttime" to become the primary time stamp for the event 
(previously it was "endtime")


Thanks in advance

Markus

Re: RFR 8134641: serviceability/dcmd/compiler/CodelistTest.java fails on sun.misc.Unsafe.getUnsafe

[Erik Gahlin]

Looks good, not a (R)eviewer.

Erik

Den 09/09/15 kl. 12:54, skrev Alexander Kulyakhtin:

Hi,

Could someone, please, review the small, test-only fix in the mail below?

Best regards,
Alexander

- Original Message -
From: alexander.kulyakh...@oracle.com
To: serviceability-dev@openjdk.java.net
Sent: Monday, September 7, 2015 7:56:45 PM GMT +03:00 Iraq
Subject: Re: RFR 8134641: serviceability/dcmd/compiler/CodelistTest.java fails 
on sun.misc.Unsafe.getUnsafe


The fix has been updated to make sure that strings matching "sun.misc.Unsafe.getUnsafe", 
and not simply "getUnsafe" get filtered

Webrev: http://cr.openjdk.java.net/~akulyakh/8134641_01/index.html

Best regards,
Alexander

- Original Message -
From: alexander.kulyakh...@oracle.com
To: serviceability-dev@openjdk.java.net
Sent: Monday, September 7, 2015 7:35:09 PM GMT +03:00 Iraq
Subject: RFR 8134641: serviceability/dcmd/compiler/CodelistTest.java fails on 
sun.misc.Unsafe.getUnsafe

Could you, please, review the following small test-only change:

Issue: https://bugs.openjdk.java.net/browse/JDK-8134641 
"serviceability/dcmd/compiler/CodelistTest.java fails with "Test failed on: 
sun.misc.Unsafe.getUnsafe()Lsun/misc/Unsafe;"
Webrev: http://cr.openjdk.java.net/~akulyakh/8134641/index.html

The test calls Jcmd (diagnostic command tool) Compiler.codelist and then parses 
the output, making sure that the first methods in the list is valid by 
reflection.

However Unsafe.getUnsafe() method is hidden from reflection.
Before the fix the test did not take that into account and failed whenever 
Unsafe.getUnsafe happened to be among the methods to be validated.

The test has been changed to skip Unsafe.getUnsafe() method if present in the 
test input.

Best regards,
Alexander

Re: RFR(XXS): 8130057: serviceability/sa/TestStackTrace.java should be quarantined

[Erik Gahlin]

Looks good

Erik

Yekaterina Kantserova skrev den 14/07/15 11:44:

Hi,

Could I please have a review of this very small fix.

bug: https://bugs.openjdk.java.net/browse/JDK-8130057
webrev: http://cr.openjdk.java.net/~ykantser/8130057/webrev.00/

Thanks,
Katja

Re: RFR 8054890: Serviceability: New diagnostic commands 'VM.set_flag' and 'JVMTI.data_dump'

[Erik Gahlin]

Looks good!

Erik

Jaroslav Bachorik skrev 2015-03-19 10:59:

Please, review the following change

Issue : https://bugs.openjdk.java.net/browse/JDK-8054890
Webrev: http://cr.openjdk.java.net/~jbachorik/8054890/webrev.00

This patch is about adding 2 new diagnostic commands - VM.set_flag and 
JVMTI.data_dump.


VM.set_flag allows to set any writeable flag. It takes the flag name 
and the flag value in textual form. The mutability of the flag and the 
value format checks are forwarded to the shared vm management code.


JVMTI.data_dump will send the data dump request to JVMTI.

Both of these commands are covered by the corresponding tests.

Thanks,

-JB-

Re: RFR(XXS): 8064923: [TESTBUG] jps doesn't display anything on embedded platforms and it causes some tests to fail

[Erik Gahlin]

Looks good.

Erik

Yekaterina Kantserova skrev den 18/03/15 17:37:

Hi,

Could I please have a review of this very small fix.

bug: https://bugs.openjdk.java.net/browse/JDK-8064923
webrev: http://cr.openjdk.java.net/~ykantser/8064923/webrev.00/

Thanks,
Katja

Re: RFR: JDK-8075056 Remove Version.java.template from jconsole

[Erik Gahlin]

Looks good, jconsole now compile in Eclipse!

Erik

Staffan Larsen skrev 2015-03-12 13:54:
The build for jconsole currently takes a template file and inserts the 
version number of the build into the file. We can simplify this by 
removing the template file and reading the java.runtime.version system 
property at runtime.


bug: https://bugs.openjdk.java.net/browse/JDK-8075056
webrev: http://cr.openjdk.java.net/~sla/8075056/webrev.00/ 
http://cr.openjdk.java.net/%7Esla/8075056/webrev.00/


Thanks,
/Staffan

Re: RFR(L): 8071908, 8071909: Port internal Diagnostic Command tests and test framework to jtreg (plus some testlibrary changes)

[Erik Gahlin]

Looks good!

Erik

Mikael Auno skrev 2015-01-30 10:44:

Hi, could I please some reviews for this test port?

Issues: https://bugs.openjdk.java.net/browse/JDK-8071908
 https://bugs.openjdk.java.net/browse/JDK-8071909
Webrev: http://cr.openjdk.java.net/~miauno/8071908_8071909/webrev.00/

Read on for the rationale on a few questions that might arise.

* Why two issues?

These changes are mainly a port of the Diagnostic Command (DCMD) tests
and corresponding framework/utility classes from an internal (non-open)
test framework to jtreg. The reason for the two issues is that the
changes depend on a few modifications to testlibrary that are available
in jdk/test but not yet in hotspot/test, as well as a small new addition
to OutputAnalyzer, that are not specific to main subject (i.e. the DCMD
tests). To keep the history as clean and coherent as possible, those
changes will go in under JDK-8071909 while the new tests and
DCMD-related additions to testlibrary go in under JDK-8071908.

* Isn't there already utility classes for calling Diagnostic Commands?

The main idea with the new utility classes in testlibrary is to provide
a single interface to call Diagnostic Commands from tests, regardless of
the transport used (e.g. jcmd or JMX). There are a few tests scattered
around jdk/test and hotspot/test today that already utilize Diagnostic
Commands in some way, but they either use different utility classes for
this in different places or just do it directly in the test. Also, some
of these utility classes or tests go through jcmd and some through JMX
(most often without any real requirement for one transport over the
other in the test). All this means that there are, today, numerous
different implementations for calling Diagnostic Commands, and
consequently a lot of code duplication. These utility classes are meant
to replace all of these implementations, and with a single interface
regardless of the transport at that.

* You've missed this or that test using one of the existing utility classes!

This is by design. In order to keep the change at a more manageable
size and to get the core of this change in sooner, we've chosen to do
this transition in multiple steps. The first of those steps is what is
in this review request; the core utility classes, the tests ported from
the internal test framework and the adaption of the tests already in
hotspot/test/serviceability/dcmd (since they happened to reside in the
directory where we wanted to put the ported tests). When this is
integrated and have gone a few rounds through nightly testing, the
adaption of other tests in hotspot/test will follow, and after that
jdk/test.

Thanks,
Mikael

Re: RFR(XS): 8066814: Reduce accessibility in TraceEvent

[Erik Gahlin]

Looks good

Erik

Markus Grönlund skrev 2014-12-17 15:45:

Greetings,

  


Kindly asking for reviews for the following changeset:

  


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

Webrev: http://cr.openjdk.java.net/~mgronlun/8066814/webrev01/

  


Description:

TraceEvent currently exposes internals unnecessarily.

  


Therefore:

  


Remove unnecessarily exposed methods.
Add assert for not committing a cancelled event.
Add method stubs for !INCLUDE_TRACE

  


Thanks in advance

Markus

Re: RFR(S): 6977426: sun/tools tests can intermittently fail to find app's Java pid

[Erik Gahlin]

You could use a lambda instead of a boolean and the duplicated 
functions, i.e.


public static void main(String[] args) throws Exception {
if (args[0].equals(Logger)) {
testIfLeaking(TestLoggerWeakRefLeak::getLogger);
} else {
testIfLeaking(TestLoggerWeakRefLeak::getAnonymousLogger);
}
}

private static void testIfLeaking(Runnable getLogger) {
...
getLogger.run();
...
}

Erik

Yekaterina Kantserova skrev 2014-12-16 16:57:

Hi,

Could I please have a review of this fix.

bug: https://bugs.openjdk.java.net/browse/JDK-6977426
webrev: http://cr.openjdk.java.net/~ykantser/6977426/webrev.00/

java/util/logging/LoggerWeakRefLeak.sh and 
java/util/logging/AnonLoggerWeakRefLeak.sh are merged and rewritten in 
java. sun/tools/common/CommonTests.sh is removed because it doesn't 
test the product but sun/tool/common library functionality.


Thanks,
Katja

Re: RFR: JDK-8066441 - Add PLAB trace event

[Erik Gahlin]

Looks good!


Staffan Friberg skrev 02/12/14 19:50:

Hi,

As noted in the original thread [1] about this event we split up the 
commit in 4 different steps. This is the first step that only adds the 
event and methods to send them, but the usage will be added separately 
for the 3 GCs using PLABs.


Bug (sub-task): https://bugs.openjdk.java.net/browse/JDK-8066441
Webrev: http://cr.openjdk.java.net/~sfriberg/JDK-8066441/webrev.00

Thanks,
Staffan

[1] - (RFR: JDK-8055845 - Add trace event for promoted objects) - 
http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2014-December/011477.html 

Re: RFR(XS) 6364329 jstat displays invalid argument count with usage

[Erik Gahlin]

Looks good, and works on all platforms I have tried it with.

I have put up your changeset for review here:
http://cr.openjdk.java.net/~egahlin/6364329_0/

As soon as I get Reviewer approval,  I will update with reviewers name 
and push your change.


Thanks for contributing!

Erik

Yuri Gaevsky skrev 2014-11-28 12:33:

Thanks for the clarifications, Erik.

Please see below the updated patch for JDK-6364329:

--- start ---
$ hg diff
diff --git a/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java 
b/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java
--- a/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java
+++ b/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java
@@ -141,8 +141,9 @@
  public Arguments(String[] args) throws IllegalArgumentException {
  int argc = 0;
  
-if (args.length  1) {

-throw new IllegalArgumentException(invalid argument count);
+if (args.length == 0) {
+help = true;
+return;
  }
  
  if ((args[0].compareTo(-?) == 0)

diff --git a/test/sun/tools/jstat/jstatHelp.sh 
b/test/sun/tools/jstat/jstatHelp.sh
--- a/test/sun/tools/jstat/jstatHelp.sh
+++ b/test/sun/tools/jstat/jstatHelp.sh
@@ -22,9 +22,9 @@
  #
  
  # @test

-# @bug 4990825
+# @bug 4990825 6364329
  # @run shell jstatHelp.sh
-# @summary Test that output of 'jstat -?' matches the usage.out file
+# @summary Test that output of 'jstat -?', 'jstat -help' and 'jstat' matches 
the usage.out file
  
  . ${TESTSRC-.}/../../jvmstat/testlibrary/utils.sh
  
@@ -38,7 +38,7 @@

  diff -w jstat.out ${TESTSRC}/usage.out
  if [ $? != 0 ]
  then
-  echo Output of jstat -? differ from expected output. Failed.
+  echo Output of jstat -? differs from expected output. Failed.
exit 1
  fi
  
@@ -48,7 +48,17 @@

  diff -w jstat.out ${TESTSRC}/usage.out
  if [ $? != 0 ]
  then
-  echo Output of jstat -help differ from expected output. Failed.
+  echo Output of jstat -help differs from expected output. Failed.
+  exit 1
+fi
+
+rm -f jstat.out 2/dev/null
+${JSTAT} -J-XX:+UsePerfData  jstat.out 21
+
+diff -w jstat.out ${TESTSRC}/usage.out
+if [ $? != 0 ]
+then
+  echo Output of jstat differs from expected output. Failed.
exit 1
  fi
--- end ---

Best regards,
-Yuri

-Original Message-
From: Erik Gahlin [mailto:erik.gah...@oracle.com]
Sent: Friday, November 28, 2014 12:00 AM
To: Yuri Gaevsky
Cc: serviceability-dev@openjdk.java.net
Subject: Re: RFR(XS) 6364329 jstat displays invalid argument count with usage

First make sure your change doesn't break existing jtreg tests, the ones
in jdk/test/sun/tools/jstat.

If everything is fine, you could add output verification to
jstatHelp.sh, similar to what exists today for -? and -help.

Thanks
Erik

Re: RFR 8065783: DCMD parser fails to recognize one character argument when it's positioned last

[Erik Gahlin]

Looks ok,

Things that could be fixed,  but not necessary.

src/share/vm/services/diagnosticFramework.cpp : 63 // skipping spaces
could be changed to skipping delimiters

src/share/vm/services/diagnosticFramework.cpp : 66 _cursor = _len - 1;
Could be removed, since it must be true to enter if clause.

Erik

Jaroslav Bachorik skrev 2014-11-26 14:23:

Please, review the following change

Issue : https://bugs.openjdk.java.net/browse/JDK-8065783
Webrev: http://cr.openjdk.java.net/~jbachorik/8065783/webrev.00

Currently, the DCMD parser fails to recognize the trailing one 
character long arguments. This is caused by the parser ignoring the 
last character when immediately following a delimiter char.


The fix itself consists of checking whether the last character is 
indeed a delimiter char 
(src/share/vm/services/diagnosticFramework.cpp#68).


The rest of the patch deals with testing - the WhiteBox support needs 
to be enhanced to handle DCMD arguments (currently only options) and 
custom delimiters (currently only comma).


Thanks,

-JB-

Re: RFR(XS) 6364329 jstat displays invalid argument count with usage

[Erik Gahlin]

Hi Yuri,

Not sure what the correct behavior is. This is what I get:

jinfo - prints help
jhat  - prints ERROR: No arguments supplied + help
jstack -prints help
jstatd -prints Could not create remote object and a security exception
jstat - prints invalid argument count + help
jmap - prints help

I'm not a reviewer, but I can sponsor your patch if it is accepted. If 
so, could you update the test as well.


Thanks
Erik


Yuri Gaevsky skrev 2014-11-26 16:08:

ping...

-Original Message-
From: serviceability-dev [mailto:serviceability-dev-boun...@openjdk.java.net] 
On Behalf Of Yuri Gaevsky
Sent: Thursday, November 20, 2014 1:11 AM
To: serviceability-dev@openjdk.java.net
Subject: RFR(XS) 6364329 jstat displays invalid argument count with usage

Hello.

Please see below a small fix for integration into JDK 9:

$ hg diff
diff --git a/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java 
b/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java
--- a/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java
+++ b/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java
@@ -141,8 +141,9 @@
  public Arguments(String[] args) throws IllegalArgumentException {
  int argc = 0;
  
-if (args.length  1) {

-throw new IllegalArgumentException(invalid argument count);
+if (args.length == 0) {
+help = true;
+return;
  }
  
  if ((args[0].compareTo(-?) == 0)



Thanks,
-Yuri

Re: RFR(XS) 6364329 jstat displays invalid argument count with usage

[Erik Gahlin]

First make sure your change doesn't break existing jtreg tests, the ones 
in jdk/test/sun/tools/jstat.


If everything is fine, you could add output verification to 
jstatHelp.sh, similar to what exists today for -? and -help.


Thanks
Erik

Yuri Gaevsky skrev 2014-11-27 18:10:

Hi Erik,

The change just makes jstat's behaviour more consistent with jinfo/jstack/jmap.
Please also notice that there is a bug [1] about jhat tool removal (in Java SE 
9?).


I'm not a reviewer, but I can sponsor your patch if it is accepted. If
so, could you update the test as well.

It would be great, thank you so much. What test in 'jdk/test/sun/tools/jstat/' 
should be
updated for that?

Best regards,
-Yuri

[1] https://bugs.openjdk.java.net/browse/JDK-8044830

-Original Message-
From: serviceability-dev [mailto:serviceability-dev-boun...@openjdk.java.net] 
On Behalf Of Erik Gahlin
Sent: Thursday, November 27, 2014 6:29 PM
To: serviceability-dev@openjdk.java.net
Subject: Re: RFR(XS) 6364329 jstat displays invalid argument count with usage

Hi Yuri,

Not sure what the correct behavior is. This is what I get:

jinfo - prints help
jhat  - prints ERROR: No arguments supplied + help
jstack -prints help
jstatd -prints Could not create remote object and a security exception
jstat - prints invalid argument count + help
jmap - prints help

I'm not a reviewer, but I can sponsor your patch if it is accepted. If
so, could you update the test as well.

Thanks
Erik


Yuri Gaevsky skrev 2014-11-26 16:08:

ping...

-Original Message-
From: serviceability-dev [mailto:serviceability-dev-boun...@openjdk.java.net] 
On Behalf Of Yuri Gaevsky
Sent: Thursday, November 20, 2014 1:11 AM
To: serviceability-dev@openjdk.java.net
Subject: RFR(XS) 6364329 jstat displays invalid argument count with usage

Hello.

Please see below a small fix for integration into JDK 9:

$ hg diff
diff --git a/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java 
b/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java
--- a/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java
+++ b/src/jdk.jcmd/share/classes/sun/tools/jstat/Arguments.java
@@ -141,8 +141,9 @@
   public Arguments(String[] args) throws IllegalArgumentException {
   int argc = 0;
   
-if (args.length  1) {

-throw new IllegalArgumentException(invalid argument count);
+if (args.length == 0) {
+help = true;
+return;
   }
   
   if ((args[0].compareTo(-?) == 0)



Thanks,
-Yuri

RFR(S): 6618335: ThreadReference.stop(null) throws NPE instead of InvalidTypeException

[Erik Gahlin]

Hi,

Could I have a review of a small fix to the JDI implementation.

If ThreadReference#stop(null) is called, a NPE is triggered before the 
code that should throw an InvalidTypeException is reached. Fix is to use 
validateMirrorOrNull, which ignores the vaildation if it's null.


This is similar to how ThreadReference#forceEarlyReturn is implemented. 
Tested with jdk_jdi.


Thanks
Erik

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

Webrev:
http://cr.openjdk.java.net/~egahlin/6618335/

Re: RFR 8064441: java/lang/management/ThreadMXBean/Locks.java fails intermittently, blocked on wrong object

[Erik Gahlin]

Looks good!

Erik

Jaroslav Bachorik skrev 2014-11-26 13:57:

Please, review the following test change

Issue : https://bugs.openjdk.java.net/browse/JDK-8064441
Webrev: http://cr.openjdk.java.net/~jbachorik/8064441/webrev.00

The test fails because of System.out.println() may induce unexpected 
locking. The solution is to use an already existing LockFreeLogManager 
library class to collect the logs in the lock-free manner and print 
the logs only after all the test code has been run.


Thanks,

-JB-

Re: RFR(S): 6542634: TEST BUG: MISC_REGRESSION tests need to have minimum timeouts examined

[Erik Gahlin]

Looks good!

Erik

Yekaterina Kantserova skrev 2014-11-18 12:07:

Hi,

Could I please have a review of this fix.

In this fix I take an opportunity to refactor sun/tools/jinfo/Basic.sh 
and to add more tests for jinfo utility. sun/tools/jinfo/Basic.sh is a 
last unstable test among tests listed 
inhttps://bugs.openjdk.java.net/browse/JDK-6542634  that's why I've 
chosen to use this bug number instead of creating a new one.


bug: https://bugs.openjdk.java.net/browse/JDK-6542634
webrev: http://cr.openjdk.java.net/~ykantser/6542634/webrev.00/

The tests have been run and passed on all basic platforms.

Thanks,
Katja

Re: RFR(S): JDK-8065220: Include alternate sa.make file for MacOSX

[Erik Gahlin]

Looks good

Erik

Poonam Bajaj skrev 2014-11-18 15:33:

Hello,

Could I request reviews for this simple change to include alternate 
sa.make file in make/bsd/makefiles/sa.make:


Bug:JDK-8065220: https://bugs.openjdk.java.net/browse/JDK-8065220 
Include alternate sa.make file for MacOSX

Webrev: http://cr.openjdk.java.net/~poonam/8065220/webrev.00/
Testing: JPRT

Thanks,
Poonam

Re: RFR 8058506: ThreadMXBeanStateTest throws exception

[Erik Gahlin]

Looks good!

Erik

Jaroslav Bachorik skrev 2014-10-23 11:45:

On 10/21/2014 07:47 PM, Jaroslav Bachorik wrote:

On 10/21/2014 03:27 PM, Erik Gahlin wrote:

Have you considered creating a LogMessage class that keeps the logCntr
value and the log message, instead of putting the counter into the log
string and parsing it.


Yes. And didn't go that way in order to prevent creating a lot of
throwaway stringbuilder instances (the Formatter works that way) - but
it might (almost certainly) be a premature optimization. I will clean it
up and resubmit the request.


http://cr.openjdk.java.net/~jbachorik/8058506/webrev.01

I moved the log collection logic to a separate class available in the 
testlib now. The code is much more concise now.


-JB-



-JB-



Seems simpler and easier to understand.

Erik

Jaroslav Bachorik skrev 2014-10-20 13:12:

Please, review the following test change

Issue : https://bugs.openjdk.java.net/browse/JDK-8058506
Webrev: http://cr.openjdk.java.net/~jbachorik/8058506/webrev.00

The test fails intermittently due to the log printing blocking the
test thread from time to time, resulting in incorrect data reported by
ThreadMXBean.

The solution is to use per-thread non-blocking StringBuilders (wrapped
in Formatter instances) and aggregate the log output only after the
test is finished.

Thanks,

-JB-

Re: RFR 8058506: ThreadMXBeanStateTest throws exception

[Erik Gahlin]

Have you considered creating a LogMessage class that keeps the logCntr 
value and the log message, instead of putting the counter into the log 
string and parsing it.


Seems simpler and easier to understand.

Erik

Jaroslav Bachorik skrev 2014-10-20 13:12:

Please, review the following test change

Issue : https://bugs.openjdk.java.net/browse/JDK-8058506
Webrev: http://cr.openjdk.java.net/~jbachorik/8058506/webrev.00

The test fails intermittently due to the log printing blocking the 
test thread from time to time, resulting in incorrect data reported by 
ThreadMXBean.


The solution is to use per-thread non-blocking StringBuilders (wrapped 
in Formatter instances) and aggregate the log output only after the 
test is finished.


Thanks,

-JB-

Re: RFR (XXS): JDK-8059105 : Add better failure reporting to com/sun/jdi/RunToExit.java test

[Erik Gahlin]

Looks good.

/Erik

Fredrik Arvidsson skrev 2014-09-25 14:24:

Hi

Please help me review this tiny change to make a test produce better 
information when it fails. This fix is about adding some 
instrumentation to be able to understand an intermittent failure in a 
test.
The test fails and this bug: 
https://bugs.openjdk.java.net/browse/JDK-8058734 is probably a 
duplicate of another bug: 
https://bugs.openjdk.java.net/browse/JDK-6573254, but to be sure we 
need more data when test fails.



Webrev: http://cr.openjdk.java.net/~farvidsson/8059105/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8059105

Cheers
/Fredrik

Re: RFR: JDK-8055845 - Add trace event for promoted objects

[Erik Gahlin]

Metadata looks good (trace.xml).

/E

Staffan Friberg skrev 2014-09-06 00:20:

Hi,

I have uploaded a new webrev here, 
cr.openjdk.java.net/~sfriberg/8055845/webrev.03


It contains several changes

- Split event into two events (PromoteObjectInNewPLAB, 
PromoteObjectOutsidePLAB)

- Moved events to vm/gc/detailed/PromoteObject...
- Supporting ParNew+CMS and ParNew+SerialOld tenuring
 - Not sure if the way I do it with passing the ParNewTracer 
is the best solution, please let me know if you have an idea how to 
improve it
- Simplified the G1 code to avoid sending age and having a single 
call site
- Fixed so that the generated event has size information in bytes 
rather than words


Thanks for offline comments and suggestions from Dmitry and Thomas.

Cheers,
Staffan

On 08/29/2014 03:32 PM, Staffan Friberg wrote:

Hi Erik,

On 08/28/2014 11:34 PM, Erik Helin wrote:
(it seems like we lost hotspot-gc-...@openjdk.java.net somewhere in 
this thread, I'm adding it back)


On 2014-08-28 23:15, Staffan Friberg wrote:

Hi Erik,

Thanks for the comments.
- Aren't the events for promotion to the tenured generation 
(SerialOld)

  and the CMS generation missing?

The reason for leaving out these two, Serial completely and CMS
promotion, was due to that neither as far as I understand make use of
PLABs.


I might be wrong here, but looking at the function 
TenuredGeneration::par_promote (in tenuredGeneration.cpp) it looks 
to me like SerialOld is using PLABs when ParNew is promoting objects 
from young to old.


As for CMS, looking at ConcurrentMarkSweepGeneration::par_promote 
(in concurrentMarkSweepGeneration.cpp) it seems like each 
CMSParGCThreadState has a CFLS_LAB (CompactibleFreeListSpace Local 
Allocation Buffer) that is a thread-local allocation buffer. See 
compactibleFreeListSpace.{hpp,cpp} for more details.


Given this, I think we should add events for Serial and CMS as well.



Ok I see what you mean with CMS, basically the equivalent to getting 
a PLAB would be when we refill the CFLS_LAB in the alloc function. It 
still does the allocation to a small memory (~ size of object) area 
from the freelist, but at least we did extra work to refill the local 
CFLS_LAB. Need to do some analysis to see how often we refill so the 
overhead doesn't get too high.
The only issue I run into is how I can in a nice way get access to 
the ParNewTracer from ParNewGeneration to call the report function. 
Let's sync up next week and see how it can be solved.


The tenured GC requires something similar to be supported, however 
since ParNewGC is deprecated for usage without CMS in JDK 8 we might 
want to skip that combination.





On 2014-08-28 23:15, Staffan Friberg wrote:
- Would it make sense to differentiate, either by separate events 
or by

  a field in a event, between promotions to to-space and to the old
  generation?
- The are two events for TLAB allocations,
  java/object_alloc_in_new_TLAB and java/object_alloc_outside_TLAB.
  What do you think about using two events for PLAB allocations as 
well:

  - java/object_alloc_in_new_PLAB
  - java/object_alloc_outside_PLAB

I think this is a matter of taste and probably how similar we want the
event to be to the existing allocation event. I personally prefer a
single event but if the GC team and serviceability team feel it 
would be
better to have two I can certainly rewrite. The reason for me 
preferring
a single event is just ease of analysis, you can easily filter a 
list of

events on a field, it is harder to merge two different events with
different fields and work with them in an straight forward fashion.

Any general preference for having a single or multiple events?


I would prefer to have two events in this case and try to follow the 
existing allocation events as much as possible (both in naming and 
in style). Keeping the tenured field (I missed it the first time I 
read the patch) is good.


Yes, tenured would be independent of having two events, only PLAB 
size and directAllocation would be affected when having two event types.


*Erik Gahlin*, any preference from your end?




On 2014-08-28 23:15, Staffan Friberg wrote:

- In PSPromotionManager, instead of utilizing the C++ friendship with
  PSScavenge, should we add a getter function for the gc_tracer?

Created a getter function.


Thanks! If you make report_promotion_sample const, then the getter 
can return a const ParallelScavengeTracer*, right?



Done

//Staffan

Re: RFR: JDK-8055845 - Add trace event for promoted objects

[Erik Gahlin]

Looks good

Bengt Rutisson skrev 2014-08-26 10:50:


Hi all,

Staffan sent me an updated webrev based on Erik's comments. I uploaded 
it here:

http://cr.openjdk.java.net/~brutisso/8055845/webrev.01/

Thanks,
Bengt


On 2014-08-25 19:32, Staffan Friberg wrote:

Hi Erik,

No issue with naming the field class, since the event is similar to 
the Allocation event I used that as a starting point and it also uses 
class as name together with a couple of other events.


Will go through the descriptions and remove periods.

Object age is basically the how many times an object has been kept in 
survivor region, it is incremented each time a YC happens and the 
object is aged.
Will see how I can update the description to make it clearer. Calling 
the field tenuringAge might help?


From the documentation, 
http://docs.oracle.com/javase/8/docs/technotes/tools/unix/java.html


-XX:MaxTenuringThreshold=/threshold/

Sets the maximum tenuring threshold for use in adaptive GC
sizing. The largest value is 15. The default value is 15 for the
parallel (throughput) collector, and 6 for the CMS collector.

The following example shows how to set the maximum tenuring
threshold to 10:

-XX:MaxTenuringThreshold=10


-XX:+PrintTenuringDistribution

Enables printing of tenuring age information. The following is
an example of the output:

Desired survivor size 48286924 bytes, new threshold 10 (max 10)
- age 1: 28992024 bytes, 28992024 total
- age 2: 1366864 bytes, 3035 total
- age 3: 1425912 bytes, 31784800 total
...

Age 1 objects are the youngest survivors (they were created
after the previous scavenge, survived the latest scavenge, and
moved from eden to survivor space). Age 2 objects have survived
two scavenges (during the second scavenge they were copied from
one survivor space to the next). And so on.

In the preceding example, 28 992 024 bytes survived one scavenge
and were copied from eden to survivor space, 1 366 864 bytes are
occupied by age 2 objects, etc. The third value in each row is
the cumulative size of objects of age n or less.

By default, this option is disabled.



Thanks for the comments,
Staffan

On 08/25/2014 09:55 AM, Erik Gahlin wrote:
Did you manage to call the field class? It's a reserved word in 
C++, so we had to use klass in the past


Descriptions with only one sentence shouldn't end with .

How is Object Age measured?

As a user, I would expect the number to be in seconds/minutes/hours, 
but that is not the case. Perhaps an explanation in the description 
and/or a field name that better reflects what we really mean with age.


Otherwise trace.xml looks good!

Erik

Staffan Friberg skrev 25/08/14 18:28:

Hi,

Could I please get reviews for this RFE that adds a trace event for 
aging and promotion for young collections in G1, CMS and Parallel GC.
It works similarly to allocation events, and generates the event on 
the slow path when either a direct copy occurs or a new PLAB is 
request.


Since I'm adding an event I cc:ed the serviceability list in case 
anyone has any comments on the event and changes to trace.xml.


RFE: https://bugs.openjdk.java.net/browse/JDK-8055845
Webrev: http://cr.openjdk.java.net/~brutisso/8055845/webrev.00

Bengt has been kind and agreed to both sponsor and host the the 
webrev.


Thanks,
Staffan

Re: RFR: JDK-8055845 - Add trace event for promoted objects

[Erik Gahlin]

Did you manage to call the field class? It's a reserved word in C++, 
so we had to use klass in the past


Descriptions with only one sentence shouldn't end with .

How is Object Age measured?

As a user, I would expect the number to be in seconds/minutes/hours, but 
that is not the case. Perhaps an explanation in the description and/or a 
field name that better reflects what we really mean with age.


Otherwise trace.xml looks good!

Erik

Staffan Friberg skrev 25/08/14 18:28:

Hi,

Could I please get reviews for this RFE that adds a trace event for 
aging and promotion for young collections in G1, CMS and Parallel GC.
It works similarly to allocation events, and generates the event on 
the slow path when either a direct copy occurs or a new PLAB is request.


Since I'm adding an event I cc:ed the serviceability list in case 
anyone has any comments on the event and changes to trace.xml.


RFE: https://bugs.openjdk.java.net/browse/JDK-8055845
Webrev: http://cr.openjdk.java.net/~brutisso/8055845/webrev.00

Bengt has been kind and agreed to both sponsor and host the the webrev.

Thanks,
Staffan

Re: RFR (XXS): 8055662: Update mapfile for libjfr

[Erik Gahlin]

Looks good

Markus Grönlund skrev 20/08/14 12:27:


Greetings,

Kindly asking for review for this very small change.

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

Webrev: http://cr.openjdk.java.net/~mgronlun/8055662/webrev01/ 
http://cr.openjdk.java.net/%7Emgronlun/8055662/webrev01/


Thanks

Markus

Re: 8046282: SA update seems to break the SA/jstack in OpenJDK

[Erik Gahlin]

Hi ,

I can confirm the macro evaluates to _trace_id in Oracles closed code :(

Sorry about that. I have filed a P2 bug to have it fixed. See.
https://bugs.openjdk.java.net/browse/JDK-8049881

Thanks for reporting
Erik

Volker Simonis skrev 2014-07-10 12:30:

Hi,

the change 8046282: SA update introduced the following new code in
agent/src/share/classes/sun/jvm/hotspot/oops/Klass.java

+traceIDField  = type.getField(_trace_id);

But I can not find the corresponding field in
src/share/vm/oops/klass.hpp. The Klass class only contains the macro
TRACE_DEFINE_KLASS_TRACE_ID which is defined to typedef int
___IGNORED_hs_trace_type2 in traceMacros.hpp.

This leads to an error when calling for example jstack on a core file:

$ images/j2sdk-image/bin/jstack ./images/j2sdk-image/bin/java core
Attaching to core core from executable ./images/j2sdk-image/bin/java,
please wait...
Debugger attached successfully.
Server compiler detected.
JVM version is 1.9.0-internal-d046063_2014_07_10_11_16-b00
Deadlock Detection:

Exception in thread main java.lang.reflect.InvocationTargetException
 at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
 at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
 at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
 at java.lang.reflect.Method.invoke(Method.java:484)
 at sun.tools.jstack.JStack.runJStackTool(JStack.java:140)
 at sun.tools.jstack.JStack.main(JStack.java:106)
Caused by: java.lang.ExceptionInInitializerError
 at sun.jvm.hotspot.oops.ObjectHeap.initialize(ObjectHeap.java:74)
 at sun.jvm.hotspot.oops.ObjectHeap.init(ObjectHeap.java:110)
 at sun.jvm.hotspot.runtime.VM.getObjectHeap(VM.java:582)
 at sun.jvm.hotspot.runtime.DeadlockDetector.print(DeadlockDetector.java:55)
 at sun.jvm.hotspot.runtime.DeadlockDetector.print(DeadlockDetector.java:39)
 at sun.jvm.hotspot.tools.StackTrace.run(StackTrace.java:62)
 at sun.jvm.hotspot.tools.StackTrace.run(StackTrace.java:45)
 at sun.jvm.hotspot.tools.JStack.run(JStack.java:66)
 at sun.jvm.hotspot.tools.Tool.startInternal(Tool.java:260)
 at sun.jvm.hotspot.tools.Tool.start(Tool.java:223)
 at sun.jvm.hotspot.tools.Tool.execute(Tool.java:118)
 at sun.jvm.hotspot.tools.JStack.main(JStack.java:92)
 ... 6 more
Caused by: java.lang.RuntimeException: field _trace_id not found in type Klass
 at sun.jvm.hotspot.types.basic.BasicType.getField(BasicType.java:183)
 at sun.jvm.hotspot.types.basic.BasicType.getField(BasicType.java:190)
 at sun.jvm.hotspot.types.basic.BasicType.getField(BasicType.java:194)
 at sun.jvm.hotspot.oops.Klass.initialize(Klass.java:58)
 at sun.jvm.hotspot.oops.Klass.access$000(Klass.java:33)
 at sun.jvm.hotspot.oops.Klass$1.update(Klass.java:37)
 at sun.jvm.hotspot.runtime.VM.registerVMInitializedObserver(VM.java:394)
 at sun.jvm.hotspot.oops.Klass.clinit(Klass.java:35)
 ... 18 more

Is this a problem with OpenJDK only (i.e. is '_trace_id' defined in
Oracle proprietary builds) ?. Or is this another problem?

Thank you and best regards,
Volker

Re: RFR: 8042778: Getting all visible methods in ReferenceTypeImpl is slow

[Erik Gahlin]

Looks good!

Not a Reviewer, but I can sponsor it.

Erik

Jeremy Manson skrev 08/05/14 20:56:
I'm testing out my newly acquired OpenJDK authorship with something 
simple.  If I did this wrong, I apologize.


Basically, the debugger becomes very slow if there are a lot of 
methods in a class.  This can happen (O(thousands)) if you are using a 
code generation tool.


http://cr.openjdk.java.net/~jmanson/8042778/webrev.00/ 
http://cr.openjdk.java.net/%7Ejmanson/8042778/webrev.00/


Reviews from reviewers and committing from committers would be 
appreciated.  Thanks!


Jeremy

Re: RFR(S): 8049340: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.java timed out

[Erik Gahlin]

Thanks, will remove unused import before I push. Still need a (R)eviewer.

Or I will have to wait until next Tuesday, when you hopefully will 
become one ;)


Erik

Jaroslav Bachorik skrev 07/07/14 13:19:

On 07/07/2014 12:28 PM, Erik Gahlin wrote:

Hi,

I'm generally in favor of looping forever, but for this test it doesn't
work since the test spawns child processes. Here is an updated version
that forwards the system property.

http://cr.openjdk.java.net/~egahlin/8049340_3/

I avoided Utils.TIMEOUT_FACTOR, since it would mean passing the
testlibrary on the class path, making the test more complex.


Ok. I see. No problem.
L39 import jdk.testlibrary.Utils; - unused import

-JB-



Thanks
Erik

Jaroslav Bachorik skrev 2014-07-07 10:23:

Hi Erik,

I have a comment regarding the test timeout - you should scale the
timeout according to test.timeout.factor system property set by
JTReg (you can use jdk.testlibrary.Utils.TIMEOUT_FACTOR to access it
easily) to prevent intermittent timeouts on slow configurations. Or
you could completely omit the timeout and loop forever, relying on the
harness to timeout the test.

Other than that the change looks fine.

Cheers,

-JB-

On 07/06/2014 06:53 PM, Erik Gahlin wrote:

Hi,

Could I have a review of a small test fix.

Webrev:
http://cr.openjdk.java.net/~egahlin/8049340_1/
Bug:
https://bugs.openjdk.java.net/browse/JDK-8049340

Testing:
100 iterations locally
25 iterations locally under high load (CPU  95%)
5 JPRT runs

L226-229:
If the file is removed by the test process before Files.exist 
statement

is reached in the child process
an Error is thrown. The test will pass without this change, but it 
makes

log easier to read.

L244:
Use println instead of print.

L292-299:
The actual problem, see comment in file.

L312:
Typo, should be stderr instead of stder

Thanks
Erik

Re: RFR(S): 8049340: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.java timed out

[Erik Gahlin]

Hi,

I'm generally in favor of looping forever, but for this test it doesn't 
work since the test spawns child processes. Here is an updated version 
that forwards the system property.


http://cr.openjdk.java.net/~egahlin/8049340_3/

I avoided Utils.TIMEOUT_FACTOR, since it would mean passing the 
testlibrary on the class path, making the test more complex.


Thanks
Erik

Jaroslav Bachorik skrev 2014-07-07 10:23:

Hi Erik,

I have a comment regarding the test timeout - you should scale the 
timeout according to test.timeout.factor system property set by 
JTReg (you can use jdk.testlibrary.Utils.TIMEOUT_FACTOR to access it 
easily) to prevent intermittent timeouts on slow configurations. Or 
you could completely omit the timeout and loop forever, relying on the 
harness to timeout the test.


Other than that the change looks fine.

Cheers,

-JB-

On 07/06/2014 06:53 PM, Erik Gahlin wrote:

Hi,

Could I have a review of a small test fix.

Webrev:
http://cr.openjdk.java.net/~egahlin/8049340_1/
Bug:
https://bugs.openjdk.java.net/browse/JDK-8049340

Testing:
100 iterations locally
25 iterations locally under high load (CPU  95%)
5 JPRT runs

L226-229:
If the file is removed by the test process before Files.exist statement
is reached in the child process
an Error is thrown. The test will pass without this change, but it makes
log easier to read.

L244:
Use println instead of print.

L292-299:
The actual problem, see comment in file.

L312:
Typo, should be stderr instead of stder

Thanks
Erik

Re: RFR 8049194: com/sun/tools/attach/StartManagementAgent.java start failing after JDK-8048193

[Erik Gahlin]

Looks good

Erik

Jaroslav Bachorik skrev 2014-07-07 12:15:

Please, review the following test change

Issue : https://bugs.openjdk.java.net/browse/JDK-8049194
Webrev: http://cr.openjdk.java.net/~jbachorik/8049194/webrev.00

jdk.testlibrary.ProcessThread was erroneously starting the external 
application with timeout of -1 - meaning no waiting for the target 
application to initialize. What it should have done was to wait for 
the target application indefinitely and let the harness timeout the test.


The change modifies jdk.testlibrary.ProcessTools.startProcess() 
methods to be explicit about the meaning of -1 and 0 (newly added) 
timeouts. It also adds a convenient method where you can start a 
process waiting for the warmup indefinitely without actually providing 
0 and a dummy TimeUnit.


I also took liberty and encapsulated the test.timeout.factor system 
property handling into jdk.testlibrary.Utils.adjustTimeou(to) method.


The changes to the test library keep the current semantics (eg. the 
timeout of -1 still means no wait etc.) - they are only enhancing it.


Thanks,

-JB-

RFR(S): 8049340: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.java timed out

[Erik Gahlin]

Hi,

Could I have a review of a small test fix.

Webrev:
http://cr.openjdk.java.net/~egahlin/8049340_1/
Bug:
https://bugs.openjdk.java.net/browse/JDK-8049340

Testing:
100 iterations locally
25 iterations locally under high load (CPU  95%)
5 JPRT runs

L226-229:
If the file is removed by the test process before Files.exist statement 
is reached in the child process
an Error is thrown. The test will pass without this change, but it makes 
log easier to read.


L244:
Use println instead of print.

L292-299:
The actual problem, see comment in file.

L312:
Typo, should be stderr instead of stder

Thanks
Erik

Re: RFR(M): 8028474: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.sh timeout, leaves looping process behind

[Erik Gahlin]

Hi Roger,

The test has been updated. It now uses System.nanoTime.

http://cr.openjdk.java.net/~egahlin/8028474_6/

Thanks
Erik

roger riggs skrev 2014-07-01 14:35:

Hi Erik,

Consider switching to System.nanoTime;  it is not sensitive to clock 
changes
and avoids leaving a land mine that may cause a spurious 
non-repeatable test failure.
'Deducing it from the log' means there is a failure and creates 
probably an hour or two of work
for some quality engineer and burns a couple of hours re-running the 
test run.


Roger



On 7/1/2014 3:37 AM, Erik Gahlin wrote:



JavaProcess.waitForRemoval: How about using timestamps 
(currentTimeMillis()) before the loop and for each ite

ration to determine if the timeout has expired (instead of time+=100”)?

The code now uses currentTimeMillis(). Premature timeouts due to 
clock changes can be deduced from the log.

Re: RFR(M): 8028474: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.sh timeout, leaves looping process behind

[Erik Gahlin]

Updated webrev:
http://cr.openjdk.java.net/~egahlin/8028474_5/

See comments inline.

Staffan Larsen skrev 2014-06-26 13:22:

Indentation around JavaProcess.getId() is weird.


Fixed

JavaProcess.getPid/setPid/pid do not appear to be used.


Fixed

JavaProcess.waitForRemoval: How about using timestamps 
(currentTimeMillis()) before the loop and for each ite

ration to determine if the timeout has expired (instead of time+=100”)?

The code now uses currentTimeMillis(). Premature timeouts due to clock 
changes can be deduced from the log.


I also cleaned up the log for releaseStarted/releaseTerminated


nit: missing empty line before line 139, method releaseStarted().


Fixed

Thanks
Erik


/Staffan


On 26 jun 2014, at 00:56, Erik Gahlin erik.gah...@oracle.com 
mailto:erik.gah...@oracle.com wrote:



It didn't work.

There's no termination notification, if I use 
Process#destroyForcibly(). I believe the hsperfdata file is left 
behind so it will not be able to detect that the process has died.


Here is an updated version that renames some variable/methods.

http://cr.openjdk.java.net/~egahlin/8028474_3/

Thanks
Erik

Erik Gahlin skrev 2014-06-18 09:37:

Didn't know about destroyForcibly(). I could try that.

Erik

Staffan Larsen skrev 18/06/14 09:26:

Erik,

How about using Process.destroyForcibly() as a means to terminate 
the process instead of using files for signaling?


/Staffan

On 17 jun 2014, at 23:13, Erik Gahlin erik.gah...@oracle.com 
mailto:erik.gah...@oracle.com wrote:



Yes, very weird

Could have been that I needed the tools.jar in the child 
processes, or it could have been something else. If I remember 
correctly, I got a CNF that I didn't get with the shell script, 
but it was few months back.


Anyway, I retried with JPRT and now it works without the shell script.

http://cr.openjdk.java.net/~egahlin/8028474_2/

Erik

Staffan Larsen skrev 2014-06-16 13:49:


On 16 jun 2014, at 12:32, Erik Gahlin erik.gah...@oracle.com 
mailto:erik.gah...@oracle.com wrote:



Yekaterina Kantserova skrev 13/06/14 12:59:

Erik,

is there some reason why we need to keep 
MonitorVmStartTerminate.sh? I've moved the JTreg header to 
MonitorVmStartTerminate.java

Hi Katja,

That's how I did the test initially, and it works locally, but I 
could never get it to work in JPRT without the shell script. I 
believe the tools.jar is not on the class path.


That is weird. I see other tests that depend in tools.jar and 
they work fine.


/Staffan




Erik


/*
 * @test
 * @bug 4990825
 * @summary attach to external but local JVM processes
 * @library /lib/testlibrary
 * @build jdk.testlibrary.*
 * @run main MonitorVmStartTerminate
 */

and the test works just fine.

The JTreg run contains all pathes and system properties 
MonitorVmStartTerminate.sh tries to construct:
${JAVA} ${TESTVMOPTS} -Dtest.jdk=${TESTJAVA} 
-Dtest.classes=${TESTCLASSES} -classpath ${CP} 
MonitorVmStartTerminate


See the log attached.

Note *@build jdk.testlibrary.** instead of *@build 
jdk.testlibrary.ProcessTools* to make sure all testlibrary 
classes are compiled

to the right place when running tests concurrently.

Thanks,
Katja (Not a Reviewer)



On 06/12/2014 12:37 AM, Erik Gahlin wrote:

Hi,

Could I have a review of a test that has been failing
intermittently. The test now uses files for synchronization
instead of waiting for a process that sleeps.

Webrev:
http://cr.openjdk.java.net/~egahlin/8028474/

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

Description:

The test starts ten Java processes, each with a unique id.

 Each process creates a file named after the id and then it 
waits for

 the test to remove the file, at which the Java process exits.

 The processes are monitored by the test to make sure 
notifications

 are sent when processes are started/terminated.

 To avoid Java processes being left behind, in case of an 
unexpected
 failure, shutdown hooks are registered that remove files when 
the test

 exits. If files are not removed, i.e. due to a JVM crash,
 the Java processes will exit themselves after 1000 s.

Thanks
Erik