Re: RFR: 8074948 javadoc typo in DiagnosticCommandMBean.java: {code instead of {@code
On 3/11/15 5:38 AM, Staffan Larsen wrote:
Please review this addition of a missing @.
/Staffan
diff --git
a/src/java.management/share/classes/com/sun/management/DiagnosticCommandMBean.java
b/src/java.management/share/classes/com/sun/management/DiagnosticCommandMBean.java
---
a/src/java.management/share/classes/com/sun/management/DiagnosticCommandMBean.java
+++
b/src/java.management/share/classes/com/sun/management/DiagnosticCommandMBean.java
@@ -31,7 +31,7 @@
/**
* Management interface for the diagnostic commands for the HotSpot Virtual
Machine.
*
- * pThe {code DiagnosticCommandMBean} is registered to the
+ * pThe {@code DiagnosticCommandMBean} is registered to the
* {@linkplain java.lang.management.ManagementFactory#getPlatformMBeanServer
* platform MBeanServer} as are other platform MBeans.
*
Looks good to me.
Mandy
Re: RFR: 8074812 More specific error message when the .java_pid well-known file is not secure
On 12.3.2015 08:18, Staffan Larsen wrote: On 11 mar 2015, at 20:37, Martin Buchholz marti...@google.com mailto:marti...@google.com wrote: Producing good error messages is such hard work! Aye. And so often forgotten. Instead of 0%3o, use 0%03o Since you want to print the lowest 9 bits of the mode, don't you want 0x1ff Absolutely. I opted for the octal representation 0777 instead which seemed fitting here. new webrev: http://cr.openjdk.java.net/~sla/8074812/webrev.02/ This looks fine. -JB- Thanks, /Staffan On Wed, Mar 11, 2015 at 2:30 AM, Staffan Larsen staffan.lar...@oracle.com mailto:staffan.lar...@oracle.com wrote: Thanks for the feedback. Here is a new version that prints out more details for each of the errors messages. Let me know if you have suggestions for better wording. It also adds an #include for jvm.h that was missing from some of the files (it is needed for jio_snprintf). webrev: http://cr.openjdk.java.net/~sla/8074812/webrev.01/ /Staffan On 10 mar 2015, at 19:07, Martin Buchholz marti...@google.com mailto:marti...@google.com wrote: On Tue, Mar 10, 2015 at 10:53 AM, Jaroslav Bachorik jaroslav.bacho...@oracle.com mailto:jaroslav.bacho...@oracle.com wrote: This just got me thinking - would including [sb.st_uid, uid] and [sb.st_gid, gid] in the error message be of any additional benefit? Yes. How much do you want to improve the quality of error messages? You could use the word effective only when effective and real users don't match. You could print out the two mismatched values.
Re: RFR: JDK-8075056 Remove Version.java.template from jconsole
On 2015-03-12 13:54, Staffan Larsen wrote: 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/~sla/8075056/webrev.00/ Thanks, /Staffan Looks good. Thank you! /Magnus
Re: inlining AllocateHeap()
Hi Yasumasa,
On 12/03/2015 9:58 PM, Yasumasa Suenaga wrote:
Hi all,
I tried to use NMT with details option on OpenJDK7 on RHEL6.6, but I got
address at AllocateHeap() as malloc() caller.
I checked symbol in libjvm.so http://libjvm.so/ in OracleJDK8u40 Linux
x64, it has AllocateHeap()
symbol.
AllocateHeap() is defined as inline function, and it gives CURRENT_PC to
os::malloc(). I guess that implementation expects AllocateHeap() will be
inlined.
It seems so.
It may occur with GCC (g++) optimization only, however I want to fix it to
analyze native memory with NMT on Linux.
According to the docs [1]:
GCC does not inline any functions when not optimizing unless you
specify the ‘always_inline’ attribute for the function
I applied patch as below. This patch makes AllocateHeap() as inline
function.
--
diff -r af3b0db91659 src/share/vm/memory/allocation.inline.hpp
--- a/src/share/vm/memory/allocation.inline.hpp Mon Mar 09 09:30:16 2015
-0700
+++ b/src/share/vm/memory/allocation.inline.hpp Thu Mar 12 20:45:57 2015
+0900
@@ -62,11 +62,18 @@
}
return p;
}
+
+#ifdef __GNUC__
+__attribute__((always_inline))
+#endif
I dislike seeing the gcc specific directives in common code. I'm
wondering whether we should perhaps only use CURRENT_PC in product (and
optimized?) builds and use CALLER_PC otherwise. That would be imperfect
of course It also makes me wonder whether the inlining is occurring as
expected on other platforms.
I'd like to get other people's views on this.
Thanks,
David
[1] https://gcc.gnu.org/onlinedocs/gcc/Inline.html
inline char* AllocateHeap(size_t size, MEMFLAGS flags,
AllocFailType alloc_failmode = AllocFailStrategy::EXIT_OOM) {
return AllocateHeap(size, flags, CURRENT_PC, alloc_failmode);
}
+#ifdef __GNUC__
+__attribute__((always_inline))
+#endif
inline char* ReallocateHeap(char *old, size_t size, MEMFLAGS flag,
AllocFailType alloc_failmode = AllocFailStrategy::EXIT_OOM) {
char* p = (char*) os::realloc(old, size, flag, CURRENT_PC);
--
If this patch is accepted, I will file it to JBS and will upload webrev.
Thanks,
Yasumasa
RFR (M) 8067662: java.lang.NullPointerException: Method name is null from StackTraceElement.init
Please, review the jdk 9 fix for: https://bugs.openjdk.java.net/browse/JDK-8067662 Open hotspot webrev: http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8067662-JVMTI-trace.1/ Open webrev for unit test update: http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/jdk/8067662-JVMTI-trace-test.1/ Summary: An NPE is trown in a thread dumping via JMX when doing CPU profiling in NetBeans Profiler and VisualVM. The issue is that the methods and related klass versions are not kept alive if a class redefinition takes place between catching a Throwable and taking a thread dump. It can result in loosing an obsolete method, and so, the reconstruction of method name becomes impossible. In such a case, the null reference is returned instead of a valid method name. The solution is to use current klass version and method orig_idnum instead of ordinary method idnum to find required method pointer. In the worst case when the related klass version is gone (was not included to or was removed from the previous_versions linked list), a saved method name CP index of the latest klass version can be used to restore the method name. The footprint extra overhead for this approach is u2 per stack frame. The plan is also to backport the fix to 8u60. Testing: In progress: nsk redefine classes tests, JTREG java/lang/instrument Thanks, Serguei
RE: RFR: JDK-8075056 Remove Version.java.template from jconsole
Hi, Staffan. bug: https://bugs.openjdk.java.net/browse/JDK-8075056 webrev: http://cr.openjdk.java.net/~sla/8075056/webrev.00/ http://cr.openjdk.java.net/~sla/8075056/webrev.00/ Looks good. I'm always happy to see changes where complexity is reduced. Thanks, iris
Re: RFR: 8074812 More specific error message when the .java_pid well-known file is not secure
Looks good to me! On Thu, Mar 12, 2015 at 12:18 AM, Staffan Larsen staffan.lar...@oracle.com wrote: On 11 mar 2015, at 20:37, Martin Buchholz marti...@google.com wrote: Producing good error messages is such hard work! Aye. And so often forgotten. Instead of 0%3o, use 0%03o Since you want to print the lowest 9 bits of the mode, don't you want 0x1ff Absolutely. I opted for the octal representation 0777 instead which seemed fitting here. new webrev: http://cr.openjdk.java.net/~sla/8074812/webrev.02/ Thanks, /Staffan On Wed, Mar 11, 2015 at 2:30 AM, Staffan Larsen staffan.lar...@oracle.com wrote: Thanks for the feedback. Here is a new version that prints out more details for each of the errors messages. Let me know if you have suggestions for better wording. It also adds an #include for jvm.h that was missing from some of the files (it is needed for jio_snprintf). webrev: http://cr.openjdk.java.net/~sla/8074812/webrev.01/ /Staffan On 10 mar 2015, at 19:07, Martin Buchholz marti...@google.com wrote: On Tue, Mar 10, 2015 at 10:53 AM, Jaroslav Bachorik jaroslav.bacho...@oracle.com wrote: This just got me thinking - would including [sb.st_uid, uid] and [sb.st_gid, gid] in the error message be of any additional benefit? Yes. How much do you want to improve the quality of error messages? You could use the word effective only when effective and real users don't match. You could print out the two mismatched values.
Re: RFR: JDK-8075056 Remove Version.java.template from jconsole
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
RFR: JDK-8075056 Remove Version.java.template from jconsole
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/~sla/8075056/webrev.00/ Thanks, /Staffan
Re: RFR: JDK-8072913: [REDO] GCCause should distinguish jcmd GC.run from System.gc()
Hi, On 2015-03-11 14:13, Yasumasa Suenaga wrote: Hi all, So I think we can remove _jvmti_force_gc from is_user_requested_gc() and add _dcmd_gc_run to it. I've uploaded new webrev, and I've applied it to new patch. Could you review it? http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.01/ Sorry for the delay, I've been pretty busy lately. Does anyone have time to look at this change? /Mikael I also updated jtreg testcase. It works fine in my environment. Thanks, Yasumasa On 2015/02/14 22:10, Yasumasa Suenaga wrote: Hi Mikael, I'd prefer if you could add a GCCause::is_system_gc_equivalent() which returns true for some set of GCCause enum values, such as _java_lang_system_gc and _dcmd_gc_run Can I add _dcmd_gc_run to GCCause::is_user_requested_gc() ? This function is used with GCCause::is_serviceability_requested_gc() . CMSCollector::is_external_interruption() and AdaptiveSizePolicy::check_gc_overhead_limit() is_user_requested_gc() and is_serviceability_requested_gc() checkes _jvmti_force_gc is selected. So I think we can remove _jvmti_force_gc from is_user_requested_gc() and add _dcmd_gc_run to it. A grep for _java_lang_system_gc should yield more places where updates may be necessary. We can use GCCause::is_user_requested_gc() if the proposal in above is accepted. Thanks Yasumasa On 2015/02/13 21:33, Mikael Gerdin wrote: Hi Yasumasa, On 2015-02-11 15:02, Yasumasa Suenaga wrote: Hi all, I've committed JDK-8068589 to add new GCCause - Diagnostic Command. However, it has been backouted because test is failed [1] and it is not considered about concurrent GC: -XX:+ExplicitGCInvokesConcurrent [2]. I've created patch for this enhancement. Could you review it? http://cr.openjdk.java.net/~ysuenaga/JDK-8072913/webrev.00/ I'd prefer if you could add a GCCause::is_system_gc_equivalent() which returns true for some set of GCCause enum values, such as _java_lang_system_gc and _dcmd_gc_run Given that the documentation of the GC.run command is: GC.run Call java.lang.System.gc(). Impact: Medium: Depends on Java heap size and content. Syntax: GC.run I interpret the documentation that the GC is supposed to be (for all intents and purposes) equivalent to the application invoking System.gc(). This would also require updates to other places where we refer to the _java_lang_system_gc GCCause, such as UseAdaptiveSizePolicyWithSystemGC A grep for _java_lang_system_gc should yield more places where updates may be necessary. /Mikael I'm jdk9 committer, but I'm not employee at Oracle. So I need a Sponsor. Thanks, Yasumasa [1] http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-February/011957.html [2] http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2015-February/011962.html
Re: RFR: JDK-8075056 Remove Version.java.template from jconsole
Looks good. /Erik On 2015-03-12 13:54, Staffan Larsen wrote: 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/~sla/8075056/webrev.00/ Thanks, /Staffan
Re: RFR: JDK-8075056 Remove Version.java.template from jconsole
On 12/03/2015 12:54, Staffan Larsen wrote: 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/~sla/8075056/webrev.00/ Looks okay to me. -Alan
inlining AllocateHeap()
Hi all,
I tried to use NMT with details option on OpenJDK7 on RHEL6.6, but I got
address at AllocateHeap() as malloc() caller.
I checked symbol in libjvm.so in OracleJDK8u40 Linux x64, it has
AllocateHeap()
symbol.
AllocateHeap() is defined as inline function, and it gives CURRENT_PC to
os::malloc(). I guess that implementation expects AllocateHeap() will be
inlined.
It may occur with GCC (g++) optimization only, however I want to fix it to
analyze native memory with NMT on Linux.
I applied patch as below. This patch makes AllocateHeap() as inline
function.
--
diff -r af3b0db91659 src/share/vm/memory/allocation.inline.hpp
--- a/src/share/vm/memory/allocation.inline.hpp Mon Mar 09 09:30:16 2015
-0700
+++ b/src/share/vm/memory/allocation.inline.hpp Thu Mar 12 20:45:57 2015
+0900
@@ -62,11 +62,18 @@
}
return p;
}
+
+#ifdef __GNUC__
+__attribute__((always_inline))
+#endif
inline char* AllocateHeap(size_t size, MEMFLAGS flags,
AllocFailType alloc_failmode = AllocFailStrategy::EXIT_OOM) {
return AllocateHeap(size, flags, CURRENT_PC, alloc_failmode);
}
+#ifdef __GNUC__
+__attribute__((always_inline))
+#endif
inline char* ReallocateHeap(char *old, size_t size, MEMFLAGS flag,
AllocFailType alloc_failmode = AllocFailStrategy::EXIT_OOM) {
char* p = (char*) os::realloc(old, size, flag, CURRENT_PC);
--
If this patch is accepted, I will file it to JBS and will upload webrev.
Thanks,
Yasumasa
RFR: Resolve disabled warnings for the JVMTI demos
Please review these small fixes to remove a couple of warnings in the JVMTI demos. bug: https://bugs.openjdk.java.net/browse/JDK-8074841 bug: https://bugs.openjdk.java.net/browse/JDK-8074842 webrev: http://cr.openjdk.java.net/~sla/8074841/webrev.00/ Thanks, /Staffan
