Re: RFR JDK-8198253: ThreadInfo.from(CompositeData) assigning fields incorrectly in JDK 9

[mandy chung]

Webrev at:
  http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198253/webrev.01

This patch updates the spec to clarify what attributes are required
since which release.  There is a spec bug that "classLoaderName"
added in JDK 9 is missing in the CompositeData for StackTraceElement
but the implementation is correct.  I will file a CSR for this update.

This patch ensures that CompositeData for ThreadInfo of version N
must have the attributes defined since <= N.
ThreadInfo::from also makes sure 'stackTrace' and 'lockedMonitors'
attributes of version N while it can include additional attributes
which has been the current behavior.

JDK-8139587 intended to support older versions of StackTraceElement
which does not seem a complete solution.  I reverted the fix for
JDK-8139587 (mostly) and removed TypeVersionMapper.  The fix constructs
the CompositeType for ThreadInfo and MonitorInfo of different
versions and used them for validation.  Minor cleanup: the static
final variables are renamed to all capitals.  CompatibilityTest.java
test is missing the copyright header.

thanks
Mandy
[1] 
http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8198253/api/java/lang/management/ThreadInfo.html#from(javax.management.openmbean.CompositeData)

Re: Thread Native ID Access

[Hohensee, Paul]

Here’s a concrete proposal on which people may want to comment.

We could add a method to ThreadInfo, vis.,

long getPlatformThreadId()

Returns the platform-specific thread ID of the thread associated with this 
ThreadInfo. The ID has no meaning to the Java runtime and is not guaranteed to 
have any meaning at all.
I.e., it may be a constant value for all threads, or a native address, and may 
not be the same across multiple invocations.

Returns:

the platform-specific ID of the associated thread.

For a fiber, a reasonable value would be the OS thread id for the OS thread 
that’s running the fiber at ThreadInfo construction time. If, say, the platform 
used a string or some other larger-than-long value, it could be a native 
address or some other value that mapped to one.

If we don’t want to add a new method to ThreadInfo, we could define a 
com.sun.management.ThreadInfo to go with com.sun.management.ThreadMXBean.

Parenthetically, to me a fiber is enough different from a thread to warrant a 
separate MXBean. I.e., a thread is a species of fiber, but not vica versa. So, 
we shouldn’t gate changes to ThreadMXBean on having a thorough understanding of 
how to support fibers in JMX.

We could also add to com.sun.management.ThreadMXBean the methods 
getThreadId(long id), getThreadId(long [] ids), getPlatformThreadId(long id), 
and getPlatformThreadId(long [] ids). These presumably would be fast compared 
to creating ThreadInfo(s).

We don’t want to add Thread.getPlatformId(), so the way to get the current 
thread’s platform id would be

getThreadInfo( Thread.currentThread().getId() ).getPlatformThreadId()

which is cumbersome and slow. Instead, by analogy to 
ThreadMXBean.getCurrentThreadCpuTime(), we could add to 
com.sun.management.ThreadMXBean the methods getCurrentThreadId() and 
getCurrentPlatformThreadId().

All this may seem somewhat overkill, but it’s no more so than how we currently 
create ThreadInfo objects and the various ThreadCpu/UserTime method definitions.

Thanks,

Paul

From: serviceability-dev  on 
behalf of Jeremy Manson 
Date: Thursday, February 22, 2018 at 9:42 PM
To: David Holmes 
Cc: "serviceability-dev@openjdk.java.net" 
, "loom-...@openjdk.java.net" 
, John Rose 
Subject: Re: Thread Native ID Access

I think we're all pretty much agreed that fibers imply native tids can change.  
I think that, as long as we document that, we're fine.

An API where there is probably a worse implementation problem is 
ThreadMXBean#getThreadCpuTime.
  Right now, the implementation assumes a 1:1 mapping between OS thread and 
Java thread.  Removing that assumption implies you have to do something like 
track cpu time every time the scheduler switches you out, which is going to 
make your switches expensive.

Jeremy

On Thu, Feb 22, 2018 at 9:16 PM, David Holmes 
> wrote:
On 23/02/2018 3:04 PM, John Rose wrote:
On Feb 22, 2018, at 8:49 PM, David Holmes 
 
>> wrote:

I don't think this request has any impact on Fibers at all. At any given time a 
piece of executing Java code is executing on a current Thread, and that current 
Thread must be running on a native thread (regardless of mapping) and the 
native thread has an id. The only use for exposing such an id is to allow you 
to correlate information obtained from inside the VM, with information observed 
outside, possibly by external tools.

We may or may not bind fiber identity to the legacy java.lang.Thread type.

I'm not assuming that exposing native thread ids implies anything about the 
binding between those entities and anything higher-level. I have no issue with 
a Fiber/Continuation/Task/whatever running on different java.lang.Threads over 
its lifetime. For that matter I don't think it matters if java.lang.Threads 
could execute on different native threads (they don't but they could).

David
-

This affects the question of whether the following code can return false:

boolean testThreadShift(Runnable r) {
   Thread t0 = Thread.current();
   r.run();
   Thread t1 = Thread.current();
   return t0 == t1;
}

This method can return false if (a) Thread.current() reports the identity
of the OS thread (not the fiber currently running), and (b) the runnable
includes a blocking operation which causes the current fiber to reschedule
on a different OS thread before it returns.

Having this method return false is, perhaps, surprising enough to cause
us to inject fiber identity into java.lang.Thread, so that a java.lang.Thread
identity is 1-1 with a fiber, rather than an OS "dinosaur" thread.  

Re: RFR(XS): 8198585: add asserts to verify that ServiceUtil::visible_oop is not needed

[Chris Plummer]

Ok. I'll make that change.

thanks,

Chris

On 2/23/18 6:45 AM, Daniel D. Daugherty wrote:

Instead of "assert(false, ...", I recommend "fatal(...".
This will cause a failure in all build configs including 'release' bits.

Dan


On 2/22/18 8:16 PM, Chris Plummer wrote:

Hello,

Please review the following:

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

Before removing ServiceUtil::visible_oop(), I want to make sure it 
really isn't needed. Supposedly it should never return false, thus 
negating the need for its existence. This change adds asserts 
whenever false is returned. If it makes it all the way through 
promotion testing, then I'll delete the ServiceUtil::visible_oop() 
code and the references to it.


I tested by running all jdk and hotspot tier1-3 tests.

thanks,

Chris

Re: RFR(XS): 8198585: add asserts to verify that ServiceUtil::visible_oop is not needed

[Daniel D. Daugherty]

Instead of "assert(false, ...", I recommend "fatal(...".
This will cause a failure in all build configs including 'release' bits.

Dan


On 2/22/18 8:16 PM, Chris Plummer wrote:

Hello,

Please review the following:

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

Before removing ServiceUtil::visible_oop(), I want to make sure it 
really isn't needed. Supposedly it should never return false, thus 
negating the need for its existence. This change adds asserts whenever 
false is returned. If it makes it all the way through promotion 
testing, then I'll delete the ServiceUtil::visible_oop() code and the 
references to it.


I tested by running all jdk and hotspot tier1-3 tests.

thanks,

Chris

Re: RFR(S): 8196992: Resolve disabled warnings for libdt_socket

[Magnus Ihse Bursie]

On 2018-02-23 02:03, Chris Plummer wrote:

Hello,

Please review the following:

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

diff --git a/make/lib/Lib-jdk.jdwp.agent.gmk 
b/make/lib/Lib-jdk.jdwp.agent.gmk

--- a/make/lib/Lib-jdk.jdwp.agent.gmk
+++ b/make/lib/Lib-jdk.jdwp.agent.gmk
@@ -43,7 +43,6 @@
 OPTIMIZATION := LOW, \
 CFLAGS := $(CFLAGS_JDKLIB) -DUSE_MMAP \
 $(LIBDT_SOCKET_CPPFLAGS), \
-    DISABLED_WARNINGS_gcc := shift-negative-value, \
 MAPFILE := $(TOPDIR)/make/mapfiles/libdt_socket/mapfile-vers, \
 LDFLAGS := $(LDFLAGS_JDKLIB) \
 $(call SET_SHARED_LIBRARY_ORIGIN), \

Looks good. Thanks for fixing these code quality issues!

/Magnus


This change is undoing the makefile change done as part of 
JDK-8196985. The only warning that was turning up in libdt_socket code 
before JDK-8196985 was done has already been fixed by JDK-8196909. 
Thus no warnings need to be fixed.


After removing the above makefile code, I tested by building with the 
new toolchain. As a first test I undid the socketTransport.cpp fix 
from JDK-8196909 to verify that the new toolchain exposed the warning. 
Then I reverted socketTransport.cpp back to tip sources and saw no 
warnings with the new toolchain.


thanks,

Chris

Re: Thread Native ID Access

[Alan Bateman]

On 23/02/2018 05:04, John Rose wrote:

We may or may not bind fiber identity to the legacy java.lang.Thread type.
This affects the question of whether the following code can return false:

boolean testThreadShift(Runnable r) {
  Thread t0 = Thread.current();
  r.run();
  Thread t1 = Thread.current();
  return t0 == t1;
}
This is an area that I have been spending time recently. A big benefit 
of a Thread object per fiber, and the above always returning true, is 
that things that are thread local today (ThreadLocal, thread context 
class loader, uncaught exception handler, ...) become fiber local 
without too much effort. It means we have Thread objects that aren't 
tied to a kernel thread or don't have thread meta data in the VM (the 
lifecycle of Threads means we do this today anyway). Lots of details of 
course but it's a direction that potentially allows a lot of existing 
code to be executed in a fiber without changes.


In the java.lang.management API, ThreadMXBean is more tied to kernel 
threads due to methods that reveal thread CPU counters. A possible 
direction is to continue with a ThreadMXBean per dinosaur thread, in 
which case Jeremy's proposal to expose the native thread identifier 
might be okay. That direction would require sorting out a details with 
the thread identifier (Thread::getId) as it can be used today to link a 
Thread to a ThreadMXBean but that shouldn't be too hard.


-Alan.

Re: RFR(XS): 8198585: add asserts to verify that ServiceUtil::visible_oop is not needed

[serguei.spit...@oracle.com]

Hi Chris,

+1

Thanks,
Serguei


On 2/23/18 01:58, Stefan Karlsson wrote:

Looks good.

StefanK

On 2018-02-23 02:16, Chris Plummer wrote:

Hello,

Please review the following:

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

Before removing ServiceUtil::visible_oop(), I want to make sure it 
really isn't needed. Supposedly it should never return false, thus 
negating the need for its existence. This change adds asserts 
whenever false is returned. If it makes it all the way through 
promotion testing, then I'll delete the ServiceUtil::visible_oop() 
code and the references to it.


I tested by running all jdk and hotspot tier1-3 tests.

thanks,

Chris

Re: RFR(S): 8196992: Resolve disabled warnings for libdt_socket

[serguei.spit...@oracle.com]

Chris,

This looks good to me too.

Thanks,
Serguei


On 2/23/18 00:48, Langer, Christoph wrote:

Looks good, Chris.


-Original Message-
From: build-dev [mailto:build-dev-boun...@openjdk.java.net] On Behalf Of
Chris Plummer
Sent: Freitag, 23. Februar 2018 02:04
To: build-...@openjdk.java.net build-dev ;
serviceability-dev 
Subject: RFR(S): 8196992: Resolve disabled warnings for libdt_socket

Hello,

Please review the following:

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

diff --git a/make/lib/Lib-jdk.jdwp.agent.gmk
b/make/lib/Lib-jdk.jdwp.agent.gmk
--- a/make/lib/Lib-jdk.jdwp.agent.gmk
+++ b/make/lib/Lib-jdk.jdwp.agent.gmk
@@ -43,7 +43,6 @@
   OPTIMIZATION := LOW, \
   CFLAGS := $(CFLAGS_JDKLIB) -DUSE_MMAP \
   $(LIBDT_SOCKET_CPPFLAGS), \
-    DISABLED_WARNINGS_gcc := shift-negative-value, \
   MAPFILE := $(TOPDIR)/make/mapfiles/libdt_socket/mapfile-vers, \
   LDFLAGS := $(LDFLAGS_JDKLIB) \
   $(call SET_SHARED_LIBRARY_ORIGIN), \

This change is undoing the makefile change done as part of JDK-8196985.
The only warning that was turning up in libdt_socket code before
JDK-8196985 was done has already been fixed by JDK-8196909. Thus no
warnings need to be fixed.

After removing the above makefile code, I tested by building with the
new toolchain. As a first test I undid the socketTransport.cpp fix from
JDK-8196909 to verify that the new toolchain exposed the warning. Then I
reverted socketTransport.cpp back to tip sources and saw no warnings
with the new toolchain.

thanks,

Chris

Re: RFR(XS): 8198585: add asserts to verify that ServiceUtil::visible_oop is not needed

[Stefan Karlsson]

Looks good.

StefanK

On 2018-02-23 02:16, Chris Plummer wrote:

Hello,

Please review the following:

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

Before removing ServiceUtil::visible_oop(), I want to make sure it 
really isn't needed. Supposedly it should never return false, thus 
negating the need for its existence. This change adds asserts whenever 
false is returned. If it makes it all the way through promotion 
testing, then I'll delete the ServiceUtil::visible_oop() code and the 
references to it.


I tested by running all jdk and hotspot tier1-3 tests.

thanks,

Chris

RE: RFR (XS): 8198539: Cleanup of unused imports in java/util/jar/Attributes.java (java.base) and JdpController.java (jdk.management.agent)

[Langer, Christoph]

Thanks, Thomas.

From: Thomas Stüfe [mailto:thomas.stu...@gmail.com]
Sent: Donnerstag, 22. Februar 2018 09:54
To: Langer, Christoph 
Cc: Java Core Libs ; 
serviceability-dev@openjdk.java.net; andrew_m_leon...@uk.ibm.com
Subject: Re: RFR (XS): 8198539: Cleanup of unused imports in 
java/util/jar/Attributes.java (java.base) and JdpController.java 
(jdk.management.agent)

Hi Christoph,

Looks fine.

.. Thomas

On Feb 22, 2018 09:42, "Langer, Christoph" 
> wrote:
Hi,

please review a simple import cleanup fix for java/util/jar/Attributes.java and 
sun/management/jdp/JdpController.java.

Bug: https://bugs.openjdk.java.net/browse/JDK-8198539
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8198539.0/

Thanks and best regards
Christoph

RE: RFR(S): 8196992: Resolve disabled warnings for libdt_socket

[Langer, Christoph]

Looks good, Chris.

> -Original Message-
> From: build-dev [mailto:build-dev-boun...@openjdk.java.net] On Behalf Of
> Chris Plummer
> Sent: Freitag, 23. Februar 2018 02:04
> To: build-...@openjdk.java.net build-dev ;
> serviceability-dev 
> Subject: RFR(S): 8196992: Resolve disabled warnings for libdt_socket
> 
> Hello,
> 
> Please review the following:
> 
> https://bugs.openjdk.java.net/browse/JDK-8196992
> 
> diff --git a/make/lib/Lib-jdk.jdwp.agent.gmk
> b/make/lib/Lib-jdk.jdwp.agent.gmk
> --- a/make/lib/Lib-jdk.jdwp.agent.gmk
> +++ b/make/lib/Lib-jdk.jdwp.agent.gmk
> @@ -43,7 +43,6 @@
>   OPTIMIZATION := LOW, \
>   CFLAGS := $(CFLAGS_JDKLIB) -DUSE_MMAP \
>   $(LIBDT_SOCKET_CPPFLAGS), \
> -    DISABLED_WARNINGS_gcc := shift-negative-value, \
>   MAPFILE := $(TOPDIR)/make/mapfiles/libdt_socket/mapfile-vers, \
>   LDFLAGS := $(LDFLAGS_JDKLIB) \
>   $(call SET_SHARED_LIBRARY_ORIGIN), \
> 
> This change is undoing the makefile change done as part of JDK-8196985.
> The only warning that was turning up in libdt_socket code before
> JDK-8196985 was done has already been fixed by JDK-8196909. Thus no
> warnings need to be fixed.
> 
> After removing the above makefile code, I tested by building with the
> new toolchain. As a first test I undid the socketTransport.cpp fix from
> JDK-8196909 to verify that the new toolchain exposed the warning. Then I
> reverted socketTransport.cpp back to tip sources and saw no warnings
> with the new toolchain.
> 
> thanks,
> 
> Chris