Re: RFR: 8159590: Remove deprecated methods from jdk.internal.misc.VM

2016-06-16 Thread David Holmes

Hi Claes,

On 17/06/2016 8:20 AM, Claes Redestad wrote:

Hi David,

On 2016-06-16 23:11, David Holmes wrote:

On 16/06/2016 10:52 PM, Claes Redestad wrote:

On 06/16/2016 02:48 PM, Chris Hegarty wrote:

Apologies, you corrected me off-line,   TG.allowThreadSuspension calls
VM.unsuspendSomeThreads ( which is a no-op ) and not
VM.unsuspendThreads
( which I thought it did ).   In which case I am ok with the change.


Ok, thanks! :-)


Isn't this variable unused now:

  63 boolean vmAllowSuspension;


Interestingly enough there are hooks in hotspot to compute offsets of
various fields in j.l.ThreadGroup, so even though the vmAllowSuspension
field is unused (on both sides), removing the field would require
changing code in hotspot.

Tumbled down that rabbit hole to see how deep it goes, and was
pleasantly surprised that it didn't go very far at all:

HS: http://cr.openjdk.java.net/~redestad/8159590/hotspot.02/
JDK: http://cr.openjdk.java.net/~redestad/8159590/jdk.02/

I think the cleanup on the vm side might be trivial enough to be wrapped
into this change. What do you think?


I'd need to look into the SA code as well and double-check some things. 
But I'm off till next week :) If you want to move ahead urgently then we 
can file a follow up cleanup bug for hotspot.


Thanks.

David



Thanks!

/Claes



Thanks,
David



I think we should add ‘forRemoval = true’ while here.


I'm good with that, but does modifying the way in which something is
deprecated require a CCC request?

/Claes



-Chris.



On 16 Jun 2016, at 13:39, Chris Hegarty 
wrote:

On 15 Jun 2016, at 14:30, Claes Redestad 
wrote:

Hi,

after VM.java was encapsulated and moved from sun.misc to
jdk.internal.misc, the rationale for keeping a number of deprecated
methods and constants no longer applies and these methods should be
removed:

Webrev: http://cr.openjdk.java.net/~redestad/8159590/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8159590

I had forgotten that there was a public API, ThreadGroup, that
exposed some
of this functionality.  Sadly I think that we may have to keep this,
for now, and
remove it in 10.

ThreadGroup::allowThreadSuspension should, however, have its
@Deprecated
annotation updated with ‘forRemoval = true’, then removed early in 10.

I can’t remember if this was on Stuarts list or not, but I think it
is ok to do it
separately anyway.

-Chris.




Re: RFR: 8150680 JarFile.Release enum needs reconsideration with respect to it's values

2016-06-16 Thread Paul Sandoz

> On 16 Jun 2016, at 14:44, Steve Drach  wrote:
> 
> This webrev uses methods instead of fields to return the base and runtime 
> values used internally by JarFile.  I’ve also optimized it a bit.
> 
> http://cr.openjdk.java.net/~sdrach/8150680/webrev.02/ 
> 
> 

JarFIle
—

 132 private final static int base_version;

You are using lower case, here, this caught me out as i thought it was an 
non-static field. Call it something like BASE_VERSION_MAJOR.


 155 BASE_VERSION = Runtime.Version.parse(String.valueOf(base_version));

 164 RUNTIME_VERSION = 
Runtime.Version.parse(String.valueOf(runtimeVersion));

Use Integer.toString rather than String.valueOf (also update specification).


 337 public final Runtime.Version getVersion() {
 338 if (VERSION == null) {
 339 if (isMultiRelease()) {
 340 VERSION = Runtime.Version.parse(String.valueOf(version));
 341 } else {
 342 VERSION = BASE_VERSION;
 343 }
 344 }
 345 return VERSION;
 346 }
 347 private Runtime.Version VERSION;

You are using the style for a static field.

In the JarFile constructor why don’t you just store the version passed in 
unless MULTI_RELEASE_FORCED?

Have final fields:

  final Runtime.Version version;
  final int version_major;

then do:

  if (MULTI_RELEASE_FORCED || version.major() == RUNTIME_VERSION.major()) {
  // This also deals with the common case where the value from 
JarFile.runtimeVersion() is passed
  this.version = RUNTIME_VERSION;
  } else if (version.major() <= BASE_VERSION_MAJOR) {
  // This also deals with the common case where the value from 
JarFile.baseVersion() is passed
  this.version = BASE_VERSION;
  } else {
 // Canonicalize
 this.version = Runtime.Version.parse(Integer.toString(version.major()));
  }
  this.version_major = version.major();

Paul.




>> On Jun 15, 2016, at 4:31 PM, Joseph D. Darcy  wrote:
>> 
>> Steve,
>> 
>> In JarFile, please use methods not fields to return the new information. The 
>> information in question is not constant across versions. Using methods 
>> instead of fields avoid over-committing on a particular implementation, etc.
>> 
>> Cheers,
>> 
>> -Joe
>> 
>> On 6/15/2016 3:49 PM, Steve Drach wrote:
>>> I’ve updated the webrev to address the issue of the constructor accepting 
>>> values like Version.parse(“7.1”)
>>> 
>>> http://cr.openjdk.java.net/~sdrach/8150680/webrev.01/ 
>>> 
>>> 
 On Jun 15, 2016, at 8:56 AM, Steve Drach  wrote:
 
>> Please review the following changeset:
>> 
>> webrev: http://cr.openjdk.java.net/~sdrach/8150680/webrev.00/index.html 
>> 
>> issue: https://bugs.openjdk.java.net/browse/JDK-8150680 
>> 
>> 
>> The issue calls for reconsidering the JarFile.Release enum.  A comment 
>> in the bug report suggests replacing JarFile.Release with 
>> Runtime.Version, and that’s what I did.  Specifically I removed the 
>> enum, changed the constructor to accept a Runtime.Version object instead 
>> of a JarFile.Release object, updated all places in the JDK that invoked 
>> the constructor and updated all tests.
>> 
> Moving to Runtime.Version seems right but doesn't the javadoc for the 
> constructor need to be updated to make it clear how it behavior when 
> invoking with something like Version.parse("7.1") ? If I read the code 
> correctly then this will be accepted and getVersion() will return 7.1.
 Yes, it needs to be updated and it needs to be fixed.  Thanks for finding 
 that.
 
> Fields or methods is another discussion point for the base and runtime 
> versions.
 My thinking is, in this case fields and methods are equivalent, the method 
 not giving any more flexibility than a field.  For example the method 
 JarFile.baseVersion will just return the value contained in the private 
 final static field BASE_VERSION.  Or the public final static field 
 BASE_VERSION can be directly accessed.  I see no advantage of a method 
 here.  But I’m willing to be enlightened.
 
> -Alan.
>> 
> 



Re: RFR: 8159590: Remove deprecated methods from jdk.internal.misc.VM

2016-06-16 Thread Claes Redestad

Hi David,

On 2016-06-16 23:11, David Holmes wrote:

On 16/06/2016 10:52 PM, Claes Redestad wrote:

On 06/16/2016 02:48 PM, Chris Hegarty wrote:

Apologies, you corrected me off-line,   TG.allowThreadSuspension calls
VM.unsuspendSomeThreads ( which is a no-op ) and not VM.unsuspendThreads
( which I thought it did ).   In which case I am ok with the change.


Ok, thanks! :-)


Isn't this variable unused now:

  63 boolean vmAllowSuspension;


Interestingly enough there are hooks in hotspot to compute offsets of 
various fields in j.l.ThreadGroup, so even though the vmAllowSuspension 
field is unused (on both sides), removing the field would require 
changing code in hotspot.


Tumbled down that rabbit hole to see how deep it goes, and was 
pleasantly surprised that it didn't go very far at all:


HS: http://cr.openjdk.java.net/~redestad/8159590/hotspot.02/
JDK: http://cr.openjdk.java.net/~redestad/8159590/jdk.02/

I think the cleanup on the vm side might be trivial enough to be wrapped 
into this change. What do you think?


Thanks!

/Claes



Thanks,
David



I think we should add ‘forRemoval = true’ while here.


I'm good with that, but does modifying the way in which something is
deprecated require a CCC request?

/Claes



-Chris.



On 16 Jun 2016, at 13:39, Chris Hegarty 
wrote:

On 15 Jun 2016, at 14:30, Claes Redestad 
wrote:

Hi,

after VM.java was encapsulated and moved from sun.misc to
jdk.internal.misc, the rationale for keeping a number of deprecated
methods and constants no longer applies and these methods should be
removed:

Webrev: http://cr.openjdk.java.net/~redestad/8159590/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8159590

I had forgotten that there was a public API, ThreadGroup, that
exposed some
of this functionality.  Sadly I think that we may have to keep this,
for now, and
remove it in 10.

ThreadGroup::allowThreadSuspension should, however, have its
@Deprecated
annotation updated with ‘forRemoval = true’, then removed early in 10.

I can’t remember if this was on Stuarts list or not, but I think it
is ok to do it
separately anyway.

-Chris.




[9] RFR: 8159548: Formatter returns unexpected strings if locale is null.

2016-06-16 Thread Naoto Sato

Hi,

Please review the fix to the following issue:

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

The proposed fix is located at:

http://cr.openjdk.java.net/~naoto/8159548/webrev.00/

The previous fix to 8146156 overly localized the formatted text for the 
"null" locale. The offending localization code were reverted to the 
original.


Naoto


Re: RFR: 8150680 JarFile.Release enum needs reconsideration with respect to it's values

2016-06-16 Thread Steve Drach
This webrev uses methods instead of fields to return the base and runtime 
values used internally by JarFile.  I’ve also optimized it a bit.

http://cr.openjdk.java.net/~sdrach/8150680/webrev.02/ 

 
> On Jun 15, 2016, at 4:31 PM, Joseph D. Darcy  wrote:
> 
> Steve,
> 
> In JarFile, please use methods not fields to return the new information. The 
> information in question is not constant across versions. Using methods 
> instead of fields avoid over-committing on a particular implementation, etc.
> 
> Cheers,
> 
> -Joe
> 
> On 6/15/2016 3:49 PM, Steve Drach wrote:
>> I’ve updated the webrev to address the issue of the constructor accepting 
>> values like Version.parse(“7.1”)
>> 
>> http://cr.openjdk.java.net/~sdrach/8150680/webrev.01/ 
>> 
>> 
>>> On Jun 15, 2016, at 8:56 AM, Steve Drach  wrote:
>>> 
> Please review the following changeset:
> 
> webrev: http://cr.openjdk.java.net/~sdrach/8150680/webrev.00/index.html 
> 
> issue: https://bugs.openjdk.java.net/browse/JDK-8150680 
> 
> 
> The issue calls for reconsidering the JarFile.Release enum.  A comment in 
> the bug report suggests replacing JarFile.Release with Runtime.Version, 
> and that’s what I did.  Specifically I removed the enum, changed the 
> constructor to accept a Runtime.Version object instead of a 
> JarFile.Release object, updated all places in the JDK that invoked the 
> constructor and updated all tests.
> 
 Moving to Runtime.Version seems right but doesn't the javadoc for the 
 constructor need to be updated to make it clear how it behavior when 
 invoking with something like Version.parse("7.1") ? If I read the code 
 correctly then this will be accepted and getVersion() will return 7.1.
>>> Yes, it needs to be updated and it needs to be fixed.  Thanks for finding 
>>> that.
>>> 
 Fields or methods is another discussion point for the base and runtime 
 versions.
>>> My thinking is, in this case fields and methods are equivalent, the method 
>>> not giving any more flexibility than a field.  For example the method 
>>> JarFile.baseVersion will just return the value contained in the private 
>>> final static field BASE_VERSION.  Or the public final static field 
>>> BASE_VERSION can be directly accessed.  I see no advantage of a method 
>>> here.  But I’m willing to be enlightened.
>>> 
 -Alan.
> 



Re: RFR: 8159590: Remove deprecated methods from jdk.internal.misc.VM

2016-06-16 Thread David Holmes

On 16/06/2016 10:52 PM, Claes Redestad wrote:

On 06/16/2016 02:48 PM, Chris Hegarty wrote:

Apologies, you corrected me off-line,   TG.allowThreadSuspension calls
VM.unsuspendSomeThreads ( which is a no-op ) and not VM.unsuspendThreads
( which I thought it did ).   In which case I am ok with the change.


Ok, thanks! :-)


Isn't this variable unused now:

 63 boolean vmAllowSuspension;

Thanks,
David



I think we should add ‘forRemoval = true’ while here.


I'm good with that, but does modifying the way in which something is
deprecated require a CCC request?

/Claes



-Chris.



On 16 Jun 2016, at 13:39, Chris Hegarty 
wrote:

On 15 Jun 2016, at 14:30, Claes Redestad 
wrote:

Hi,

after VM.java was encapsulated and moved from sun.misc to
jdk.internal.misc, the rationale for keeping a number of deprecated
methods and constants no longer applies and these methods should be
removed:

Webrev: http://cr.openjdk.java.net/~redestad/8159590/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8159590

I had forgotten that there was a public API, ThreadGroup, that
exposed some
of this functionality.  Sadly I think that we may have to keep this,
for now, and
remove it in 10.

ThreadGroup::allowThreadSuspension should, however, have its @Deprecated
annotation updated with ‘forRemoval = true’, then removed early in 10.

I can’t remember if this was on Stuarts list or not, but I think it
is ok to do it
separately anyway.

-Chris.




Re: RFR 9: 8155808: Process.getInputStream().skip() method is faulty

2016-06-16 Thread Roger Riggs

Ping?


On 6/13/2016 11:06 AM, Roger Riggs wrote:
A latent issue with Process InputStreams support for the skip method 
was reported[1].


Though the InputStream returned is a BufferedInputStream, in some cases,
the skip method calls FileInputStream.seek on the pipe containing output
from the process but the pipes do not support seek.  The proposed fix 
is to override
the skip method to implement skip as reading the bytes instead of 
invoking seek.


Please review and comment:

Webrev:

http://cr.openjdk.java.net/~rriggs/webrev-skip-8155808/index.html

Issue:

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

Thanks, Roger






Re: Proposal: java.lang.reflect.Proxy and default methods

2016-06-16 Thread Steven Schlansker

> On Jun 16, 2016, at 12:29 PM, Mandy Chung  wrote:
> 
> Can you elaborate how proxy and default methods affects your library to able 
> to run with security manager?

So in this particular case, I wanted to create an interface type where some 
methods are automatically generated
from declarative annotations and some are not.

interface MyDao {
@SqlQuery("SELECT count(1) FROM table WHERE state='foo'")
int getFooCount();

@SqlQuery("SELECT count(1) FROM table WHERE state='bar'")
int getBarCount();

@InTransaction
default getFooAndBarCount() {
return getFooCount() + getBarCount();
}
}

Another use case I have, generating configuration beans from properties:

interface SomeConfig {
@Config("key.a")
int getA(); // required

@Config("key.b")
default int getB() {
return 3;  // a nice way to specify the default value
}
}

Neither of these are impossible to work around.  But I think the list of 
situations
where this is useful is long and mostly not explored because it is very hard
to do so today :)

(Specifically about the security manager comment, I am using the workaround 
outlined
in this thread which involves calling setAccessible on the MethodHandle 
constructor
to avoid the unreflectSpecial security check.)

> 
> This is a good enhancement.  I have created a JBS issue to track that [1].  
> There is no such existing enhancement request in JBS.
> 
> Mandy
> [1] https://bugs.openjdk.java.net/browse/JDK-8159746
> 
>> On Jun 16, 2016, at 11:14 AM, Steven Schlansker  
>> wrote:
>> 
>> Hi everyone,
>> 
>> I am glad this was discussed on the list again.
>> I just wanted to throw out there, as a library developer and
>> end user, I now have had to reproduce this hack in no fewer
>> than four different projects for various reasons.
>> 
>> It is even worse because it means that my library cannot run
>> under a SecurityManager.  While this isn't a problem for me
>> personally it does limit the utility of any libraries I might produce.
>> 
>> It really is a glaring missing API and if it does not make Java 9
>> I can't imagine I will be the only person that is extremely disappointed.
>> 
>> Thanks,
>> Steven
>> 
>>> On Jun 6, 2016, at 1:58 PM, Mandy Chung  wrote:
>>> 
>>> Hi Peter,
>>> 
>>> Thanks for the proposal. This feature has been lacking.  When this subject 
>>> was brought, I also have similiar thought to provide a way in Proxy class 
>>> for InvocationHandler to invoke a default method (but of course no time to 
>>> put into it).
>>> 
>>> I appreciate your contribution and good work.  I support to add this 
>>> feature in a future release.
>>> 
>>> I personally don’t feel comfortable to absorb this small API targetting for 
>>> JDK 9 (since I don’t have the cycle to shepherd this in the next few 
>>> months). I may be overly conversative but I won’t take security assessment 
>>> lightly.  Maybe someone else is able to help you move this forward before I 
>>> am available.
>>> 
>>> Mandy
>>> 
 On Jun 3, 2016, at 7:58 AM, Peter Levart  wrote:
 
 Hi,
 
 Since Java SE 8 introduced default methods in interfaces there was a 
 question what to do with java.lang.reflect.Proxy API. Nothing was done to 
 the API at that time, so the default behavior is to proxy default methods 
 too. InvocationHandler gets invoked for default methods, but it has not 
 provision to forward such calls to the default implementations in the 
 interfaces.
 
 I propose a simple API addition that allows calling super default methods 
 in proxy instances:
 
 http://cr.openjdk.java.net/~plevart/jdk9-dev/Proxy.invokeSuperDefaults/webrev.02/
 
 With this addition one can simply decide in the InvocationHandler what to 
 do with invocations to default methods and can forward such invocation to 
 the default implementation:
 
 public class Test {
 
 interface I1 {
 default void m() {
 System.out.println("  default I1.m() called");
 }
 }
 
 interface I2 {
 default void m() {
 System.out.println("  default I2.m() called");
 }
 }
 
 interface I12 extends I1, I2 {
 @Override
 void m();
 
 default int sum(int a, int b) {
 return a + b;
 }
 
 default Object[] concat(Object first, Object... rest) {
 Object[] result = new Object[1 + rest.length];
 result[0] = first;
 System.arraycopy(rest, 0, result, 1, rest.length);
 return result;
 }
 }
 
 public static void main(String[] args) {
 
 InvocationHandler h = (proxy, method, params) -> {
 System.out.println("\nInvocationHandler called for: " + method +
" with parameters: " + Arrays.toString(params));
 if (method.isDefault()) {
 try {
 return Proxy.invo

Re: Proposal: java.lang.reflect.Proxy and default methods

2016-06-16 Thread Mandy Chung
Can you elaborate how proxy and default methods affects your library to able to 
run with security manager?

This is a good enhancement.  I have created a JBS issue to track that [1].  
There is no such existing enhancement request in JBS.

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

> On Jun 16, 2016, at 11:14 AM, Steven Schlansker  
> wrote:
> 
> Hi everyone,
> 
> I am glad this was discussed on the list again.
> I just wanted to throw out there, as a library developer and
> end user, I now have had to reproduce this hack in no fewer
> than four different projects for various reasons.
> 
> It is even worse because it means that my library cannot run
> under a SecurityManager.  While this isn't a problem for me
> personally it does limit the utility of any libraries I might produce.
> 
> It really is a glaring missing API and if it does not make Java 9
> I can't imagine I will be the only person that is extremely disappointed.
> 
> Thanks,
> Steven
> 
>> On Jun 6, 2016, at 1:58 PM, Mandy Chung  wrote:
>> 
>> Hi Peter,
>> 
>> Thanks for the proposal. This feature has been lacking.  When this subject 
>> was brought, I also have similiar thought to provide a way in Proxy class 
>> for InvocationHandler to invoke a default method (but of course no time to 
>> put into it).
>> 
>> I appreciate your contribution and good work.  I support to add this feature 
>> in a future release.
>> 
>> I personally don’t feel comfortable to absorb this small API targetting for 
>> JDK 9 (since I don’t have the cycle to shepherd this in the next few 
>> months). I may be overly conversative but I won’t take security assessment 
>> lightly.  Maybe someone else is able to help you move this forward before I 
>> am available.
>> 
>> Mandy
>> 
>>> On Jun 3, 2016, at 7:58 AM, Peter Levart  wrote:
>>> 
>>> Hi,
>>> 
>>> Since Java SE 8 introduced default methods in interfaces there was a 
>>> question what to do with java.lang.reflect.Proxy API. Nothing was done to 
>>> the API at that time, so the default behavior is to proxy default methods 
>>> too. InvocationHandler gets invoked for default methods, but it has not 
>>> provision to forward such calls to the default implementations in the 
>>> interfaces.
>>> 
>>> I propose a simple API addition that allows calling super default methods 
>>> in proxy instances:
>>> 
>>> http://cr.openjdk.java.net/~plevart/jdk9-dev/Proxy.invokeSuperDefaults/webrev.02/
>>> 
>>> With this addition one can simply decide in the InvocationHandler what to 
>>> do with invocations to default methods and can forward such invocation to 
>>> the default implementation:
>>> 
>>> public class Test {
>>> 
>>>  interface I1 {
>>>  default void m() {
>>>  System.out.println("  default I1.m() called");
>>>  }
>>>  }
>>> 
>>>  interface I2 {
>>>  default void m() {
>>>  System.out.println("  default I2.m() called");
>>>  }
>>>  }
>>> 
>>>  interface I12 extends I1, I2 {
>>>  @Override
>>>  void m();
>>> 
>>>  default int sum(int a, int b) {
>>>  return a + b;
>>>  }
>>> 
>>>  default Object[] concat(Object first, Object... rest) {
>>>  Object[] result = new Object[1 + rest.length];
>>>  result[0] = first;
>>>  System.arraycopy(rest, 0, result, 1, rest.length);
>>>  return result;
>>>  }
>>>  }
>>> 
>>>  public static void main(String[] args) {
>>> 
>>>  InvocationHandler h = (proxy, method, params) -> {
>>>  System.out.println("\nInvocationHandler called for: " + method +
>>> " with parameters: " + Arrays.toString(params));
>>>  if (method.isDefault()) {
>>>  try {
>>>  return Proxy.invokeSuper(proxy, method, params);
>>>  } catch (InvocationTargetException e) {
>>>  throw e.getCause();
>>>  }
>>>  } else {
>>>  switch (method.getName()) {
>>>  case "m":
>>>  System.out.println("  abstract I12.m(): called");
>>>  return null;
>>>  default:
>>>  throw new UnsupportedOperationException(
>>>  "Unsupported method: " + method);
>>>  }
>>>  }
>>>  };
>>> 
>>>  I1 i1 = (I1) Proxy.newProxyInstance(
>>>  I1.class.getClassLoader(), new Class[]{I1.class}, h);
>>>  i1.m();
>>> 
>>>  I2 i2 = (I2) Proxy.newProxyInstance(
>>>  I2.class.getClassLoader(), new Class[]{I2.class}, h);
>>>  i2.m();
>>> 
>>>  I12 i12 = (I12) Proxy.newProxyInstance(
>>>  I12.class.getClassLoader(), new Class[]{I12.class}, h);
>>>  i12.m();
>>> 
>>>  System.out.println("  1 + 2 = " + i12.sum(1, 2));
>>>  System.out.println("  [1] concat [2, 3, 4] = " +
>>> Arrays.toString(i12.concat(1, 2, 3, 4)));
>>>  }
>>> }
>>> 
>>> 
>>> I know FC date is over, but this is really a small change and I have he

Re: [PATCH] Implement a noop clear() for Collections#EMPTY_LIST

2016-06-16 Thread Paul Sandoz
Hi,

Late to this thread…

I think for consistency it’s ok to apply in all three cases. I will push today.

—

Another trivial fix in Collections would be to go through the nested classes 
and mark appropriate classes as final.

Paul.

> On 28 May 2016, at 11:27, Louis Wasserman  wrote:
> 
> Do you actually expect that to represent a significant performance 
> difference?  All those calls sound like things easy to JIT.
> 
> 
> On Sat, May 28, 2016, 11:26 AM Mohamed Naufal  > wrote:
> Hi,
> 
> You're right of course, I should have been clearer, no allocation happens for 
> clear() on EmptyMap or EmptySet, but a lot of unnecessary calls are made.
> 
> The call stack for clear() on EmptySet looks something like this:
> AbstractCollection#clear() ->EmptySet#iterator() -> 
> Collections#emptyIterator() -> returns the singleton 
> EmptyIterator#EMPTY_ITERATOR on which hasNext() is called.
> 
> And for EmptyMap:
> AbstractMap#clear() -> EmptyMap#entrySet() -> Collections#emptySet(), from 
> which point onwards, it's similar to that of EmptySet.
> 
> This could be avoided with the direct override.
> 
> Thanks,
> Naufal
> 
> On 28 May 2016 at 22:32, Louis Wasserman  > wrote:
> Is it?  IIRC EmptyMap and EmptySet will use a singleton unmodifiable empty 
> Iterator, so they won't incur any allocation, and the clear() will finish 
> immediately with no work anyway.
> 
> 
> On Sat, May 28, 2016, 2:05 AM Mohamed Naufal  > wrote:
> Hi,
> 
> I see that this is applicable to EmptyMap and EmptySet as well, here's an
> updated patch with clear() overridden for all 3 classes.
> 
> Thanks,
> Naufal
> 
> On 23 May 2016 at 16:13, Paul Sandoz  > wrote:
> 
> > Hi Naufal,
> >
> > Thanks for looking at this.
> >
> > For us to accept your patch (no matter how small) you need to become a
> > contributor, which requires that you agree to the Oracle Contributor
> > Agreement (OCA), see:
> >
> >   http://openjdk.java.net/contribute/ 
> >
> > Thanks,
> > Paul.
> >
> > > On 22 May 2016, at 12:10, Mohamed Naufal  > > > wrote:
> > >
> > > Hi,
> > >
> > > A call to clear() on Collections#EMPTY_LIST is currently redirected to
> > > AbstractList#clear(), which performs a bunch of checks and creates a
> > > ListItr object, all of which is unnecessary for an EmptyList.
> > >
> > > PFA a patch that implements a noop clear() for EmptyList.
> > >
> > > Thanks,
> > > Naufal
> > > 
> >
> >
> 



Re: Proposal: java.lang.reflect.Proxy and default methods

2016-06-16 Thread Steven Schlansker
Hi everyone,

I am glad this was discussed on the list again.
I just wanted to throw out there, as a library developer and
end user, I now have had to reproduce this hack in no fewer
than four different projects for various reasons.

It is even worse because it means that my library cannot run
under a SecurityManager.  While this isn't a problem for me
personally it does limit the utility of any libraries I might produce.

It really is a glaring missing API and if it does not make Java 9
I can't imagine I will be the only person that is extremely disappointed.

Thanks,
Steven

> On Jun 6, 2016, at 1:58 PM, Mandy Chung  wrote:
> 
> Hi Peter,
> 
> Thanks for the proposal. This feature has been lacking.  When this subject 
> was brought, I also have similiar thought to provide a way in Proxy class for 
> InvocationHandler to invoke a default method (but of course no time to put 
> into it).
> 
> I appreciate your contribution and good work.  I support to add this feature 
> in a future release.
> 
> I personally don’t feel comfortable to absorb this small API targetting for 
> JDK 9 (since I don’t have the cycle to shepherd this in the next few months). 
> I may be overly conversative but I won’t take security assessment lightly.  
> Maybe someone else is able to help you move this forward before I am 
> available.
> 
> Mandy
> 
>> On Jun 3, 2016, at 7:58 AM, Peter Levart  wrote:
>> 
>> Hi,
>> 
>> Since Java SE 8 introduced default methods in interfaces there was a 
>> question what to do with java.lang.reflect.Proxy API. Nothing was done to 
>> the API at that time, so the default behavior is to proxy default methods 
>> too. InvocationHandler gets invoked for default methods, but it has not 
>> provision to forward such calls to the default implementations in the 
>> interfaces.
>> 
>> I propose a simple API addition that allows calling super default methods in 
>> proxy instances:
>> 
>> http://cr.openjdk.java.net/~plevart/jdk9-dev/Proxy.invokeSuperDefaults/webrev.02/
>> 
>> With this addition one can simply decide in the InvocationHandler what to do 
>> with invocations to default methods and can forward such invocation to the 
>> default implementation:
>> 
>> public class Test {
>> 
>>   interface I1 {
>>   default void m() {
>>   System.out.println("  default I1.m() called");
>>   }
>>   }
>> 
>>   interface I2 {
>>   default void m() {
>>   System.out.println("  default I2.m() called");
>>   }
>>   }
>> 
>>   interface I12 extends I1, I2 {
>>   @Override
>>   void m();
>> 
>>   default int sum(int a, int b) {
>>   return a + b;
>>   }
>> 
>>   default Object[] concat(Object first, Object... rest) {
>>   Object[] result = new Object[1 + rest.length];
>>   result[0] = first;
>>   System.arraycopy(rest, 0, result, 1, rest.length);
>>   return result;
>>   }
>>   }
>> 
>>   public static void main(String[] args) {
>> 
>>   InvocationHandler h = (proxy, method, params) -> {
>>   System.out.println("\nInvocationHandler called for: " + method +
>>  " with parameters: " + Arrays.toString(params));
>>   if (method.isDefault()) {
>>   try {
>>   return Proxy.invokeSuper(proxy, method, params);
>>   } catch (InvocationTargetException e) {
>>   throw e.getCause();
>>   }
>>   } else {
>>   switch (method.getName()) {
>>   case "m":
>>   System.out.println("  abstract I12.m(): called");
>>   return null;
>>   default:
>>   throw new UnsupportedOperationException(
>>   "Unsupported method: " + method);
>>   }
>>   }
>>   };
>> 
>>   I1 i1 = (I1) Proxy.newProxyInstance(
>>   I1.class.getClassLoader(), new Class[]{I1.class}, h);
>>   i1.m();
>> 
>>   I2 i2 = (I2) Proxy.newProxyInstance(
>>   I2.class.getClassLoader(), new Class[]{I2.class}, h);
>>   i2.m();
>> 
>>   I12 i12 = (I12) Proxy.newProxyInstance(
>>   I12.class.getClassLoader(), new Class[]{I12.class}, h);
>>   i12.m();
>> 
>>   System.out.println("  1 + 2 = " + i12.sum(1, 2));
>>   System.out.println("  [1] concat [2, 3, 4] = " +
>>  Arrays.toString(i12.concat(1, 2, 3, 4)));
>>   }
>> }
>> 
>> 
>> I know FC date is over, but this is really a small change and I have heard 
>> several people that such feature is missing from the Proxy API.
>> 
>> I'm prepared to create jtreg tests covering the specification if this 
>> proposal is accepted.
>> 
>> Regards, Peter
>> 
>> 
>> 
> 



RFR(L): 8143211: provide bytecode intrinsics for loop and try/finally executors

2016-06-16 Thread Michael Haupt
Dear all,

please review this change.
RFE: https://bugs.openjdk.java.net/browse/JDK-8143211
Webrev: http://cr.openjdk.java.net/~mhaupt/8143211/webrev.00/

The change puts the tryFinally and loop method handle combinator 
implementations, which were introduced as part of the JEP 274 effort, on a 
LambdaForm basis, which executes in bytecode generating (default) and 
LambdaForm interpretation 
(-Djava.lang.invoke.MethodHandle.COMPILE_THRESHOLD=-1) modes. It also changes 
the output formatting of LambdaForms, introducing a (hopefully) more readable 
format.

Thanks,

Michael

-- 

 
Dr. Michael Haupt | Principal Member of Technical Staff
Phone: +49 331 200 7277 | Fax: +49 331 200 7561
Oracle Java Platform Group | LangTools Team | Nashorn
Oracle Deutschland B.V. & Co. KG | Schiffbauergasse 14 | 14467 Potsdam, Germany

ORACLE Deutschland B.V. & Co. KG | Hauptverwaltung: Riesstraße 25, D-80992 
München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V. | Hertogswetering 163/167, 
3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Nederland, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher
  Oracle is committed to developing 
practices and products that help protect the environment



Re: RFR: 8159590: Remove deprecated methods from jdk.internal.misc.VM

2016-06-16 Thread Claes Redestad

On 06/16/2016 02:48 PM, Chris Hegarty wrote:

Apologies, you corrected me off-line,   TG.allowThreadSuspension calls
VM.unsuspendSomeThreads ( which is a no-op ) and not VM.unsuspendThreads
( which I thought it did ).   In which case I am ok with the change.


Ok, thanks! :-)



I think we should add ‘forRemoval = true’ while here.


I'm good with that, but does modifying the way in which something is 
deprecated require a CCC request?


/Claes



-Chris.



On 16 Jun 2016, at 13:39, Chris Hegarty  wrote:

On 15 Jun 2016, at 14:30, Claes Redestad  wrote:

Hi,

after VM.java was encapsulated and moved from sun.misc to jdk.internal.misc, 
the rationale for keeping a number of deprecated methods and constants no 
longer applies and these methods should be removed:

Webrev: http://cr.openjdk.java.net/~redestad/8159590/webrev.01/
Bug: https://bugs.openjdk.java.net/browse/JDK-8159590

I had forgotten that there was a public API, ThreadGroup, that exposed some
of this functionality.  Sadly I think that we may have to keep this, for now, 
and
remove it in 10.

ThreadGroup::allowThreadSuspension should, however, have its @Deprecated
annotation updated with ‘forRemoval = true’, then removed early in 10.

I can’t remember if this was on Stuarts list or not, but I think it is ok to do 
it
separately anyway.

-Chris.




Re: RFR: 8159590: Remove deprecated methods from jdk.internal.misc.VM

2016-06-16 Thread Chris Hegarty
Apologies, you corrected me off-line,   TG.allowThreadSuspension calls
VM.unsuspendSomeThreads ( which is a no-op ) and not VM.unsuspendThreads
( which I thought it did ).   In which case I am ok with the change. 

I think we should add ‘forRemoval = true’ while here.

-Chris.


> On 16 Jun 2016, at 13:39, Chris Hegarty  wrote:
> 
> On 15 Jun 2016, at 14:30, Claes Redestad  wrote:
>> 
>> Hi,
>> 
>> after VM.java was encapsulated and moved from sun.misc to jdk.internal.misc, 
>> the rationale for keeping a number of deprecated methods and constants no 
>> longer applies and these methods should be removed:
>> 
>> Webrev: http://cr.openjdk.java.net/~redestad/8159590/webrev.01/
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8159590
> 
> I had forgotten that there was a public API, ThreadGroup, that exposed some
> of this functionality.  Sadly I think that we may have to keep this, for now, 
> and 
> remove it in 10.
> 
> ThreadGroup::allowThreadSuspension should, however, have its @Deprecated
> annotation updated with ‘forRemoval = true’, then removed early in 10.
> 
> I can’t remember if this was on Stuarts list or not, but I think it is ok to 
> do it
> separately anyway.
> 
> -Chris.



Re: RFR: 8159590: Remove deprecated methods from jdk.internal.misc.VM

2016-06-16 Thread Chris Hegarty
On 15 Jun 2016, at 14:30, Claes Redestad  wrote:
> 
> Hi,
> 
> after VM.java was encapsulated and moved from sun.misc to jdk.internal.misc, 
> the rationale for keeping a number of deprecated methods and constants no 
> longer applies and these methods should be removed:
> 
> Webrev: http://cr.openjdk.java.net/~redestad/8159590/webrev.01/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8159590

I had forgotten that there was a public API, ThreadGroup, that exposed some
of this functionality.  Sadly I think that we may have to keep this, for now, 
and 
remove it in 10.

ThreadGroup::allowThreadSuspension should, however, have its @Deprecated
annotation updated with ‘forRemoval = true’, then removed early in 10.

I can’t remember if this was on Stuarts list or not, but I think it is ok to do 
it
separately anyway.

-Chris.

Re: RFR: JDK-8146975 - NullPointerException in IIOPInputStream.inputClassFields

2016-06-16 Thread Mark Sheppard


OK grand so, will make that change ... thanks Chris

regards
Mark



On 16/06/2016 06:40, Chris Hegarty wrote:

On 15 Jun 2016, at 19:33, Mark Sheppard  wrote:

Hi,
please oblige and review the updated webrev:

http://cr.openjdk.java.net/~msheppar/8146975/jdk9/webrev.01/

I’m happy with the code changes here.

Trivially, an alternative stylistic option for the method declaration:

 private static Field getDeclaredField(final Class c,
   final String 
fieldName)
 throws PrivilegedActionException, NoSuchFieldException, 
SecurityException
 {

-Chris.




Question about signature polymorphic, the VM spec of Java 7+, and VarHandles

2016-06-16 Thread Uwe Schindler
Hi,

the Java VM spec tells you the following how to "detect" signature polymorphic 
methods in bytecode:

===
A method is signature polymorphic if and only if all of the following 
conditions hold :
- It is declared in the java.lang.invoke.MethodHandle class.
- It has a single formal parameter of type Object[].
- It has a return type of Object.
- It has the ACC_VARARGS and ACC_NATIVE flags set.

In Java SE 7, the only signature polymorphic methods are the invoke and 
invokeExact methods of the class java.lang.invoke.MethodHandle.

The Java Virtual Machine gives special treatment to signature polymorphic 
methods in the invokevirtual instruction (§invokevirtual), in order to effect 
invocation of a method handle. A method handle is a typed, directly executable 
reference to an underlying method, constructor, field, or similar low-level 
operation (§5.4.3.5), with optional transformations of arguments or return 
values. These transformations are quite general, and include such patterns as 
conversion, insertion, deletion, and substitution. See the java.lang.invoke 
package in the Java SE platform API for more information.
===

Now the question is: In java 9 the MethodHandle class is not the only one, it 
also added VerHandle to the list of classes that use signature polymorphic!

Now the problem I have: In the Javadocs it says: "Bytecode generators, 
including the compiler back end, are required to emit untransformed symbolic 
type descriptors for these methods. Tools which determine symbolic linkage are 
required to accept such untransformed descriptors, without reporting linkage 
errors."

I am writing a bytecode analysis tool that needs to detect signature 
polymorphic methods to have a special treatment with them when invoked in 
bytecode. In my opinion, the above spec is fine for Java 7 and Java 8, but 
breaks for Java 9, because also VarHandle was added. I think the spec should be 
a bit more clear how to detect those methods:
- Add a reflective getter on Method to check if its signature polymorphic
- Make the annotation in MethodHandle public and document it. Also change the 
spec to say: All native varargs methods with the *public* annotation on it are 
signature polymorphic.
- Change the spec to say: The above described flags and method desriptor are 
used to detect (no change), but anything below java.lang.invoke is also 
required. I don't really like that spec, because it may break again in future.

I don't like the first part of the spec that says: this signature + ACC_VARARGS 
+ ACC_NATIVE. A 3rd party tool could create a method with exactly that 
signature outside the JDK code, so I have to limit the package name. Or use the 
annotations! That’s my problem with the spec!

Whats the plan for the VM spec and "what's" the best way to detect if a method 
is signature polymorphic - that is also future proof?

In my code of forbiddenapis, see 
https://github.com/policeman-tools/forbidden-apis/pull/106, I implemented 
purely the Java VM 8 spec, just extending the class name matcher to "everything 
below java.lang.invoke", so it also works for VarHandles. But as said, this 
does not look like "future-proof".

Uwe

-
Uwe Schindler
uschind...@apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/