Re: RFR (S) 8025238: nsk/jvmti/scenarios/bcinstr/BI04/bi04t002 crashed with SIGSEGV

[Coleen Phillimore]

We didn't file any bugs because I don't remember finding anything 
specific, other than "gosh that code is scary" and "I wish we didn't 
have to do this".


If you find a null 'm' below and call m->print() is the method "obsolete"?

Coleen

On 4/30/14, 8:24 PM, Jeremy Manson wrote:

Did the new bugs get filed for this?

I'm triggering this check with a redefined class (from the 
bootclasspath, if that matters).  To investigate it a little, I 
instrumented StackTraceElement::create thus:


oop java_lang_StackTraceElement::create(methodHandle method, int bci, 
TRAPS) {

  Handle mirror (THREAD, method->method_holder()->java_mirror());
  int method_id = method->method_idnum();

  InstanceKlass* holder = method->method_holder();
  Method* m = holder->method_with_idnum(method_id);
  Method* mp = holder->find_method(method->name(), method->signature());
  method->print_name();
  fprintf(stderr, " method = %p id = %d method' = %p \n", m, 
method_id, mp);
  return create(mirror, method_id, method->constants()->version(), 
bci, THREAD);

}

m is null, and mp isn't.  method->print_name works fine.  This makes 
me feel that the idnum array is out of date somehow.  Before I go down 
the rabbit hole and try to figure out why that is, does someone else 
know why?


Thanks!

Jeremy



On Thu, Oct 3, 2013 at 11:02 AM, Coleen Phillimore 
mailto:coleen.phillim...@oracle.com>> 
wrote:


Summary: Redefined class in stack trace may not be found by
method_idnum so handle null.

This is a simple change.  I had another change to save the method
name (as u2) in the backtrace, but it's not worth the extra
footprint in backtraces for this rare case.

The root problem was that we save method_idnum in the backtrace
(u2) instead of Method* to avoid Method* from being redefined and
deallocated.  I made a change to
InstanceKlass::method_from_idnum() to return null rather than the
last method in the list, which causes this crash. Dan and I went
down the long rabbit-hole of why method_idnum is changed for
obsolete methods and we think there's some cleanup and potential
bugs in this area.  But this is not that change.  I'll file
another bug to continue this investigation for jdk9 (or 8uN).

Staffan created a test - am including core-libs for the review
request.  Also tested with all of the vm testbase tests, mlvm
tests, and java/lang/instrument tests.

open webrev at http://cr.openjdk.java.net/~coleenp/8025238/

bug link https://bugs.openjdk.java.net/browse/JDK-8025238

test case for jdk8 repository:

open webrev at http://cr.openjdk.java.net/~coleenp/8025238_jdk


Thanks,
Coleen

Re: Covariant overrides on the Buffer Hierachy

[Joe Darcy]

Hello,

I'm reminded of Professor Knuth's observation that "Premature 
optimization is the root of all evil."


If from an API perspective the new code is preferable, I would say that 
should take precedence over an at most marginal performance degradation. 
If performance of is a high concern in that area, perhaps a jmh 
experiment could help tease about the difference in speed.


-Joe

On 04/27/2014 04:08 PM, Richard Warburton wrote:

Hi,

There are multiple possible targets for invokevirtual

position:(I)Ljava/nio/Buffer; - all the methods that override it in all
subclasses loaded. It doesn't matter if they are final or not (only if
they are effectively final or not). The non-finality of a method has a
performance impact only if the method *is* overridden in any of the
loaded subclasses, otherwise it is effectively final and treated as such
by JIT (at least that's how I understand it - any hotspot JIT expert
please debunk my claims).


That is only true if you are calling the method on the base class, which
is normally seldom done (though it will be nearly always in legacy code,
but see below).

That might also be the answer to why the synthetic method need not be

marked as final if the covariant method is. The synthetic method can
never be overridden in a sub-class (at least not by javac) - only the
covariant method can be.


Doesn't sound quite right to me, but I'll defer to any experts who might
wish to discuss it on compiler-dev.


  But as Paul noted, the methods on Buffer are probably not used in hot

loops alone, since they are just for reading/adjusting
position/limit/mark. The hot loops probably also contain methods for
reading/writing the buffer and those are only defined on particular
sub-types of java.nio.Buffer, so it can reasonably be expected that the
static (guaranteed) type of target upon which methods are called in hot
loops is a particular subtype of java.nio.Buffer and JIT only has one
method to choose from in this case.


Yeah new code will call the covariant final method directly in almost all
cases for sure, having been compiled against it.  The tricky case is old
code - even if the JIT can figure out that it is really a subclass being
invoked upon, it'd still be forced to target the non-covariant synthetic
method due to the specific method reference in the bytecode. Hopefully at
this point though, as you say, the lack of overriding classes will be
enough to optimize the dispatch.


So interestingly the original patch didn't add final to the covariant
overrides. I've updated the patch to do this.

http://cr.openjdk.java.net/~rwarburton/buffer-overrides-1/

I'm not sure what would satisfy people on the performance front. Some
guidance would be most appreciated here.

regards,

   Richard Warburton

   http://insightfullogic.com
   @RichardWarburto 

Draft JEP: JDK Core Libraries Test Stabilization

[Stuart Marks]

Hi all,

Here's a draft JEP for stabilizing the core libraries regression test suite, 
that is, fixing up the spuriously failing tests. Please review and comment.


Thanks!

s'marks




Title: JDK Core Libraries Test Stabilization
Author: Stuart Marks
Organization: Oracle
Discussion: core-libs-dev@openjdk.java.net
[...other metadata elided...]

Summary
---

The JDK Regression Test Suite has several thousand fully automated
tests. These tests are valuable and effective in that they serve to
prevent bugs from entering the code base. However, they suffer from
many intermittent failures. Many of these failures are "spurious" in
that they are not caused by bugs in the product. Spurious failures add
considerable noise to test reports; they make it impossible for
developers to ascertain whether a particular change has introduced a
bug; and they obscure actual failures.

The reliability of the regression suite has improved considerably over
the past few years. However, there are perhaps still 100-200 tests
that fail intermittently, and most of these failures are spurious.
This project aims to reduce the number and frequency of spuriously
failing tests to a level where it no longer is an impediment to
development.

This project targets tests from the regression suite that cover the
JDK Core Libraries, including base packages (java.lang, io, nio, util)
I18N, Networking, RMI, Security, and Serviceability. JAXP and CORBA
are also included, although they have relatively few regression tests
at present.


Non-Goals
-

Regression tests for other areas, including Hotspot, Langtools, and
Client areas, are not included in this project.

This project does not address operational issues that might cause
builds or test runs to fail or for reports not to be delivered in a
timely fashion.

This project is not focused on product bugs that cause test
failures. Such test failures are "good" in that the test suite is
providing valid information about the product.

Test runs on embedded platforms are not covered by this project.


Success Metrics
---

The reliability of a successful test run (100% pass) currently stands
at approximately 0.5%. The goal is to improve this success rate to
98%, exclusive of true failures (i.e., those caused by bugs in the
product). At a 98% success rate, a continuous build system that runs ten
jobs per day, five days a week would have one or fewer spurious
failures per week.


Motivation
--

Developers are continually hampered by the unreliability of the
regression test suite. Intermittently failing tests add significant
noise to the results of every test run. The consequence is that
developers cannot tell whether test failures were caused by bugs
introduced by a recent change or whether they are spurious
failures. In addition, the intermittent failures mask actual failures
in the product, slowing development and reducing quality. Developers
should be able to rely on the test suite telling them accurate
information: test failures should indicate the introduction of a bug
into the system, and absence of test failures should be usable as
evidence that changes are correct.


Description
---

Spurious test failures fall into two broad categories:

 - test bugs
 - environmental issues

Our working assumption for most intermittent test failures is that
they are spurious, and further, that they are caused by bugs in the
test itself. While it is possible for a product bug to cause an
intermittent failure, this is relatively rare. The majority of
intermittent failures encountered so far have indeed proven to be test
bugs.

"Environmental" issues, such as misconfigured test machines, temporary
dysfunction on the machine running the test job (e.g., filesystem
full), or transient network failures, also contribute to spurious
failures. Test should be made more robust, if possible. Environment
issues should be fed back to the infrastructure team for resolution
and future infrastructure improvements.

A variety of techniques will be employed to diagnose, track, and help
develop fixes for intermittently failing tests:

 - track all test failures in JBS
 - repeated test runs against the same build
 - gather statistics about failure rates, # tests with bugs, and track 
continuously
 - investigate pathologies for common test failure modes
 - develop techniques for fixing common test bugs
 - develop test library code to improve commonality across tests and to
   avoid typical failure modes
 - add instrumentation to tests (and to the test suite) to improve 
diagnosability
 - exclude tests judiciously, preferably only as a last resort
 - change reviews
 - code inspections


Alternatives


The most likely alternative to diagnosing and fixing intermittent
failures is to aggressively exclude intermittently failing tests from
the test suite. This trades off code coverage in favor of test
reliability, adding risk of undetected bug introduction.


Testing
---

The subject of 

Re: 8035782 : sun/launcher/LauncherHelper$FXHelper loaded unnecessarily

[Kumar Srinivasan]

For completeness the bugid line needs the bugid as shown, otherwise SQE 
will open

another bug to have you fix this.

-26  * @bug 8001533 8004547
+26  * @bug 8001533 8004547 8035782
 


other than that it looks good, I can push this with the above change.

Anyone else have any concerns with this change before I push ?

Thanks
Kumar


On 4/30/2014 1:47 PM, Neil Toda wrote:


Please review Launcher change and test.

I've added to the Launcher test : FXLauncherTest.java
The test will now check that LauncherHelper$FXHelper is not loaded for 
non-JavaFX class and jar files.


webrev.02 contains only review suggestions from webrev.01 and the new 
test class.


http://cr.openjdk.java.net/~ntoda/8035782/webrev.02/

for bug:

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

Thanks

-neil

Re: Remove redundant calls of toString()

[Joe Darcy]

In general, I think Objects.requireNonNull() should often be considered 
idiomatic Java.


If the constant-folding is to be avoided, I would prefer to see

"foo".toString();

have a comment like

"foo".toString(); // Avoid mandatory constant propagation

-Joe

On 04/27/2014 08:05 PM, Otávio Gonçalves de Santana wrote:

In my opinion not, because Objects.requireNonNull is more readable than
just string.toString. This way is more understandable which field is
required and doesn't impact on performance.


On Sun, Apr 27, 2014 at 11:33 PM, David Holmes wrote:


On 28/04/2014 3:41 AM, Otávio Gonçalves de Santana wrote:


sorry.
I tried answer and the message was twice.
   But Yes when has null pointer possibility I replaced to
Objects.requireNonNull.


In my opinion that is making the code worse not better.

David
-


  I am review the code again.

The code below:

diff -r e323c74edabd
src/share/classes/com/sun/tools/example/debug/tty/Commands.java
--- a/src/share/classes/com/sun/tools/example/debug/tty/Commands.java Wed
Apr 23 11:35:40 2014 -0700
+++ b/src/share/classes/com/sun/tools/example/debug/tty/Commands.java Sun
Apr 27 14:33:45 2014 -0300
@@ -1653,20 +1653,20 @@
   String expr = t.nextToken("");
   Value val = evaluate(expr);
   if (val == null) {
-MessageOutput.println("expr is null", expr.toString());
+MessageOutput.println("expr is
null",Objects.requireNonNull(expr));
   } else if (dumpObject && (val instanceof ObjectReference) &&
  !(val instanceof StringReference)) {
   ObjectReference obj = (ObjectReference)val;
   ReferenceType refType = obj.referenceType();
   MessageOutput.println("expr is value",
-  new Object [] {expr.toString(),
+  new Object []
{Objects.requireNonNull(expr),

MessageOutput.format("grouping begin character")});
   dump(obj, refType, refType);
   MessageOutput.println("grouping end character");
   } else {
 String strVal = getStringValue();
 if (strVal != null) {
- MessageOutput.println("expr is value", new Object []
{expr.toString(),
+ MessageOutput.println("expr is value", new Object []
{Objects.requireNonNull(expr),

   strVal});
  }
   }
diff -r e323c74edabd
src/share/classes/java/lang/annotation/IncompleteAnnotationException.java
---
a/src/share/classes/java/lang/annotation/IncompleteAnnotationException.java
Wed
Apr 23 11:35:40 2014 -0700
+++
b/src/share/classes/java/lang/annotation/IncompleteAnnotationException.java
Sun
Apr 27 14:33:45 2014 -0300
@@ -25,6 +25,8 @@

   package java.lang.annotation;

+import java.util.Objects;
+
   /**
* Thrown to indicate that a program has attempted to access an element
of
* an annotation type that was added to the annotation type definition
after
@@ -56,7 +58,7 @@
   Class annotationType,
   String elementName) {
   super(annotationType.getName() + " missing element " +
-  elementName.toString());
+Objects.requireNonNull(elementName));

   this.annotationType = annotationType;
   this.elementName = elementName;
diff -r e323c74edabd src/share/classes/java/text/DateFormatSymbols.java
--- a/src/share/classes/java/text/DateFormatSymbols.java Wed Apr 23
11:35:40 2014 -0700
+++ b/src/share/classes/java/text/DateFormatSymbols.java Sun Apr 27
14:33:45 2014 -0300
@@ -594,7 +594,7 @@
*/
   public void setLocalPatternChars(String newLocalPatternChars) {
   // Call toString() to throw an NPE in case the argument is null
-localPatternChars = newLocalPatternChars.toString();
+localPatternChars = Objects.requireNonNull(
newLocalPatternChars);
   cachedHashCode = 0;
   }

diff -r e323c74edabd
src/share/classes/javax/management/modelmbean/DescriptorSupport.java
--- a/src/share/classes/javax/management/modelmbean/DescriptorSupport.java
Wed
Apr 23 11:35:40 2014 -0700
+++ b/src/share/classes/javax/management/modelmbean/DescriptorSupport.java
Sun
Apr 27 14:33:45 2014 -0300
@@ -43,13 +43,7 @@
   import java.lang.reflect.Constructor;

   import java.security.AccessController;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Map;
-import java.util.Set;
-import java.util.SortedMap;
-import java.util.StringTokenizer;
-import java.util.TreeMap;
+import java.util.*;
   import java.util.logging.Level;

   import javax.management.Descriptor;
@@ -665,7 +659,7 @@
   "getFieldNames()", "Field is null");
   }
   } else {
-responseFields[i] = currElement.getKey().toString();
+responseFields[i] =
Objects.requireNonNull(currElement.getKey());
 

8035782 : sun/launcher/LauncherHelper$FXHelper loaded unnecessarily

[Neil Toda]

Please review Launcher change and test.

I've added to the Launcher test : FXLauncherTest.java
The test will now check that LauncherHelper$FXHelper is not loaded for 
non-JavaFX class and jar files.


webrev.02 contains only review suggestions from webrev.01 and the new 
test class.


http://cr.openjdk.java.net/~ntoda/8035782/webrev.02/

for bug:

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

Thanks

-neil

Re: RFR: 8000975: (process) Merge UNIXProcess.java.bsd & UNIXProcess.java.linux (& .solaris & .aix)

[Alan Bateman]

On 25/04/2014 17:47, roger riggs wrote:

Hi Peter,

Including the test update with the updated changeset is fine.

(I think Alan had some comments on the refactoring and has not yet had 
a chance to comment).


Thanks, Roger

I reviewed previous rounds and you've addressed my points so I think I'm 
mostly happy with this. As some point I think we should look at Platform 
again as there may be an opportunity later in JDK 9 to move it out of 
UNIXProcess.


One thing that I wasn't sure about is the additions to the @author tags 
as we've mostly been trying not to grow these (contentious topic, I 
don't have a strong opinion as I never use @author).


A minor comment looking at the latest webrev.07 is that some of the 
lines are very long. It's not a problem now but I could image future 
side-by-side reviews needing a scroll 
bar.DeferredCloseProcessPipeInputStream is very long too.


-Alan.

Re: Guidance about binary / data files for JTREG tests

[Florian Weimer]

On 04/30/2014 04:03 PM, Wang Weijun wrote:

Florian

Just curious, how do you deal with a file like this

   
http://hg.openjdk.java.net/jdk9/dev/jdk/file/4e7f3aac979b/test/sun/security/krb5/ktab/HighestKvno.java

Is the byte array inside considered binary?


We consider content problematic for which source code apparently exists 
(and which is the preferred form of making modifications), but it's not 
part of the sources we use to build RPM packages.


The comment above the byte array indicates that this situation is 
different in this case, and I don't think Fedora or Debian has problems 
with it.  For more authoritative information on this particular case, 
you'd have to ask on the fedora-legal or debian-legal mailing lists.


(I'm treating this a question about policies for content inclusion in 
free software distributions—this is not legal advice.)


--
Florian Weimer / Red Hat Product Security Team

Re: Guidance about binary / data files for JTREG tests

[Wang Weijun]

Florian

Just curious, how do you deal with a file like this

  
http://hg.openjdk.java.net/jdk9/dev/jdk/file/4e7f3aac979b/test/sun/security/krb5/ktab/HighestKvno.java

Is the byte array inside considered binary?

Thanks
Max

On Apr 30, 2014, at 18:54, Florian Weimer  wrote:

> On 04/29/2014 02:35 PM, Alan Bateman wrote:
> 
>> The other thing that came up previously is distributions that have a
>> policy of not allowing binary files. If I'm not mistake then they are
>> deleted by downstream patches, which in this case would lead to a test
>> failure.
> 
> Debian and Fedora are concerned with binary files which have closed source 
> files.  These are stripped when noticed.  There is no indiscriminate deletion 
> of binary files during the packaging process.
> 
> The Debian packaging tools had a problem with shipping binary data in local 
> changes, but that's history now.
> 
> -- 
> Florian Weimer / Red Hat Product Security Team

Re: Remove redundant calls of toString()

[Claes Redestad]

On 2014-04-30 13:08, Remi Forax wrote:

On 04/29/2014 11:20 AM, Claes Redestad wrote:


On 2014-04-29 09:31, Remi Forax wrote:

On 04/28/2014 05:43 PM, Claes Redestad wrote:

On 04/28/2014 08:57 AM, David Holmes wrote:

On 28/04/2014 1:05 PM, Otávio Gonçalves de Santana wrote:
In my opinion not, because Objects.requireNonNull is more 
readable than

just string.toString. This way is more understandable which field is
required and doesn't impact on performance.


An invocation of requireNonNull is potentially more expensive than 
the implicit null check that happens with foo.toString().


David
- 


My thought was that these two would be inlined to the exact same 
thing, so I did a quick test to see what happens when you do 
foo.toString() versus Objects.requireNonNull(foo) on a set of 
randomly generated String[]'s with different amounts of null 
elements(0p: no null entries, 1p: 1% null entries etc):


Benchmark Mode Samples Mean   
Mean errorUnits
s.m.ThrowAwayBenchmark.nullToString0pthrpt 6 356653.044 
3573.707   ops/ms
s.m.ThrowAwayBenchmark.nullToString1p thrpt 6 
353128.903 2764.102   ops/ms
s.m.ThrowAwayBenchmark.nullToString10p thrpt 6 
297956.571 9580.251   ops/ms
s.m.ThrowAwayBenchmark.nullToString50p thrpt 6 
158172.036 1893.096   ops/ms
s.m.ThrowAwayBenchmark.nullToString100p thrpt 6 
18194.614  472.091   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull0p thrpt 6 
357855.126 2979.090   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull1p thrpt 6 
67601.134 7004.689   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull10p thrpt 6 
8150.595  538.970   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull50p thrpt 6 
1604.919  220.903   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull100p thrpt 6 
820.626   60.752   ops/ms


Yikes! As long as the value is never null they're inlined nicely 
and neither have the upper hand performance-wise, but as soon as 
you get some null values, Objects.requireNonNull degenerates much 
faster than its foo.toString counterpart. I think this is a JIT 
issue - optimizing exception-paths might not be the highest 
priority, but Objects.requireNonNull is used pretty extensively in 
the JDK and my expectation would be that it shouldn't degrade 
performance when things actually are null now and then.


/Claes


This is a know issue, I think it's not related to the way the JIT 
handle exception path but what is called 'profile pollution'.
Hotspot JITs have two ways to do a null check, either do nothing 
(yes nothing) and let the system do a fault and come back from dead 
using a signal handler, this solution is named implicit null check 
or by doing an explicit null check, i.e a conditional jump.
Implicit null check is faster but if the receiver is null, it cost 
you an harm, so the VM profiles receiver to remember if the receiver 
of each call can be null or not. The problem is that when you call 
foo.toString(), the profile information is associated with the 
instruction that does foo.toString() while if the call is 
Objects.requireNonNull(foo), the profile associated with the 
nullcheck is stored inside the method requireNonNull, so if one call 
to requireNonNull in the entire program throw a NPE, the profile 
inside requireNonNull now register that it may fail, so for the VM 
all calls to requireNonNull may fail. So currently you should not 
use requireNonNull is the value is not required to be null*
I guess I should have given more details. :-) I ran my micros on a 
number of forked VMs to ensure I don't get excessive run-to-run 
variations, which among other things avoids profile pollution. Also I 
sloppily collected very few samples and only did minimal warmup after 
I saw that the micros produced scores that were orders of magnitude 
apart and the values stabilized after less than two seconds: Flags: 
-f 3 -wi 4 -i 2


When I run the micros sequentially in the same VM I see some added 
degradation for the exceptional cases, but the 0p cases still inline 
to the optimal(?) 360k ops/ms, so while it seems profile pollution 
might play in when provoked, HotSpot seems to manage these simple 
micros well enough.


Ok, so you have to  take a look to the generated assembly code :)
https://wiki.openjdk.java.net/display/HotSpot/PrintAssembly
I've discussed the problem with some members of the compiler team who 
seem to think they have a pretty good idea about what's happening and 
thought I should file an RFE for this: 
https://bugs.openjdk.java.net/browse/JDK-8042127 - I suggest we continue 
the discussion there and/or start a new thread on this topic.


So the problem is that Hotspot blindly trusts the recorded profile 
compared to the profile that can come from the arguments of a call.
The good news is that recently some patches were included in the 
jdk9 tree to fix or at least mitigate that issue.


I've now run tests on JDK8 FCS and a

Re: Remove redundant calls of toString()

[Remi Forax]

On 04/29/2014 11:20 AM, Claes Redestad wrote:


On 2014-04-29 09:31, Remi Forax wrote:

On 04/28/2014 05:43 PM, Claes Redestad wrote:

On 04/28/2014 08:57 AM, David Holmes wrote:

On 28/04/2014 1:05 PM, Otávio Gonçalves de Santana wrote:
In my opinion not, because Objects.requireNonNull is more readable 
than

just string.toString. This way is more understandable which field is
required and doesn't impact on performance.


An invocation of requireNonNull is potentially more expensive than 
the implicit null check that happens with foo.toString().


David
- 


My thought was that these two would be inlined to the exact same 
thing, so I did a quick test to see what happens when you do 
foo.toString() versus Objects.requireNonNull(foo) on a set of 
randomly generated String[]'s with different amounts of null 
elements(0p: no null entries, 1p: 1% null entries etc):


Benchmark Mode Samples Mean   
Mean errorUnits
s.m.ThrowAwayBenchmark.nullToString0pthrpt 6 356653.044 
3573.707   ops/ms
s.m.ThrowAwayBenchmark.nullToString1p thrpt 6 353128.903 
2764.102   ops/ms
s.m.ThrowAwayBenchmark.nullToString10p thrpt 6 
297956.571 9580.251   ops/ms
s.m.ThrowAwayBenchmark.nullToString50p thrpt 6 
158172.036 1893.096   ops/ms
s.m.ThrowAwayBenchmark.nullToString100p thrpt 6 
18194.614  472.091   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull0p thrpt 6 
357855.126 2979.090   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull1p thrpt 6 
67601.134 7004.689   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull10p thrpt 6 
8150.595  538.970   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull50p thrpt 6 
1604.919  220.903   ops/ms
s.m.ThrowAwayBenchmark.requireNonNull100p thrpt 6 
820.626   60.752   ops/ms


Yikes! As long as the value is never null they're inlined nicely and 
neither have the upper hand performance-wise, but as soon as you get 
some null values, Objects.requireNonNull degenerates much faster 
than its foo.toString counterpart. I think this is a JIT issue - 
optimizing exception-paths might not be the highest priority, but 
Objects.requireNonNull is used pretty extensively in the JDK and my 
expectation would be that it shouldn't degrade performance when 
things actually are null now and then.


/Claes


This is a know issue, I think it's not related to the way the JIT 
handle exception path but what is called 'profile pollution'.
Hotspot JITs have two ways to do a null check, either do nothing (yes 
nothing) and let the system do a fault and come back from dead using 
a signal handler, this solution is named implicit null check or by 
doing an explicit null check, i.e a conditional jump.
Implicit null check is faster but if the receiver is null, it cost 
you an harm, so the VM profiles receiver to remember if the receiver 
of each call can be null or not. The problem is that when you call 
foo.toString(), the profile information is associated with the 
instruction that does foo.toString() while if the call is 
Objects.requireNonNull(foo), the profile associated with the 
nullcheck is stored inside the method requireNonNull, so if one call 
to requireNonNull in the entire program throw a NPE, the profile 
inside requireNonNull now register that it may fail, so for the VM 
all calls to requireNonNull may fail. So currently you should not use 
requireNonNull is the value is not required to be null*
I guess I should have given more details. :-) I ran my micros on a 
number of forked VMs to ensure I don't get excessive run-to-run 
variations, which among other things avoids profile pollution. Also I 
sloppily collected very few samples and only did minimal warmup after 
I saw that the micros produced scores that were orders of magnitude 
apart and the values stabilized after less than two seconds: Flags: -f 
3 -wi 4 -i 2


When I run the micros sequentially in the same VM I see some added 
degradation for the exceptional cases, but the 0p cases still inline 
to the optimal(?) 360k ops/ms, so while it seems profile pollution 
might play in when provoked, HotSpot seems to manage these simple 
micros well enough.


Ok, so you have to  take a look to the generated assembly code :)
https://wiki.openjdk.java.net/display/HotSpot/PrintAssembly

So the problem is that Hotspot blindly trusts the recorded profile 
compared to the profile that can come from the arguments of a call.
The good news is that recently some patches were included in the jdk9 
tree to fix or at least mitigate that issue.


I've now run tests on JDK8 FCS and a recent internal build of JDK9 and 
see the same characteristics. How recently are we talking here? :-)


I don't know, I have seen the patches but not dive into the source yet, 
it's on my todo list, maybe it's not even activated by default.





cheers,
Rémi
* read that last sentence again, it seems very logical, no ?

While your reading of the method name

Re: [9] Review request for 8029451: Tidy warnings cleanup for java.util package; minor changes in java.nio, java.sql

[alexander stepanov]

Hello Lance,

oh yes, thanks!

Regards,
Alexander

On 30.04.2014 14:39, Lance Andersen wrote:


On Apr 30, 2014, at 4:23 AM, alexander stepanov 
> wrote:


Sorry, could please anybody else review the changes? We need at least 
two positive responses to push the changes.


Why?  You only need one reviewer prior to FC: 
http://openjdk.java.net/guide/changePlanning.html#bug





Thanks,
Alexander


On 18.04.2014 23:47, Lance Andersen wrote:

looks fine
On Apr 18, 2014, at 10:04 AM, alexander stepanov 
 
> wrote:



Hello,

Could you please review the fix for the following bug:
https://bugs.openjdk.java.net/browse/JDK-8029451

Webrev corresponding:
http://cr.openjdk.java.net/~yan/JDK-8029451/webrev.00/

Just a minor cleanup of javadoc to avoid tidy warnings; no other 
code affected.


Thanks.

Regards,
Alexander




Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com  











Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

Re: Guidance about binary / data files for JTREG tests

[Florian Weimer]

On 04/29/2014 02:35 PM, Alan Bateman wrote:


The other thing that came up previously is distributions that have a
policy of not allowing binary files. If I'm not mistake then they are
deleted by downstream patches, which in this case would lead to a test
failure.


Debian and Fedora are concerned with binary files which have closed 
source files.  These are stripped when noticed.  There is no 
indiscriminate deletion of binary files during the packaging process.


The Debian packaging tools had a problem with shipping binary data in 
local changes, but that's history now.


--
Florian Weimer / Red Hat Product Security Team

Re: [9] Review request for 8029451: Tidy warnings cleanup for java.util package; minor changes in java.nio, java.sql

[Lance Andersen]

On Apr 30, 2014, at 4:23 AM, alexander stepanov 
 wrote:

> Sorry, could please anybody else review the changes? We need at least two 
> positive responses to push the changes.

Why?  You only need one reviewer prior to FC: 
http://openjdk.java.net/guide/changePlanning.html#bug


> 
> Thanks,
> Alexander
> 
> 
> On 18.04.2014 23:47, Lance Andersen wrote:
>> looks fine
>> On Apr 18, 2014, at 10:04 AM, alexander stepanov 
>> mailto:alexander.v.stepa...@oracle.com>> 
>> wrote:
>> 
>>> Hello,
>>> 
>>> Could you please review the fix for the following bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8029451
>>> 
>>> Webrev corresponding:
>>> http://cr.openjdk.java.net/~yan/JDK-8029451/webrev.00/
>>> 
>>> Just a minor cleanup of javadoc to avoid tidy warnings; no other code 
>>> affected.
>>> 
>>> Thanks.
>>> 
>>> Regards,
>>> Alexander
>> 
>> 
>> 
>> Lance Andersen| 
>> Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering
>> 1 Network Drive
>> Burlington, MA 01803
>> lance.ander...@oracle.com 
>> 
>> 
>> 
> 



Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com

Re: [9] Review request for 8029451: Tidy warnings cleanup for java.util package; minor changes in java.nio, java.sql

[alexander stepanov]

Sorry, could please anybody else review the changes? We need at least 
two positive responses to push the changes.


Thanks,
Alexander


On 18.04.2014 23:47, Lance Andersen wrote:

looks fine
On Apr 18, 2014, at 10:04 AM, alexander stepanov 
> wrote:



Hello,

Could you please review the fix for the following bug:
https://bugs.openjdk.java.net/browse/JDK-8029451

Webrev corresponding:
http://cr.openjdk.java.net/~yan/JDK-8029451/webrev.00/

Just a minor cleanup of javadoc to avoid tidy warnings; no other code 
affected.


Thanks.

Regards,
Alexander




Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com