Re: Possible addition of a default 'getOne' method on Iterable?

2018-01-30 Thread Dave Brosius

Basically, but it's

annoying and ugly to write

most likely sub-optimal

has problems with things like synchronized collections


On 01/31/2018 12:28 AM, Zheka Kozlov wrote:

Isn't iterable.getOne() the same as iterable.iterator().next()?

2018-01-31 12:15 GMT+07:00 Dave Brosius >:


Greetings,


sorry if this has been asked before, but has there been any
consideration for adding a

default T getOne() {

    Iterator it = iterator();
    if (!it.hasNext()) {
    throw new NoSuchElementException();
    }

    return it.next();
}


on the Iterable interface?


It is often the case you have a collection of some sort (un
indexed, in this case), where you know there is only one value in
the collection, or you know for some attribute of all the objects
in the Iterable, all objects can be thought of as the same, and so
you just want to get any of the elements.

Having to craft this iterator code is annoying, and it would be
much nicer to be able to do

String s = mySet.getOne();

In addition to this, it is likely that most collections could
implement getOne() more optimally than using the standard iterator
approach.

Of course i am not stuck on the choice of the name 'getOne'
anything would do. examplar() ?  As we know, naming is always the
hardest part.

thoughts?
dave






Re: Possible addition of a default 'getOne' method on Iterable?

2018-01-30 Thread Zheka Kozlov
Isn't iterable.getOne() the same as iterable.iterator().next()?

2018-01-31 12:15 GMT+07:00 Dave Brosius :

> Greetings,
>
>
> sorry if this has been asked before, but has there been any consideration
> for adding a
>
> default T getOne() {
>
> Iterator it = iterator();
> if (!it.hasNext()) {
> throw new NoSuchElementException();
> }
>
> return it.next();
> }
>
>
> on the Iterable interface?
>
>
> It is often the case you have a collection of some sort (un indexed, in
> this case), where you know there is only one value in the collection, or
> you know for some attribute of all the objects in the Iterable, all objects
> can be thought of as the same, and so you just want to get any of the
> elements.
>
> Having to craft this iterator code is annoying, and it would be much nicer
> to be able to do
>
> String s = mySet.getOne();
>
> In addition to this, it is likely that most collections could implement
> getOne() more optimally than using the standard iterator approach.
>
> Of course i am not stuck on the choice of the name 'getOne' anything would
> do. examplar() ?  As we know, naming is always the hardest part.
>
> thoughts?
> dave
>
>


Possible addition of a default 'getOne' method on Iterable?

2018-01-30 Thread Dave Brosius

Greetings,


sorry if this has been asked before, but has there been any 
consideration for adding a


default T getOne() {

    Iterator it = iterator();
    if (!it.hasNext()) {
    throw new NoSuchElementException();
    }

    return it.next();
}


on the Iterable interface?


It is often the case you have a collection of some sort (un indexed, in 
this case), where you know there is only one value in the collection, or 
you know for some attribute of all the objects in the Iterable, all 
objects can be thought of as the same, and so you just want to get any 
of the elements.


Having to craft this iterator code is annoying, and it would be much 
nicer to be able to do


String s = mySet.getOne();

In addition to this, it is likely that most collections could implement 
getOne() more optimally than using the standard iterator approach.


Of course i am not stuck on the choice of the name 'getOne' anything 
would do. examplar() ?  As we know, naming is always the hardest part.


thoughts?
dave



Re: [JDK 11] RFR 8196211: Move two sun/nio/cs tests into OpenJDK

2018-01-30 Thread Amy Lu

Thank you Paul. Pushed as suggested :-)

Thanks,
Amy

On 30/01/2018 12:37 AM, Paul Sandoz wrote:

That’s better :-)

If you wanna make it static i suggest:

  s/inputFileName/INPUT_FILE_NAME

although there is little benefit here since all access is local to 
main. Up to you to make local or keep static. No need for another review.


Paul.

On Jan 29, 2018, at 4:17 AM, Amy Lu > wrote:


Updated on the inputFileName.
Please review:http://cr.openjdk.java.net/~amlu/8196211/webrev.01/

Thanks,
Amy
On 27/01/2018 2:32 AM, Paul Sandoz wrote:

Hi,

Quick observation.

EUCTWBufferBoundaryDecodeTest uses a different data file on windows and it uses 
the line separator as a trigger. Is it possible to better formalize this by 
passing in the argument for the file via jtreg?

Paul.



On Jan 25, 2018, at 11:45 PM, Amy Lu  wrote:

Please review the patch to move two sun/nio/cs tests into OpenJDK.

bug:https://bugs.openjdk.java.net/browse/JDK-8196211
webrev:http://cr.openjdk.java.net/~amlu/8196211/webrev.00/

Thanks,
Amy








Re: Collections.addAll: remove outdated performance advice and add a note about atomicity

2018-01-30 Thread Martin Buchholz
I tried to tackle this here:
http://openjdk.markmail.org/thread/eet2zd6ig3pfpv5g
and it's still on my TODO list but not likely to get to top spot soon.

On Tue, Jan 30, 2018 at 7:00 PM, Tagir Valeev  wrote:

> Hello!
>
> I suggest a patch for java.util.Collections#addAll JavaDoc:
>
> --- Collections.java2018-01-31 09:39:31.599107500 +0700
> +++ Collections.java.patched2018-01-31 09:51:11.929059600 +0700
> @@ -5406,4 +5406,8 @@
>   * The behavior of this convenience method is identical to that of
> - * {@code c.addAll(Arrays.asList(elements))}, but this method is
> likely
> - * to run significantly faster under most implementations.
> + * {@code c.addAll(Arrays.asList(elements))} except possible
> + * difference in intermediate state visibility for concurrent or
> + * synchronized collections. Calling this method does not guarantee
> + * that the intermediate state (some of elements are added) is
> invisible,
> + * even if the collection itself provides such guarantee for its
> + * {@link Collection#addAll(Collection)} method.
>   *
>
> First, currently it says that Collections#addAll is likely to run
> significantly faster. However it's only marginally faster for
> collections which delegate their addAll method to standard
> AbstractCollection#addAll implementation. Also it could be much slower
> for collections which have optimized addAll (like ArrayList,
> CopyOnWriteArrayList, ConcurrentLinkedDeque, etc.). I don't know a
> single example of collection where Collections#addAll is actually
> significantly faster. Also it says that the behavior is identical,
> while it's not. If, e.g. c is a collection returned from
> synchronizedCollection, then intermediate state of
> c.addAll(Arrays.asList(elements)) would not be visible under
> synchronized(c) in another thread. On the other hand, replacing such
> call with Collections.addAll(c, elements) (to make it "significantly
> faster") will lift this guarantee: now you can see partially added
> array.
>
> What do you think? Should I file an issue?
>
> With best regards,
> Tagir Valeev.
>


Collections.addAll: remove outdated performance advice and add a note about atomicity

2018-01-30 Thread Tagir Valeev
Hello!

I suggest a patch for java.util.Collections#addAll JavaDoc:

--- Collections.java2018-01-31 09:39:31.599107500 +0700
+++ Collections.java.patched2018-01-31 09:51:11.929059600 +0700
@@ -5406,4 +5406,8 @@
  * The behavior of this convenience method is identical to that of
- * {@code c.addAll(Arrays.asList(elements))}, but this method is likely
- * to run significantly faster under most implementations.
+ * {@code c.addAll(Arrays.asList(elements))} except possible
+ * difference in intermediate state visibility for concurrent or
+ * synchronized collections. Calling this method does not guarantee
+ * that the intermediate state (some of elements are added) is invisible,
+ * even if the collection itself provides such guarantee for its
+ * {@link Collection#addAll(Collection)} method.
  *

First, currently it says that Collections#addAll is likely to run
significantly faster. However it's only marginally faster for
collections which delegate their addAll method to standard
AbstractCollection#addAll implementation. Also it could be much slower
for collections which have optimized addAll (like ArrayList,
CopyOnWriteArrayList, ConcurrentLinkedDeque, etc.). I don't know a
single example of collection where Collections#addAll is actually
significantly faster. Also it says that the behavior is identical,
while it's not. If, e.g. c is a collection returned from
synchronizedCollection, then intermediate state of
c.addAll(Arrays.asList(elements)) would not be visible under
synchronized(c) in another thread. On the other hand, replacing such
call with Collections.addAll(c, elements) (to make it "significantly
faster") will lift this guarantee: now you can see partially added
array.

What do you think? Should I file an issue?

With best regards,
Tagir Valeev.


Re: [PATCH] GPU Exploitation Infrastructure

2018-01-30 Thread David Holmes

Project Sumatra was looking at GPU use:

https://wiki.openjdk.java.net/display/Sumatra/Main

It's inactive though.

David

On 31/01/2018 7:46 AM, David Holmes wrote:

On 31/01/2018 1:35 AM, Alan Bateman wrote:

On 30/01/2018 13:55, Ben Walsh wrote:
This patch provides the infrastructure to enable the exploitation of 
a GPU

by the compiler to accelerate certain suitable Java constructs.

When enabled, a suitable compiler can attempt to accelerate the 
following

Java constructs by launching the corresponding lambda expression on the
GPU:

    IntStream.range().parallel().forEach()
    IntStream.rangeClosed().parallel().forEach()

    where:

    defines upper and lower bounds
    is a correctly defined lambda expression

As it stands, with the HotSpot compiler, this patch performs a "no 
op" in

the newly added in-built native library method.
This can be extended so that the HotSpot compiler attempts the
acceleration detailed above instead.

I would like to pair with a sponsor to contribute this patch ...
The function prototypes in jvm.h are JVM_* functions rather than JNI 
native method names. You may want to look at adding a JNI function to 
libjava with a wrapper or else use RegisterNatives to map the native 
method name to a JVM_* function.


BTW: Is this something for one the project repos (Valhalla or Panama) 
rather the main line (jdk/jdk)?


This should be looked at in the context of Panama I think, though IIRC 
there was a distinct GPU related project as well? Either way GPU support 
in mainline doesn't seem like something to just be dropped in like this 
without much more higher-level discussion taking place.


Plus hotspot changes - even stubs - must be reviewed on the appropriate 
hotspot mailing list.


David


-Alan


JDK-8166339, Code conversion working behavior was changed for x-IBM834

2018-01-30 Thread Xueming Shen

Hi,

Please help review the change for JDK-8166339.

issue: https://bugs.openjdk.java.net/browse/JDK-8166339
webrev: http://cr.openjdk.java.net/~sherman/8166339/webrev

This is a regression triggered by

issue: https://bugs.openjdk.java.net/browse/JDK-8008386
webrev: http://cr.openjdk.java.net/~sherman/8008386/webrev

which updated the doublebyte decoder implementation to handle unmappable
bytes more "appropriately" ( malformed (1) or unmappable(2) ).

However in case of Cp834, which is a doublebyte-only charset, the unmappable
bytes should always be treated as double-byte pair, therefor always 
unmappable(2).


Thanks
Sherman


RFR 8195059: Update java.net Socket and DatagramSocket implementations to use Cleaner

2018-01-30 Thread Roger Riggs
Please review changes to replace finalizers in socket, datagram, and 
multicast networking
with Cleaner based release of the raw file descriptors.  Each 
FileDescriptor is registered
for cleanup after the raw fd (or handle) is assigned.  Normal calls to 
close unregister the
cleaner before using the current logic to close the raw fd/handle. 
Windows networking

uses fd's with the Windows socket_ API requiring a special cased Cleanable.

The tests check that the implementation objects including 
FileDescriptors are reclaimed

and for Linux that the raw fd counts are reduced as expected.

Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-net-cleanup-8195059/

Thanks, Roger




Re: [PATCH] GPU Exploitation Infrastructure

2018-01-30 Thread David Holmes

On 31/01/2018 1:35 AM, Alan Bateman wrote:

On 30/01/2018 13:55, Ben Walsh wrote:
This patch provides the infrastructure to enable the exploitation of a 
GPU

by the compiler to accelerate certain suitable Java constructs.

When enabled, a suitable compiler can attempt to accelerate the following
Java constructs by launching the corresponding lambda expression on the
GPU:

    IntStream.range().parallel().forEach()
    IntStream.rangeClosed().parallel().forEach()

    where:

    defines upper and lower bounds
    is a correctly defined lambda expression

As it stands, with the HotSpot compiler, this patch performs a "no op" in
the newly added in-built native library method.
This can be extended so that the HotSpot compiler attempts the
acceleration detailed above instead.

I would like to pair with a sponsor to contribute this patch ...
The function prototypes in jvm.h are JVM_* functions rather than JNI 
native method names. You may want to look at adding a JNI function to 
libjava with a wrapper or else use RegisterNatives to map the native 
method name to a JVM_* function.


BTW: Is this something for one the project repos (Valhalla or Panama) 
rather the main line (jdk/jdk)?


This should be looked at in the context of Panama I think, though IIRC 
there was a distinct GPU related project as well? Either way GPU support 
in mainline doesn't seem like something to just be dropped in like this 
without much more higher-level discussion taking place.


Plus hotspot changes - even stubs - must be reviewed on the appropriate 
hotspot mailing list.


David


-Alan


Re: [PATCH] Inefficient ArrayList.subList().toArray()

2018-01-30 Thread Martin Buchholz
On Mon, Jan 29, 2018 at 1:02 PM, Martin Buchholz 
wrote:

> I'm going to expedite this a little since we are building further changes
> on top.
>
> 8196207: Inefficient ArrayList.subList().toArray()
> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-
> integration/ArrayList-subList-toArray/index.html
> https://bugs.openjdk.java.net/browse/JDK-8196207
>

 8196207 was pushed, ready for Claes or jsr166 team to add to.


Re: JDK-8196298: Add null Reader and Writer

2018-01-30 Thread Patrick Reinhart


> Am 30.01.2018 um 16:02 schrieb Alan Bateman :
> 
> On 29/01/2018 22:55, Brian Burkhalter wrote:
>> I made a few minor changes to the CSR [1] verbiage. Probably the 
>> nullWriter() documentation should also be updated to explicitly mention all 
>> variants of both append() and write().
>> 
> It could although limiting it to the abstract methods should be fine too as 
> the append methods are specified in terms of the write methods.
> 
> One other micro detail is that Reader/Writer support implementations 
> specifying the object to use for synchronization. This isn't interesting for 
> the Reader/Writer objects returned by the proposed static methods so a 
> one-line that says that the object used to synchronize operations is not 
> specified might help. It would avoid something synchronizing on the reader or 
> writer and expecting operations read/write methods to block

Is that a difference from the null InputStream/OutputStream here?

-Patrick



Re: RFR 8181594: Efficient and constant-time modular arithmetic

2018-01-30 Thread Adam Petcher

+core-libs-dev


On 1/26/2018 4:06 PM, Adam Petcher wrote:

JBS: https://bugs.openjdk.java.net/browse/JDK-8181594
Webrev: http://cr.openjdk.java.net/~apetcher/8181594/webrev.00/

This is a code review for the field arithmetic that will be used in 
implementations of X25519/X448 key agreement, the Poly1305 
authenticator, and EdDSA signatures. I believe that the library has 
all the features necessary for X25519/X448 and Poly1305, and I expect 
at most a couple of minor enhancements will be required to support 
EdDSA. There is no public API for this library, so we can change it in 
the future to suit the needs of new algorithms without breaking 
compatibility with external code. Still, I made an attempt to clearly 
structure and document the (internal) API, and I want to make sure it 
is understandable and easy to use.


This is not a general-purpose modular arithmetic library. It will only 
work well in circumstances where the sequence of operations is 
restricted, and where the prime that defines the field has some useful 
structure. Moreover, each new field will require some field-specific 
code that takes into account the structure of the prime and the way 
the field is used in the application. The initial implementation 
includes a field for Poly1305 and the fields for X25519/X448 which 
should also work for EdDSA.


The benefits of using this library are that it is much more efficient 
than using similar operations in BigInteger. Also, many operations are 
branch-free, making them suitable for use in a side-channel resistant 
implementation that does not branch on secrets.


To provide some context, I have attached a code snippet describing how 
this library can be used. The snippet is the constant-time Montgomery 
ladder from my X25519/X448 implementation, which I expect to be out 
for review soon. X25519/X448 only uses standard arithmetic operations, 
and the more unusual features (e.g. add modulo a power of 2) are 
needed by Poly1305.


The field arithmetic (for all fields) is implemented using a 32-bit 
representation similar to the one described in the Ed448 paper[1] (in 
the "Implementation on 32-bit platforms" section). Though my 
implementation uses signed limbs, and grade-school multiplication 
instead of Karatsuba. The argument for correctness is essentially the 
same for all three fields: the magnitude of each 64-bit limb is at 
most 2^(k-1) after reduction, except for the last limb which may have 
a magnitude of up to 2^k. The values of k are between 26 to 28 
(depending on the field), and we can calculate that the maximum 
magnitude for any limb during an add-multiply-carry-reduce sequence is 
always less than 2^63. Therefore, no overflow occurs and all 
operations are correct.


Process note: this enhancement is part of JEP 324 (Key Agreement with 
Curve25519 and Curve448). When this code review is complete, nothing 
will happen until all other work for this JEP is complete, and the JEP 
is accepted as part of some release. This means that this code will be 
pushed to the repo along with the X25519/X448 code that uses it.


[1] https://eprint.iacr.org/2015/625.pdf





private IntegerModuloP_Base pointMultiply(byte[] k, IntegerModuloP u){

IntegerModuloP x_1 = u;
MutableIntegerModuloP x_2 = one.mutable();
MutableIntegerModuloP z_2 = zero.mutable();
MutableIntegerModuloP x_3 = u.mutable();
MutableIntegerModuloP z_3 = one.mutable();
int swap = 0;

// Variables below are reused to avoid unnecessary allocation
// They will be assigned in the loop, so initial value doesn't matter
MutableIntegerModuloP m1 = zero.mutable();
MutableIntegerModuloP DA = zero.mutable();
MutableIntegerModuloP E = zero.mutable();
MutableIntegerModuloP a24_times_E = zero.mutable();

for(int t = params.getBits() - 1; t >= 0; t--){
int k_t = bitAt(k, t);
swap = swap ^ k_t;
x_2.conditionalSwapWith(x_3, swap);
z_2.conditionalSwapWith(z_3, swap);
swap = k_t;

// A(m1) = x_2 + z_2
m1.setValue(x_2).setSum(z_2);
// D = x_3 - z_3
// DA = D * A(m1)
DA.setValue(x_3).setDifference(z_3).setProduct(m1);
// AA(m1) = A(m1)^2
m1.setSquare();
// B(x_2) = x_2 - z_2
x_2.setDifference(z_2);
// C = x_3 + z_3
// CB(x_3) = C * B(x_2)
x_3.setSum(z_3).setProduct(x_2);
// BB(x_2) = B^2
x_2.setSquare();
// E = AA(m1) - BB(x_2)
E.setValue(m1).setDifference(x_2);
// compute a24 * E using SmallValue
a24_times_E.setValue(E);
a24_times_E.setProduct(a24);

// assign results to x_3, z_3, x_2, z_2
// x_2 = AA(m1) * BB
x_2.setProduct(m1);
// z_2 = E * (AA(m1) + a24 * E)

Re: RFR: 8196331: Optimize Character.digit for latin1 input

2018-01-30 Thread Paul Sandoz
OK! A surprising increase in code size. Thanks for checking.

Paul.

> On Jan 30, 2018, at 2:40 AM, Claes Redestad  wrote:
> 
> The ASM is harder than usual to follow and compare since everything is
> inlined aggressively, but it seems that since CharacterDataLatin1 is only
> invoked for 0 <= ch < 256 (invariant established in CharacterData.of(int ch))
> then the compiler is able to elide bounds check entirely when the byte[] is
> also 256 elements.
> 
> Shrinking the array adds more branches and grows the compiled code size
> for UUID.fromString from 751 to 1341 bytes, so it seems that even from a
> footprint perspective then keeping the array at 256 elements is a win. :-)
> 
> /Claes
> 
> On 2018-01-29 22:04, Claes Redestad wrote:
>> Right, I can't really explain why, but the effect is very visible and
>> reproducible in microbenchmarks. Differences in generated ASM might
>> be indicating that the inlining behavior changes enough to shift the
>> result around; maybe a job for @ForceInline?
>> 
>> I'll experiment and analyze a bit more tomorrow to see if I can find a
>> good explanation for the observed difference and/or a solution that
>> allows us to slim down the lookup array.
>> 
>> /Claes
>> 
>> On 2018-01-29 20:38, Paul Sandoz wrote:
>>> Smaller in only the upper bound? I would an explicit upper bounds check 
>>> would dominate that of the bounds check for the array itself.
>>> 
>>> Paul.
>>> 
 On Jan 29, 2018, at 11:37 AM, Claes Redestad > wrote:
 
 I ran with a smaller byte[] initially and saw about a 10% improvement from 
 removing the branch, which is why I felt the superfluous bytes were 
 motivated.
 
 /Claes
 
 Paul Sandoz > 
 skrev: (29 januari 2018 19:14:44 CET)
 
 
 On Jan 29, 2018, at 9:44 AM, Martin Buchholz
 > wrote:
 Thanks. I might try to shrink the size of the static array,
 by only storing values between '0' and 'z', at the cost of
 slightly greater lookup costs for each char.
 
 I was wondering the same, or just clip the end of the array to’z'
 
 if (ch <= ‘z’ && radix …) { // Might even fold the upper bounds check 
 for DIGITS
value = DIGITS[ch];
...
 }
 
 Paul.
 
 On Mon, Jan 29, 2018 at 3:15 AM, Claes Redestad
 > wrote:
 
 Hi, for the latin1 block of CharacterData we can improve
 the digit(int, int) implementation by providing an
 optimized lookup table. This improves microbenchmarks
 exercising Integer.parseInt, Long.parseLong and
 UUID.fromString etc by around 50%for typical inputs.
 Webrev:
 http://cr.openjdk.java.net/~redestad/8196331/open.00/
 
 Bug: https://bugs.openjdk.java.net/browse/JDK-8196331 The
 lookup array is pre-calculated to minimize startup impact
 (adds 1,027 bytecodes executed during initialization) /Claes
 
 
 -- 
 Sent from my Android device with K-9 Mail. Please excuse my brevity.
>>> 
>> 
> 



Re: [PATCH] GPU Exploitation Infrastructure

2018-01-30 Thread Alan Bateman

On 30/01/2018 13:55, Ben Walsh wrote:

This patch provides the infrastructure to enable the exploitation of a GPU
by the compiler to accelerate certain suitable Java constructs.

When enabled, a suitable compiler can attempt to accelerate the following
Java constructs by launching the corresponding lambda expression on the
GPU:

IntStream.range().parallel().forEach()
IntStream.rangeClosed().parallel().forEach()

where:

defines upper and lower bounds
is a correctly defined lambda expression

As it stands, with the HotSpot compiler, this patch performs a "no op" in
the newly added in-built native library method.
This can be extended so that the HotSpot compiler attempts the
acceleration detailed above instead.

I would like to pair with a sponsor to contribute this patch ...
The function prototypes in jvm.h are JVM_* functions rather than JNI 
native method names. You may want to look at adding a JNI function to 
libjava with a wrapper or else use RegisterNatives to map the native 
method name to a JVM_* function.


BTW: Is this something for one the project repos (Valhalla or Panama) 
rather the main line (jdk/jdk)?


-Alan


Re: RFR: 8011697(ScriptEngine "js" randomly means either "rhino" or "nashorn", but should instead select one)

2018-01-30 Thread Sundararajan Athijegannathan

+1

(with the copyright header change suggested by Alan)

-Sundar

On 30/01/18, 8:28 PM, Alan Bateman wrote:

On 30/01/2018 09:17, Srinivas Dama wrote:

Hi,

Please review the revised webrev at 
http://cr.openjdk.java.net/~sdama/8011697/webrev.01/  for  
https://bugs.openjdk.java.net/browse/JDK-8011697




The updated patch to ScriptEngineManager looks okay.

Can you replace the copyright header in the tests with the GPL version 
before you push this? We don't use the GPL + "Classpath" exception 
header in tests.


-Alan


Re: RFR JDK-8176379: java.util.Base64 mime encoder behaves incorrectly if initialized with a line length of size 1-3

2018-01-30 Thread Roger Riggs

Hi,

Base64GetEncoderTest does not need to test the negative values more than 
once.


Otherwise, looks fine.

Roger


On 1/30/2018 9:31 AM, Alan Bateman wrote:

On 29/01/2018 17:52, Xueming Shen wrote:


Actually I have a dedicated test case for "mime encoder maxlen".
*test/jdk/java/util/Base64/Base64GetEncoderTest.java*
I have updated it to test the negative and the < 4 maxlen.

http://cr.openjdk.java.net/~sherman/8176379/webrev/

webrev has been updated according.

Looks good.

-Alan




Re: RFR: 8196331: Optimize Character.digit for latin1 input

2018-01-30 Thread Xueming Shen
+1

On Jan 30, 2018, at 2:40 AM, Claes Redestad  wrote:

The ASM is harder than usual to follow and compare since everything is
inlined aggressively, but it seems that since CharacterDataLatin1 is only
invoked for 0 <= ch < 256 (invariant established in CharacterData.of(int ch))
then the compiler is able to elide bounds check entirely when the byte[] is
also 256 elements.

Shrinking the array adds more branches and grows the compiled code size
for UUID.fromString from 751 to 1341 bytes, so it seems that even from a
footprint perspective then keeping the array at 256 elements is a win. :-)

/Claes

> On 2018-01-29 22:04, Claes Redestad wrote:
> Right, I can't really explain why, but the effect is very visible and
> reproducible in microbenchmarks. Differences in generated ASM might
> be indicating that the inlining behavior changes enough to shift the
> result around; maybe a job for @ForceInline?
> 
> I'll experiment and analyze a bit more tomorrow to see if I can find a
> good explanation for the observed difference and/or a solution that
> allows us to slim down the lookup array.
> 
> /Claes
> 
>> On 2018-01-29 20:38, Paul Sandoz wrote:
>> Smaller in only the upper bound? I would an explicit upper bounds check 
>> would dominate that of the bounds check for the array itself.
>> 
>> Paul.
>> 
>>> On Jan 29, 2018, at 11:37 AM, Claes Redestad >> > wrote:
>>> 
>>> I ran with a smaller byte[] initially and saw about a 10% improvement from 
>>> removing the branch, which is why I felt the superfluous bytes were 
>>> motivated.
>>> 
>>> /Claes
>>> 
>>> Paul Sandoz > skrev: 
>>> (29 januari 2018 19:14:44 CET)
>>> 
>>> 
>>> On Jan 29, 2018, at 9:44 AM, Martin Buchholz
>>> > wrote:
>>> Thanks. I might try to shrink the size of the static array,
>>> by only storing values between '0' and 'z', at the cost of
>>> slightly greater lookup costs for each char.
>>> 
>>> I was wondering the same, or just clip the end of the array to’z'
>>> 
>>> if (ch <= ‘z’ && radix …) { // Might even fold the upper bounds check 
>>> for DIGITS
>>>value = DIGITS[ch];
>>>...
>>> }
>>> 
>>> Paul.
>>> 
>>> On Mon, Jan 29, 2018 at 3:15 AM, Claes Redestad
>>> >> > wrote:
>>> 
>>> Hi, for the latin1 block of CharacterData we can improve
>>> the digit(int, int) implementation by providing an
>>> optimized lookup table. This improves microbenchmarks
>>> exercising Integer.parseInt, Long.parseLong and
>>> UUID.fromString etc by around 50%for typical inputs.
>>> Webrev:
>>> http://cr.openjdk.java.net/~redestad/8196331/open.00/
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8196331 The
>>> lookup array is pre-calculated to minimize startup impact
>>> (adds 1,027 bytecodes executed during initialization) /Claes
>>> 
>>> 
>>> -- 
>>> Sent from my Android device with K-9 Mail. Please excuse my brevity.
>> 
> 




Re: RFR: 8011697(ScriptEngine "js" randomly means either "rhino" or "nashorn", but should instead select one)

2018-01-30 Thread Alan Bateman

On 30/01/2018 09:17, Srinivas Dama wrote:

Hi,

Please review the revised webrev at 
http://cr.openjdk.java.net/~sdama/8011697/webrev.01/  for  
https://bugs.openjdk.java.net/browse/JDK-8011697



The updated patch to ScriptEngineManager looks okay.

Can you replace the copyright header in the tests with the GPL version 
before you push this? We don't use the GPL + "Classpath" exception 
header in tests.


-Alan


Re: JDK-8196298: Add null Reader and Writer

2018-01-30 Thread Alan Bateman

On 29/01/2018 22:55, Brian Burkhalter wrote:

I made a few minor changes to the CSR [1] verbiage. Probably the nullWriter() 
documentation should also be updated to explicitly mention all variants of both 
append() and write().

It could although limiting it to the abstract methods should be fine too 
as the append methods are specified in terms of the write methods.


One other micro detail is that Reader/Writer support implementations 
specifying the object to use for synchronization. This isn't interesting 
for the Reader/Writer objects returned by the proposed static methods so 
a one-line that says that the object used to synchronize operations is 
not specified might help. It would avoid something synchronizing on the 
reader or writer and expecting operations read/write methods to block.


-Alan


Re: RFR JDK-8176379: java.util.Base64 mime encoder behaves incorrectly if initialized with a line length of size 1-3

2018-01-30 Thread Alan Bateman

On 29/01/2018 17:52, Xueming Shen wrote:


Actually I have a dedicated test case for "mime encoder maxlen".
*test/jdk/java/util/Base64/Base64GetEncoderTest.java*
I have updated it to test the negative and the < 4 maxlen.

http://cr.openjdk.java.net/~sherman/8176379/webrev/

webrev has been updated according.

Looks good.

-Alan


[PATCH] GPU Exploitation Infrastructure

2018-01-30 Thread Ben Walsh
This patch provides the infrastructure to enable the exploitation of a GPU 
by the compiler to accelerate certain suitable Java constructs.

When enabled, a suitable compiler can attempt to accelerate the following 
Java constructs by launching the corresponding lambda expression on the 
GPU:

   IntStream.range().parallel().forEach()
   IntStream.rangeClosed().parallel().forEach()

   where:

   defines upper and lower bounds
   is a correctly defined lambda expression

As it stands, with the HotSpot compiler, this patch performs a "no op" in 
the newly added in-built native library method.
This can be extended so that the HotSpot compiler attempts the 
acceleration detailed above instead.

I would like to pair with a sponsor to contribute this patch ...


--

diff -r fd237da7a113 make/hotspot/symbols/symbols-unix
--- a/make/hotspot/symbols/symbols-unix Mon Jan 22 23:06:29 2018 -0800
+++ b/make/hotspot/symbols/symbols-unix Tue Jan 30 10:07:18 2018 +
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2016, 2017, Oracle and/or its affiliates. All rights 
reserved.
+# Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights 
reserved.
 # DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 #
 # This code is free software; you can redistribute it and/or modify it
@@ -187,3 +187,6 @@
 JVM_AddReadsModule
 JVM_DefineModule
 JVM_SetBootLoaderUnnamedModule
+
+# GPU exploitation support
+Java_java_util_stream_IntPipeline_promoteGPUCompile
diff -r fd237da7a113 src/hotspot/share/include/jvm.h
--- a/src/hotspot/share/include/jvm.h   Mon Jan 22 23:06:29 2018 -0800
+++ b/src/hotspot/share/include/jvm.h   Tue Jan 30 10:07:18 2018 +
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights 
reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1189,6 +1189,12 @@
 JNIEXPORT jstring JNICALL
 JVM_GetTemporaryDirectory(JNIEnv *env);
 
+/*
+ * GPU exploitation support
+ */
+JNIEXPORT void JNICALL
+Java_java_util_stream_IntPipeline_promoteGPUCompile(JNIEnv *env, jobject 
obj);
+
 /* Generics reflection support.
  *
  * Returns information about the given class's EnclosingMethod
diff -r fd237da7a113 src/hotspot/share/prims/jvm.cpp
--- a/src/hotspot/share/prims/jvm.cpp   Mon Jan 22 23:06:29 2018 -0800
+++ b/src/hotspot/share/prims/jvm.cpp   Tue Jan 30 10:07:18 2018 +
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights 
reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -3661,3 +3661,11 @@
 JVM_ENTRY_NO_ENV(jint, JVM_FindSignal(const char *name))
   return os::get_signal_number(name);
 JVM_END
+
+
+// GPU exploitation support stub 
/
+
+JVM_ENTRY(void, 
Java_java_util_stream_IntPipeline_promoteGPUCompile(JNIEnv *env, jobject 
thisObj))
+  JVMWrapper("Java_java_util_stream_IntPipeline_promoteGPUCompile");
+  return;
+JVM_END
diff -r fd237da7a113 
src/java.base/share/classes/java/util/stream/AbstractPipeline.java
--- a/src/java.base/share/classes/java/util/stream/AbstractPipeline.java 
Mon Jan 22 23:06:29 2018 -0800
+++ b/src/java.base/share/classes/java/util/stream/AbstractPipeline.java 
Tue Jan 30 10:07:18 2018 +
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2016, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2012, 2018, Oracle and/or its affiliates. All rights 
reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -373,6 +373,14 @@
 return sourceStage.parallel;
 }
 
+/**
+ * Returns the sourceSpliterator
+ *
+ * @return the sourceSpliterator
+ */
+final Spliterator getSourceSpliterator() {
+   return sourceSpliterator;
+}
 
 /**
  * Returns the composition of stream flags of the stream source and 
all
diff -r fd237da7a113 
src/java.base/share/classes/java/util/stream/IntPipeline.java
--- a/src/java.base/share/classes/java/util/stream/IntPipeline.java Mon 
Jan 22 23:06:29 2018 -0800
+++ b/src/java.base/share/classes/java/util/stream/IntPipeline.java Tue 
Jan 30 10:07:18 2018 +
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2017, Oracle and/or its affiliates. All rights 
reserved.
+ * Copyright (c) 2012, 2018, Oracle and/or its affiliates. All rights 
reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -433,10 +433,33 

Re: RFR: 8196331: Optimize Character.digit for latin1 input

2018-01-30 Thread Claes Redestad

The ASM is harder than usual to follow and compare since everything is
inlined aggressively, but it seems that since CharacterDataLatin1 is only
invoked for 0 <= ch < 256 (invariant established in CharacterData.of(int 
ch))

then the compiler is able to elide bounds check entirely when the byte[] is
also 256 elements.

Shrinking the array adds more branches and grows the compiled code size
for UUID.fromString from 751 to 1341 bytes, so it seems that even from a
footprint perspective then keeping the array at 256 elements is a win. :-)

/Claes

On 2018-01-29 22:04, Claes Redestad wrote:

Right, I can't really explain why, but the effect is very visible and
reproducible in microbenchmarks. Differences in generated ASM might
be indicating that the inlining behavior changes enough to shift the
result around; maybe a job for @ForceInline?

I'll experiment and analyze a bit more tomorrow to see if I can find a
good explanation for the observed difference and/or a solution that
allows us to slim down the lookup array.

/Claes

On 2018-01-29 20:38, Paul Sandoz wrote:
Smaller in only the upper bound? I would an explicit upper bounds 
check would dominate that of the bounds check for the array itself.


Paul.

On Jan 29, 2018, at 11:37 AM, Claes Redestad 
> wrote:


I ran with a smaller byte[] initially and saw about a 10% 
improvement from removing the branch, which is why I felt the 
superfluous bytes were motivated.


/Claes

Paul Sandoz > 
skrev: (29 januari 2018 19:14:44 CET)



    On Jan 29, 2018, at 9:44 AM, Martin Buchholz
    > wrote:
    Thanks. I might try to shrink the size of the static array,
    by only storing values between '0' and 'z', at the cost of
    slightly greater lookup costs for each char.

    I was wondering the same, or just clip the end of the array to’z'

    if (ch <= ‘z’ && radix …) { // Might even fold the upper bounds 
check for DIGITS

   value = DIGITS[ch];
   ...
    }

    Paul.

    On Mon, Jan 29, 2018 at 3:15 AM, Claes Redestad
    > wrote:

    Hi, for the latin1 block of CharacterData we can improve
    the digit(int, int) implementation by providing an
    optimized lookup table. This improves microbenchmarks
    exercising Integer.parseInt, Long.parseLong and
    UUID.fromString etc by around 50%for typical inputs.
    Webrev:
http://cr.openjdk.java.net/~redestad/8196331/open.00/

    Bug: https://bugs.openjdk.java.net/browse/JDK-8196331 The
    lookup array is pre-calculated to minimize startup impact
    (adds 1,027 bytecodes executed during initialization) 
/Claes



--
Sent from my Android device with K-9 Mail. Please excuse my brevity.








Re: [8u-dev] RFR - 8156824: com.sun.jndi.ldap.pool.PoolCleaner should clear its context class loader

2018-01-30 Thread Chris Hegarty

> On 26 Jan 2018, at 16:40, Rob McKenna  wrote:
> 
> Hi folks,
> 
> I'd like to backport this change to 8u-dev.
> 
> The changes are straightforward enough but 8u needs some changes that
> were made to InnocuousThread. (strictly it doesn't need all of the
> changes I've made but I've made an effort to make the code look as close
> to 9 as possible)
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8156824
> 9u changeset: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/22e704dfa05c
> 8u webrev: http://cr.openjdk.java.net/~robm/8156824/webrev.01/

The changes looks ok to me.  Thanks Rob.

-Chris.


Re: RFR: 8196331: Optimize Character.digit for latin1 input

2018-01-30 Thread Claes Redestad

Hi,

not sure what you're suggesting, exactly, but it seems Charset coders 
generate
String literals which are then unpacked to char[]/byte[]'s in static 
blocks.


While constructing arrays from String literals might have been a startup
optimization in the past, it is now mainly a workaround to method bytecode
limits, see https://bugs.openjdk.java.net/browse/JDK-8185104

/Claes

On 2018-01-30 01:10, Ulf Zibis wrote:

Hi,

you may can construct the lookup table as a static String constant to 
slim down the footprint and treat it as a byte[] as we do in the 
Charset coders.


-Ulf


Am 29.01.2018 um 22:04 schrieb Claes Redestad:

I'll experiment and analyze a bit more tomorrow to see if I can find a
good explanation for the observed difference and/or a solution that
allows us to slim down the lookup array.






RE: RFR: 8011697(ScriptEngine "js" randomly means either "rhino" or "nashorn", but should instead select one)

2018-01-30 Thread Srinivas Dama
Hi,

Please review the revised webrev at 
http://cr.openjdk.java.net/~sdama/8011697/webrev.01/  for  
https://bugs.openjdk.java.net/browse/JDK-8011697

Regards,
Srinivas

-Original Message-
From: Alan Bateman 
Sent: Monday, December 11, 2017 10:05 PM
To: Srinivas Dama ; core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8011697(ScriptEngine "js" randomly means either "rhino" or 
"nashorn", but should instead select one)

On 11/12/2017 15:26, Srinivas Dama wrote:
> Hi,
>
> Please review http://cr.openjdk.java.net/~sdama/8011697/webrev.00/
> for https://bugs.openjdk.java.net/browse/JDK-8011697
>
> Fix is to make sure ScriptEngineManager always returns a particular engine on 
> all platforms consistently.
>
I assume using a TreeSet would work too. A different approach is replace 
engineSpis with nameToFactory and extensionToFactory maps so that the 
getEngineByXXX methods don't need to a sequential search.

-Alan