Re: RFR(S) 8212200 assert when shared java.lang.Object is redefined by JVMTI agent

2018-10-21 Thread Ioi Lam

Hi David,

Thanks for the review. Updated webrev:

http://cr.openjdk.java.net/~iklam/jdk12/8212200-cds-jvmti-clfh-critical-classes.v04/
http://cr.openjdk.java.net/~iklam/jdk12/8212200-cds-jvmti-clfh-critical-classes.v04.delta/

More comments below:



On 10/21/18 6:57 PM, David Holmes wrote:

Hi Ioi,

Generally seems okay.

On 22/10/2018 11:15 AM, Ioi Lam wrote:
Re-sending to the correct mailing lists. Please disregard the other 
email.


http://cr.openjdk.java.net/~iklam/jdk12/8212200-cds-jvmti-clfh-critical-classes.v03/ 


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

Hi,

CDS has various built-in assumptions that classes loaded by
SystemDictionary::resolve_well_known_classes must not be replaced
by JVMTI ClassFileLoadHook during run time, including

- field offsets computed in JavaClasses::compute_offsets
- the layout of the strings objects in the shared strings table

The "well-known" classes can be replaced by ClassFileLoadHook only
when JvmtiExport::early_class_hook_env() is true. Therefore, the
fix is to disable CDS under this condition.


I'm a little unclear why we have to iterate JvmtiEnv list when this 
has to be checked during JVMTI_PHASE_PRIMORDIAL?


I think you are asking about this new function? I don't like the name 
"early_class_hook_env()". Maybe I should change it to 
"has_early_class_hook_env()"?



bool JvmtiExport::early_class_hook_env() {
  JvmtiEnvIterator it;
  for (JvmtiEnv* env = it.first(); env != NULL; env = it.next(env)) {
    if (env->early_class_hook_env()) {
  return true;
    }
  }
  return false;
}

This function matches condition in the existing code that would call 
into ClassFileLoadHook:


class JvmtiClassFileLoadHookPoster {
 ...
  void post_all_envs() {
    JvmtiEnvIterator it;
    for (JvmtiEnv* env = it.first(); env != NULL; env = it.next(env)) {
        ..
    post_to_env(env, true);
    }
  }
...
  void post_to_env(JvmtiEnv* env, bool caching_needed) {
    if (env->phase() == JVMTI_PHASE_PRIMORDIAL && 
!env->early_class_hook_env()) {

  return;
    }


post_all_envs() is called just before a class is about to be loaded in 
the JVM. So if *any* env->early_class_hook_env() returns true, there's a 
chance that it will replace a well-known class.So, as a preventive 
measure, CDS will be disabled.





I have added a few test cases to try to replace shared classes,
including well-known classes and other classes. See
comments in ReplaceCriticalClasses.java for details.

As a clean up, I also renamed all use of "preloaded" in
the source code to "well-known". They refer to the same thing
in HotSpot, so there's no need to use 2 terms. Also, The word
"preloaded" is ambiguous -- it's unclear when "preloading" happens,
and could be confused with what CDS does during archive dump time.


A few specific comments:

src/hotspot/share/classfile/systemDictionary.cpp

+ bool SystemDictionary::is_well_known_klass(Symbol* class_name) {
+   for (int i = 0; ; i++) {
+ int sid = wk_init_info[i];
+ if (sid == 0) {
+   break;
+ }

I take it a zero value is a guaranteed end-of-list sentinel?



Yes. The array is defined just a few lines above:

static const short wk_init_info[] = {
  #define WK_KLASS_INIT_INFO(name, symbol) \
    ((short)vmSymbols::VM_SYMBOL_ENUM_NAME(symbol)),

  WK_KLASSES_DO(WK_KLASS_INIT_INFO)
  #undef WK_KLASS_INIT_INFO
  0
};

Also,

class vmSymbols: AllStatic {
  enum SID {
    NO_SID = 0,
    




+ for (int i=FIRST_WKID; i

Fixed.


---

test/hotspot/jtreg/runtime/SharedArchiveFile/serviceability/ReplaceCriticalClasses.java 



New file should have current copyright year only.


Fixed.

 31  * @comment CDS should not be disabled -- these critical classes 
will be replaced because JvmtiExport::early_class_hook_env() is true.


Comment seems contradictory: if we replace critical classes then CDS 
should be disabled right??



Fixed.


I expected to see tests that checked for:

"CDS is disabled because early JVMTI ClassFileLoadHook is in use."

in the output. ??



It would have been easy if jtreg lets you check the output of @run 
easily. Instead, your innocent suggestion has turned into 150+ lines of 
new code :-( Maybe "let's write all shell tests in Java" isn't such a 
great idea after all.



Now the test checks that whether CDS is indeed disabled, whether the 
affected class is loaded from the shared archive, etc.


Thanks
- Ioi


Thanks,
David




 > In early e-mails Jiangli wrote:
 >
 > We should consider including more classes from the default classlist
 > in the test. Archived classes loaded during both 'early' phase and 
after

 > should be tested.

Done.


 > For future optimizations, we might want to prevent loading additional
 > shared classes if any of the archived system classes is changed.

What's the benefit of doing this? Today we already stop loading a shared
class if its super class was not loaded from the archive.


Thanks
- Ioi





Re: RFR: (S): JDK-8200613: SA: jstack throws UnmappedAddressException with a CDS core file

2018-10-21 Thread Ioi Lam

Hi Jini,

Did you see my earlier reply? I might have sent it out during the mail 
server outage days :-(


http://openjdk.5641.n7.nabble.com/RFR-S-JDK-8200613-SA-jstack-throws-UnmappedAddressException-with-a-CDS-core-file-td352681.html#a353026

Here was my reply again:


Hi Jini,

The changes looks good to me.

Have you tested this on MacOS? CDS heap support is also enabled on
MacOS. See macros.hpp:

 #if INCLUDE_CDS && INCLUDE_G1GC && defined(_LP64) && 
!defined(_WINDOWS)

 #define INCLUDE_CDS_JAVA_HEAP 1

Also, besides CDS, do we know how often other files will be mmaped with
MAP_PRIVATE? Will users get huge core files because CDS is enabled? In
JDK 12, CDS will be enabled by default (see JDK-8202951), so all users
will be affected by the following line:

   if (UseSharedSpaces) {
 set_coredump_filter(FILE_BACKED_PVT_BIT);
   }

Maybe you can run an big app such as Eclipse, trigger a core dump, and
compare the size of the core file before/after this change?

Thanks

- Ioi 







Thanks

- Ioi


On 10/21/18 8:58 PM, Jini George wrote:

Gentle reminder!

Thanks,
- Jini

On 10/9/2018 11:31 AM, Jini George wrote:

Hello!

[Including runtime-dev since the changes are in runtime code]

Requesting reviews for:

Webrev: http://cr.openjdk.java.net/~jgeorge/8200613/webrev.01/
BugID: https://bugs.openjdk.java.net/browse/JDK-8200613

Issue: jhsdb jstack would throw an UnmappedAddressException with a 
core file generated from a CDS enabled java process. This is seen 
only with Linux and with G1GC, while trying to read in data from the 
shared strings region (the closed archive heap space). This region 
(which is a file backed private memory region) is not dumped into the 
corefile for Linux. This, being a heap region (and therefore being a 
read-write region) is also not read in from the classes.jsa file in 
SA since only the read only regions are read in while processing the 
core file. (The expectation being that the read write regions are in 
the core file).


Proposed solution: The proposed solution is to have the 
coredump_filter value corresponding to the CDS process to include bit 
2 (file-backed private memory), so that the file-backed private 
memory region also gets dumped into the corefile. The proposed fix is 
in src/hotspot/os/linux/os_linux.cpp.


Thanks,
Jini.





Re: RFR: (S): JDK-8200613: SA: jstack throws UnmappedAddressException with a CDS core file

2018-10-21 Thread Jini George

Gentle reminder!

Thanks,
- Jini

On 10/9/2018 11:31 AM, Jini George wrote:

Hello!

[Including runtime-dev since the changes are in runtime code]

Requesting reviews for:

Webrev: http://cr.openjdk.java.net/~jgeorge/8200613/webrev.01/
BugID: https://bugs.openjdk.java.net/browse/JDK-8200613

Issue: jhsdb jstack would throw an UnmappedAddressException with a core 
file generated from a CDS enabled java process. This is seen only with 
Linux and with G1GC, while trying to read in data from the shared 
strings region (the closed archive heap space). This region (which is a 
file backed private memory region) is not dumped into the corefile for 
Linux. This, being a heap region (and therefore being a read-write 
region) is also not read in from the classes.jsa file in SA since only 
the read only regions are read in while processing the core file. (The 
expectation being that the read write regions are in the core file).


Proposed solution: The proposed solution is to have the coredump_filter 
value corresponding to the CDS process to include bit 2 (file-backed 
private memory), so that the file-backed private memory region also gets 
dumped into the corefile. The proposed fix is in 
src/hotspot/os/linux/os_linux.cpp.


Thanks,
Jini.



Re: RFR(S) 8212200 assert when shared java.lang.Object is redefined by JVMTI agent

2018-10-21 Thread David Holmes

Hi Ioi,

Generally seems okay.

On 22/10/2018 11:15 AM, Ioi Lam wrote:

Re-sending to the correct mailing lists. Please disregard the other email.

http://cr.openjdk.java.net/~iklam/jdk12/8212200-cds-jvmti-clfh-critical-classes.v03/ 


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

Hi,

CDS has various built-in assumptions that classes loaded by
SystemDictionary::resolve_well_known_classes must not be replaced
by JVMTI ClassFileLoadHook during run time, including

- field offsets computed in JavaClasses::compute_offsets
- the layout of the strings objects in the shared strings table

The "well-known" classes can be replaced by ClassFileLoadHook only
when JvmtiExport::early_class_hook_env() is true. Therefore, the
fix is to disable CDS under this condition.


I'm a little unclear why we have to iterate JvmtiEnv list when this has 
to be checked during JVMTI_PHASE_PRIMORDIAL?



I have added a few test cases to try to replace shared classes,
including well-known classes and other classes. See
comments in ReplaceCriticalClasses.java for details.

As a clean up, I also renamed all use of "preloaded" in
the source code to "well-known". They refer to the same thing
in HotSpot, so there's no need to use 2 terms. Also, The word
"preloaded" is ambiguous -- it's unclear when "preloading" happens,
and could be confused with what CDS does during archive dump time.


A few specific comments:

src/hotspot/share/classfile/systemDictionary.cpp

+ bool SystemDictionary::is_well_known_klass(Symbol* class_name) {
+   for (int i = 0; ; i++) {
+ int sid = wk_init_info[i];
+ if (sid == 0) {
+   break;
+ }

I take it a zero value is a guaranteed end-of-list sentinel?

+ for (int i=FIRST_WKID; i 31  * @comment CDS should not be disabled -- these critical classes 
will be replaced because JvmtiExport::early_class_hook_env() is true.


Comment seems contradictory: if we replace critical classes then CDS 
should be disabled right??


I expected to see tests that checked for:

"CDS is disabled because early JVMTI ClassFileLoadHook is in use."

in the output. ??

Thanks,
David




 > In early e-mails Jiangli wrote:
 >
 > We should consider including more classes from the default classlist
 > in the test. Archived classes loaded during both 'early' phase and after
 > should be tested.

Done.


 > For future optimizations, we might want to prevent loading additional
 > shared classes if any of the archived system classes is changed.

What's the benefit of doing this? Today we already stop loading a shared
class if its super class was not loaded from the archive.


Thanks
- Ioi



RFR(S) 8212200 assert when shared java.lang.Object is redefined by JVMTI agent

2018-10-21 Thread Ioi Lam

Re-sending to the correct mailing lists. Please disregard the other email.

http://cr.openjdk.java.net/~iklam/jdk12/8212200-cds-jvmti-clfh-critical-classes.v03/
https://bugs.openjdk.java.net/browse/JDK-8212200

Hi,

CDS has various built-in assumptions that classes loaded by
SystemDictionary::resolve_well_known_classes must not be replaced
by JVMTI ClassFileLoadHook during run time, including

- field offsets computed in JavaClasses::compute_offsets
- the layout of the strings objects in the shared strings table

The "well-known" classes can be replaced by ClassFileLoadHook only
when JvmtiExport::early_class_hook_env() is true. Therefore, the
fix is to disable CDS under this condition.

I have added a few test cases to try to replace shared classes,
including well-known classes and other classes. See
comments in ReplaceCriticalClasses.java for details.

As a clean up, I also renamed all use of "preloaded" in
the source code to "well-known". They refer to the same thing
in HotSpot, so there's no need to use 2 terms. Also, The word
"preloaded" is ambiguous -- it's unclear when "preloading" happens,
and could be confused with what CDS does during archive dump time.


> In early e-mails Jiangli wrote:
>
> We should consider including more classes from the default classlist
> in the test. Archived classes loaded during both 'early' phase and after
> should be tested.

Done.


> For future optimizations, we might want to prevent loading additional
> shared classes if any of the archived system classes is changed.

What's the benefit of doing this? Today we already stop loading a shared
class if its super class was not loaded from the archive.


Thanks
- Ioi



OpenDataException thrown when constructing CompositeData for ThreadInfo

2018-10-21 Thread Sven Reimers
Hi,

one more problem to go

After applying the proposed fix for the
ThreadINfoCompositeData.compositeTyoe() the following exception is raised:

java.lang.reflect.InvocationTargetException
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at
org.netbeans.modules.sampler.SamplesOutputStream.toCompositeData(SamplesOutputStream.java:178)
at
org.netbeans.modules.sampler.SamplesOutputStream.access$400(SamplesOutputStream.java:44)
at
org.netbeans.modules.sampler.SamplesOutputStream$Sample.writeToStream(SamplesOutputStream.java:285)
at
org.netbeans.modules.sampler.SamplesOutputStream$Sample.access$300(SamplesOutputStream.java:253)
at
org.netbeans.modules.sampler.SamplesOutputStream.close(SamplesOutputStream.java:202)
at org.netbeans.modules.sampler.Sampler.stopSampling(Sampler.java:231)
at org.netbeans.modules.sampler.Sampler.stop(Sampler.java:207)
at
org.netbeans.core.ui.sampler.SelfSamplerAction$1.doInBackground(SelfSamplerAction.java:90)
at java.desktop/javax.swing.SwingWorker$1.call(SwingWorker.java:304)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.desktop/javax.swing.SwingWorker.run(SwingWorker.java:343)
at
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:835)
Caused by: java.lang.AssertionError:
javax.management.openmbean.OpenDataException: Argument value of wrong type
for item lockInfo: value false, type
javax.management.openmbean.CompositeType(name=java.lang.management.LockInfo,items=((itemName=className,itemType=javax.management.openmbean.SimpleType(name=java.lang.String)),(itemName=identityHashCode,itemType=javax.management.openmbean.SimpleType(name=java.lang.Integer
at
java.management/sun.management.ThreadInfoCompositeData.getCompositeData(ThreadInfoCompositeData.java:135)
at
java.management/sun.management.ThreadInfoCompositeData.toCompositeData(ThreadInfoCompositeData.java:72)
... 18 more
Caused by: javax.management.openmbean.OpenDataException: Argument value of
wrong type for item lockInfo: value false, type
javax.management.openmbean.CompositeType(name=java.lang.management.LockInfo,items=((itemName=className,itemType=javax.management.openmbean.SimpleType(name=java.lang.String)),(itemName=identityHashCode,itemType=javax.management.openmbean.SimpleType(name=java.lang.Integer
at
java.management/javax.management.openmbean.CompositeDataSupport.(CompositeDataSupport.java:235)
at
java.management/javax.management.openmbean.CompositeDataSupport.(CompositeDataSupport.java:118)
at
java.management/sun.management.ThreadInfoCompositeData.getCompositeData(ThreadInfoCompositeData.java:130)
... 19 more

Seems the sequence of values is broken for ThreadInfoCompoositeData also.

Changing the sequence so that lockInfoData is after isNative fixes the
problem:

   final Object[] threadInfoItemValues = {
threadInfo.getThreadId(),
threadInfo.getThreadName(),
threadInfo.getThreadState().name(),
threadInfo.getBlockedTime(),
threadInfo.getBlockedCount(),
threadInfo.getWaitedTime(),
threadInfo.getWaitedCount(),
threadInfo.getLockName(),
threadInfo.getLockOwnerId(),
threadInfo.getLockOwnerName(),
stackTraceData,
threadInfo.isSuspended(),
threadInfo.isInNative(),
lockInfoData,
lockedMonitorsData,
lockedSyncsData,
threadInfo.isDaemon(),
threadInfo.getPriority(),
};

With both of this fixes in place I can finally get the self sampling in
NetBeans to work on JDK 12.. (and 11)

Thanks for looking into this

-Sven
-- 
Sven Reimers

* Senior Expert Software Architect
* Java Champion
* JUG Leader JUG Bodensee: http://www.jug-bodensee.de
* Duke's Choice Award Winner 2009


Re: ThreadInfoCompositeData.compositeType() broken in 11 and later

2018-10-21 Thread Sven Reimers
Probable fix use Runtime.version().feature() instead of 0:

public static CompositeType compositeType() {
return
ThreadInfoCompositeTypes.compositeTypes.get(Runtime.version().feature());
}

Sven


On Sun, Oct 21, 2018 at 10:50 AM Sven Reimers 
wrote:

> Hi,
>
> seems with the change for
>
> 8198253: ThreadInfo.from(CompositeData) incorrectly accepts CompositeData
> with missing JDK 6 attributes
>
> ThreadInfoCompositeData.compositeType()  was broken. It just returns
>
> public static CompositeType compositeType() {
> return ThreadInfoCompositeTypes.compositeTypes.get(0);
> }
>
> while ThreadInfoCompositeTypes.compositeTypes consists of
>
> types.put(CURRENT, ctype);
> types.put(5, initV5CompositeType(ctype));
> types.put(6, initV6CompositeType(ctype));
>
> so that compositeType just returns null, which further down the stack
> leads to
>
> java.lang.reflect.InvocationTargetException
> at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
> Method)
> at
> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.base/java.lang.reflect.Method.invoke(Method.java:566)
> at
> org.netbeans.modules.sampler.SamplesOutputStream.toCompositeData(SamplesOutputStream.java:178)
> at
> org.netbeans.modules.sampler.SamplesOutputStream.access$400(SamplesOutputStream.java:44)
> at
> org.netbeans.modules.sampler.SamplesOutputStream$Sample.writeToStream(SamplesOutputStream.java:285)
> at
> org.netbeans.modules.sampler.SamplesOutputStream$Sample.access$300(SamplesOutputStream.java:253)
> at
> org.netbeans.modules.sampler.SamplesOutputStream.close(SamplesOutputStream.java:202)
> at org.netbeans.modules.sampler.Sampler.stopSampling(Sampler.java:231)
> at org.netbeans.modules.sampler.Sampler.stop(Sampler.java:207)
> at
> org.netbeans.core.ui.sampler.SelfSamplerAction$1.doInBackground(SelfSamplerAction.java:90)
> at java.desktop/javax.swing.SwingWorker$1.call(SwingWorker.java:304)
> at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
> at java.desktop/javax.swing.SwingWorker.run(SwingWorker.java:343)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
> at
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
> at java.base/java.lang.Thread.run(Thread.java:835)xx
> Caused by: java.lang.IllegalArgumentException: Argument compositeType
> cannot be null.
> at
> java.management/javax.management.openmbean.CompositeDataSupport.(CompositeDataSupport.java:206)
> at
> java.management/javax.management.openmbean.CompositeDataSupport.(CompositeDataSupport.java:118)
> at
> java.management/sun.management.ThreadInfoCompositeData.getCompositeData(ThreadInfoCompositeData.java:130)
> at
> java.management/sun.management.ThreadInfoCompositeData.toCompositeData(ThreadInfoCompositeData.java:72)
> ... 18 more
>
> See also latest comments on NETBEANS-1359
> 
> .
>
> Thanks for investigating.
>
> -Sven
>


-- 
Sven Reimers

* Senior Expert Software Architect
* Java Champion
* NetBeans Dream Team Member: http://dreamteam.netbeans.org
* Community Leader  NetBeans: http://community.java.net/netbeans
  Desktop Java:
http://community.java.net/javadesktop
* JUG Leader JUG Bodensee: http://www.jug-bodensee.de
* Duke's Choice Award Winner 2009

* XING: https://www.xing.com/profile/Sven_Reimers8
* LinkedIn: http://www.linkedin.com/in/svenreimers


Re: 8196989: Revamp G1 JMX MemoryPool and GarbageCollector MXBean definitions

2018-10-21 Thread David Holmes

Hi Paul,

On 20/10/2018 2:05 AM, Hohensee, Paul wrote:
If we put the flag into deprecation, I’d like to keep it for a year so 
people have time to change their monitoring code (one release to change 
their code, and another to run with their new code), which would be two 
releases. I don’t know how the deprecation process works either. Note 
that if/when this gets backported to jdk8u and/or jdk11u, there’s no 
mechanism there to obsolete a flag.


First the new flag has to be added to the CSR to get approval to be 
added. It would be quite strange to add a new flag and deprecate it at 
the same time - not completely out of the question given its legacy 
compatibility nature, but still odd. So I'd suggest not initially 
deprecating it but rather file a new bug and CSR to deprecate in 13, 
obsolete in 14 and expire in 15. That gives you 12 and 13 where the flag 
is active - though you'll only get a deprecation warning in 13. The 
obsoletion in 14 will require new bug, but not CSR. The expiration is 
normally handled en-masse when we bump the JDK release version.


For 8u the deprecation process is more manual. You need to add an 
explicit check and warning in arguments.cpp, then when you're ready to 
obsolete it add it to the obsolete flag table and remove the deprecation 
warning.


Cheers,
David
-


Discovered an issue with the 
jdk/java/lang/management/MemoryMXBean/CollectionUsageThreshold.java 
test, new new webrev at


http://cr.openjdk.java.net/~phh/8196989/webrev.04/ 



Paul

*From: *JC Beyler 
*Date: *Thursday, October 18, 2018 at 10:47 PM
*To: *"Hohensee, Paul" 
*Cc: *"hotspot-gc-...@openjdk.java.net" 
, "serviceability-dev@openjdk.java.net" 

*Subject: *Re: 8196989: Revamp G1 JMX MemoryPool and GarbageCollector 
MXBean definitions


Hi Paul,

Looks much better to me. The other question I have is about the legacy 
mode. I understand why, from a tool's perspective, having a legacy mode 
is practical. By doing it this way, we are ensuring we don't break any 
tools (or at least they can use a flag to be "unbroken") and give time 
to migrate. This also provides an easier means to backport this fix to 
older JDKs because now the legacy mode can be used to not break anything 
and yet provide a means to migrate to a more sane vision of G1 collector 
definitions.


Should the flag perhaps be automatically put in deprecation and then we 
can mark it as obsolete for JDK13? That would give a limited time for a 
flag but again I'm not sure this is really done?


Or is the plan to keep the flag for a given number of versions, try out 
these new pools and ensure they provide what we need?


Thanks!

Jc

On Thu, Oct 18, 2018 at 3:18 PM Hohensee, Paul > wrote:


Thanks for your review, JC.  New webrev:
http://cr.openjdk.java.net/~phh/8196989/webrev.03/


I updated the copyright date in memoryService.hpp because I forgot
to do so in the patch for
https://bugs.openjdk.java.net/browse/JDK-8195115. Thomas asked me to
fix in it a separate CR, so I’ve reverted it. Ditto the #include
fixes in g1FullGCOopClosures.inline.hpp and g1HeapVerifier.cpp. At
one point during development, clang complained about the latter, but
no longer does.

The ‘break’ on the same line as the ‘}’ was in the original version,
but I’ve moved it. :)

The comment is indeed a bit opaque. I changed it to:

     // Only check heap pools that support a usage threshold.

     // This is typically only the old generation space

     // since the other spaces are expected to get filled up.

     if (p.getType() == MemoryType.HEAP &&

    p.isUsageThresholdSupported()) {

    // In all collectors except G1, only the old
generation supports a

     // usage threshold. The G1 legacy mode "G1 Old Gen"
also does. In

     // G1 default mode, both the old space ("G1 Old
Space": it's not

     // really a generation in the non-G1 collector
sense) and the

     // humongous space ("G1 Humongous Space"), support
a usage threshold.

     // So, the following condition is true for all
non-G1 old generations,

     // for the G1 legacy old gen, and for the G1
default humongous space.

    // It is not true for the G1 default old gen.

     //

     // We're allocating humongous objects in this test,
so the G1 default

     // mode "G1 Old Space" occupancy doesn't change,
because humongous

     // objects are allocated in the "G1 Humongous
Space". If we allowed

     // the G1 default mode "G1 Old Space", notification
would never

     // happen because no objects are 

ThreadInfoCompositeData.compositeType() broken in 11 and later

2018-10-21 Thread Sven Reimers
Hi,

seems with the change for

8198253: ThreadInfo.from(CompositeData) incorrectly accepts CompositeData
with missing JDK 6 attributes

ThreadInfoCompositeData.compositeType()  was broken. It just returns

public static CompositeType compositeType() {
return ThreadInfoCompositeTypes.compositeTypes.get(0);
}

while ThreadInfoCompositeTypes.compositeTypes consists of

types.put(CURRENT, ctype);
types.put(5, initV5CompositeType(ctype));
types.put(6, initV6CompositeType(ctype));

so that compositeType just returns null, which further down the stack leads
to

java.lang.reflect.InvocationTargetException
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)
at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at
org.netbeans.modules.sampler.SamplesOutputStream.toCompositeData(SamplesOutputStream.java:178)
at
org.netbeans.modules.sampler.SamplesOutputStream.access$400(SamplesOutputStream.java:44)
at
org.netbeans.modules.sampler.SamplesOutputStream$Sample.writeToStream(SamplesOutputStream.java:285)
at
org.netbeans.modules.sampler.SamplesOutputStream$Sample.access$300(SamplesOutputStream.java:253)
at
org.netbeans.modules.sampler.SamplesOutputStream.close(SamplesOutputStream.java:202)
at org.netbeans.modules.sampler.Sampler.stopSampling(Sampler.java:231)
at org.netbeans.modules.sampler.Sampler.stop(Sampler.java:207)
at
org.netbeans.core.ui.sampler.SelfSamplerAction$1.doInBackground(SelfSamplerAction.java:90)
at java.desktop/javax.swing.SwingWorker$1.call(SwingWorker.java:304)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.desktop/javax.swing.SwingWorker.run(SwingWorker.java:343)
at
java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at
java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:835)xx
Caused by: java.lang.IllegalArgumentException: Argument compositeType
cannot be null.
at
java.management/javax.management.openmbean.CompositeDataSupport.(CompositeDataSupport.java:206)
at
java.management/javax.management.openmbean.CompositeDataSupport.(CompositeDataSupport.java:118)
at
java.management/sun.management.ThreadInfoCompositeData.getCompositeData(ThreadInfoCompositeData.java:130)
at
java.management/sun.management.ThreadInfoCompositeData.toCompositeData(ThreadInfoCompositeData.java:72)
... 18 more

See also latest comments on NETBEANS-1359

.

Thanks for investigating.

-Sven