Re: RFR(JDK11/NIO) 8205058 throw CharacterCodingException --> Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for reading/writing a string from/to a file

[Xueming Shen]

Hi Joe,

I would suggest always embed a malformed exception into the iae as showed
below, then the extra flag is no longer needed.

private static void throwMalformed(int off, int nb) {
  String msg = "malformed input off : " + off + ", length : " + nb;
  throw new IllegalArgumentException(msg, new 
MalformedInputException(nb));

}

It's an implementation details, so we might be able to get rid of the 
iae later if we
can figure out the better way to pass the malformed exception for both 
ZipCoder/Files

later, without touch the api.

An alternative is to move the iae-catch into StringCoding, with pair of 
similar methods
to throw IOE (to add into the JLA), not sure if it's better though. It 
makes the Files

impl cleaner at least.

-Sherman

On 6/25/18, 9:50 PM, Joe Wang wrote:

Hi Alan, Sherman,

Here's a version where we, as Sherman suggested, throw an IAE with CCE 
as the cause. This approach reduces code duplication in SC, although 
it complicates the impl a little bit with the added parameter and the 
different behavior between the existing usages of the methods and the 
new ones. The existing code paths are kept intact so there's no 
compatibility issue for the existing code.


This version also did not remove the try-catch in Files as Alan 
suggested earlier.


http://cr.openjdk.java.net/~joehw/jdk11/8205058/webrev02/

Thanks,
Joe

On 6/25/18, 3:41 PM, Joe Wang wrote:

Hi Alan,

The test testMalformedRead and testMalformedWrite now expect an 
UnmappableCharacterException instead of IOE:


webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8205058/webrev01/

Thanks,
Joe

On 6/25/18, 9:48 AM, Joe Wang wrote:



On 6/24/18, 12:11 PM, Alan Bateman wrote:

On 20/06/2018 04:32, Joe Wang wrote:
Thanks Alan.  I created 8205058 to capture your suggestions. 
Please see below for more details.


Changed the internal APIs to throw CCE instead. In the same way as 
the previous changeset for 8201276, these methods are made 
specific for the use cases (though they are now for 
Files.read/writeString only) so that they are not mixed up with 
existing ones that may inadvertently affect other usages.


One thing to note is that MalformedInputException or 
UnmappableCharacterException will lose one piece of information in 
comparison to the existing IAE, that is where it happens (offset). 
Should there be an improvement in the future, we could consider 
add it back to this part of code.


JBS: https://bugs.openjdk.java.net/browse/JDK-8205058
webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8205058/webrev/


I see your point on MalformedInputException and 
UnmappableCharacterException so maybe we can submit a new issue to 
track the follow-on work.


Submitted: https://bugs.openjdk.java.net/browse/JDK-8205613



The update means a lot of duplication in StringCoding. Did you 
consider (or measure) having the private encode/decode methods take 
a parameter to indicate the exception handling? Sherman might have 
suggestions on this.


I did. I didn't like the code duplication at all. But it would be 
even messier if we reuse the code since the previous usage didn't 
declare checked exception, but did capture the position (offset) and 
length information, which means the new method while unnecessary to 
capture these information for Files read/writeString would still 
need to save them in case Exception occurs. Because of the 
complication, I thought you and Sherman would again prefer a 
specific methods than adding parameters (need fields as well).


Furthermore, in the first round of the review, Sherman mentioned 
that he's been working on an improvement in this area. But I'll wait 
till Sherman could comment on this.




In the tests, shouldn't testMalformedRead and testMalformedWrite be 
updated so that expectedExceptions lists the specify exception that 
is expected? If I read the update correctly then isn't checking for 
MalformedInputException and UnmappableCharacterException anywhere 
(it passes if IOException is thrown).


MalformedInputException and UnmappableCharacterException are 
implementation details, the tests only verified what the spec 
required (IOE).


-Joe



-Alan

RE: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 Instructions

[Kamath, Smita]

Hi Vladimir,

Please find the updated webrev link. 

Webrev Link: http://cr.openjdk.java.net/~srukmannagar/Base64/webrev.00/
Bug link: https://bugs.openjdk.java.net/browse/JDK-8205528

Please let me know if you have additional questions.

Thanks and Regards,
Smita

-Original Message-
From: Vladimir Kozlov [mailto:vladimir.koz...@oracle.com] 
Sent: Monday, June 25, 2018 10:48 AM
To: Kamath, Smita 
Cc: hotspot compiler ; 
core-libs-dev@openjdk.java.net; Paul Sandoz 
Subject: Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 
Instructions

I forgot to reply to your answers.

On 6/22/18 2:49 PM, Kamath, Smita wrote:
> Hi Vladimir,
> 
> Please see my answers to your questions as below:
> 
> 1) One question so: why you have own copy of base64 charsets and not using 
> one in library:
> I am using vpgatherdd instruction to fetch multiple values from base64 array 
> in a single instruction with a vector index. Vpgatherdd instruction works on 
> 32 bit array and so I need to define base64 charset in a 32 bit array. I have 
> given reference to gather instruction below.

As was discussed in an other e-mail lets keep your copy.

> 
> 2) Some indents in new and old Assembler::emit_operand() are off. In new 
> Assembler::emit_operand() is better use } else { instead of 'return' in one 
> branch.
> I'll make the necessary changes and send an updated webrev.
> 
> 3) This is first time I see that XMM register can be used for index in 
> address. Is it true? Can you point to Intel's document which describes it.
> I am using vpgatherdd instruction which requires index vector with scale. It 
> uses VSIB addressing where the index register is a zmm register.
> Please refer to reference manual, volume 2c, page 2211: 
> https://software.intel.com/sites/default/files/managed/39/c5/325462-sd
> m-vol-1-2abcd-3abcd.pdf Also see, section 2.3.12, page 524 for VSIB 
> memory addressing information.

Got it. Thanks for document reference.

> 
> 4)  Please, add tests to test/hotspot/jtreg/compiler/intrinsics/base64 (may 
> be similar to sha or AES in compiler/codegen/aes) to make sure that all 
> flags, intrinsic is used and it produces correct result.
> I will add test cases as per your suggestion.

Thanks,
Vladimir

> 
> Please let me know if you have additional questions.
> 
> Thanks,
> Smita
> 
> -Original Message-
> From: Vladimir Kozlov [mailto:vladimir.koz...@oracle.com]
> Sent: Friday, June 22, 2018 12:29 PM
> To: Kamath, Smita 
> Cc: hotspot compiler 
> Subject: Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 
> Instructions
> 
> Hi Smita,
> 
> I CCing to Libs to review code changes in Base64.java.
> 
> Looks like you changed all need place to implement intrinsic.
> One question so: why you have own copy of base64 charsets and not using one 
> in library:
> 
>private int encode0(byte[] src, int off, int end, byte[] dst) {
>char[] base64 = isURL ? toBase64URL : toBase64;
> 
> Some indents in new and old Assembler::emit_operand() are off.
> 
> In new Assembler::emit_operand() is better use } else { instead of 'return' 
> in one branch.
> 
> This is first time I see that XMM register can be used for index in address. 
> Is it true? Can you point to Intel's document which describes it.
> 
> What testing you did?
> 
> Please, add tests to test/hotspot/jtreg/compiler/intrinsics/base64 (may be 
> similar to sha or AES in compiler/codegen/aes) to make sure that all flags, 
> intrinsic is used and it produces correct result.
> 
> I know there is test/jdk/java/util/Base64/ tests but they may not trigger 
> intrinsic usage. But you can use them as starting point for new tests.
> 
> Thanks,
> Vladimir
> 
> On 6/22/18 11:47 AM, Kamath, Smita wrote:
>> Hi Vladimir,
>>
>> I'd like to contribute an optimization for Base64 Encoding Algorithm 
>> using AVX512 Instructions. This optimization shows 1.5x improvement 
>> on
>> x86_64 platform(SKL).
>>
>> Link to Bug: https://bugs.openjdk.java.net/browse/JDK-8205528
>>
>> Link to webrev:
>> http://cr.openjdk.java.net/~vdeshpande/Base64/webrev.00/
>>
>> For testing the implementation, I have run tests under 
>> test/jdk/java/util/Base64/ and also ran 
>> test/jdk/com/sun/jndi/ldap/Base64Test.java
>>
>> I have also run jtreg.
>>
>> Please review and let me know if you have any comments.
>>
>> Thanks and Regards,
>>
>> Smita
>>

Re: RFR(JDK11/NIO) 8205058 throw CharacterCodingException --> Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for reading/writing a string from/to a file

[Joe Wang]

Hi Alan, Sherman,

Here's a version where we, as Sherman suggested, throw an IAE with CCE 
as the cause. This approach reduces code duplication in SC, although it 
complicates the impl a little bit with the added parameter and the 
different behavior between the existing usages of the methods and the 
new ones. The existing code paths are kept intact so there's no 
compatibility issue for the existing code.


This version also did not remove the try-catch in Files as Alan 
suggested earlier.


http://cr.openjdk.java.net/~joehw/jdk11/8205058/webrev02/

Thanks,
Joe

On 6/25/18, 3:41 PM, Joe Wang wrote:

Hi Alan,

The test testMalformedRead and testMalformedWrite now expect an 
UnmappableCharacterException instead of IOE:


webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8205058/webrev01/

Thanks,
Joe

On 6/25/18, 9:48 AM, Joe Wang wrote:



On 6/24/18, 12:11 PM, Alan Bateman wrote:

On 20/06/2018 04:32, Joe Wang wrote:
Thanks Alan.  I created 8205058 to capture your suggestions. Please 
see below for more details.


Changed the internal APIs to throw CCE instead. In the same way as 
the previous changeset for 8201276, these methods are made specific 
for the use cases (though they are now for Files.read/writeString 
only) so that they are not mixed up with existing ones that may 
inadvertently affect other usages.


One thing to note is that MalformedInputException or 
UnmappableCharacterException will lose one piece of information in 
comparison to the existing IAE, that is where it happens (offset). 
Should there be an improvement in the future, we could consider add 
it back to this part of code.


JBS: https://bugs.openjdk.java.net/browse/JDK-8205058
webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8205058/webrev/


I see your point on MalformedInputException and 
UnmappableCharacterException so maybe we can submit a new issue to 
track the follow-on work.


Submitted: https://bugs.openjdk.java.net/browse/JDK-8205613



The update means a lot of duplication in StringCoding. Did you 
consider (or measure) having the private encode/decode methods take 
a parameter to indicate the exception handling? Sherman might have 
suggestions on this.


I did. I didn't like the code duplication at all. But it would be 
even messier if we reuse the code since the previous usage didn't 
declare checked exception, but did capture the position (offset) and 
length information, which means the new method while unnecessary to 
capture these information for Files read/writeString would still need 
to save them in case Exception occurs. Because of the complication, I 
thought you and Sherman would again prefer a specific methods than 
adding parameters (need fields as well).


Furthermore, in the first round of the review, Sherman mentioned that 
he's been working on an improvement in this area. But I'll wait till 
Sherman could comment on this.




In the tests, shouldn't testMalformedRead and testMalformedWrite be 
updated so that expectedExceptions lists the specify exception that 
is expected? If I read the update correctly then isn't checking for 
MalformedInputException and UnmappableCharacterException anywhere 
(it passes if IOException is thrown).


MalformedInputException and UnmappableCharacterException are 
implementation details, the tests only verified what the spec 
required (IOE).


-Joe



-Alan

Re: RFR: 8205184: Delegating Iterator implementations that don't delegate forEachRemaining()

[Martin Buchholz]

Committed.

Re: Proposal: optimization of Map.copyOf and Collectors.toUnmodifiableMap

[Stuart Marks]

Hi Peter,

Sorry for the delay again. I got pulled into a P1 escalation.

I don't think I'll have time to look at this in any detail until after JDK 11 
ramp-down.


s'marks


On 6/22/18 9:51 AM, Peter Levart wrote:

Hi Stuart,

Do you still fear that "single-threaded-object" idiom is not safe?

I would just like to comment on the last approach...

On 06/19/2018 07:01 PM, Peter Levart wrote:
Here's another attempt that uses the same technique but relaxes the possible 
concurrent source map scenario (where number of pairs emitted by 
Map.forEach() doesn't agree with Map.size()):


http://cr.openjdk.java.net/~plevart/jdk-dev/UnmodifiableMap_copyOf/webrev.03/

The construction is moved entirely into a separate MapBuilder object. The 
builder is used for Map.of(...) methods too.


Here are the benchmark results that show improvements in every respect 
touched by the patch:


http://cr.openjdk.java.net/~plevart/jdk-dev/UnmodifiableMap_copyOf/UnmodifiableMapBench_results.txt 



Here are the JMH benchmarks used to produce these results:

http://cr.openjdk.java.net/~plevart/jdk-dev/UnmodifiableMap_copyOf/UnmodifiableMapCollectorBench.java 

http://cr.openjdk.java.net/~plevart/jdk-dev/UnmodifiableMap_copyOf/UnmodifiableMapCopyOfBench.java 

http://cr.openjdk.java.net/~plevart/jdk-dev/UnmodifiableMap_copyOf/UnmodifiableMapOfBench.java 




So what do you think of this one?

Regards, Peter 


When this is used to copy concurrently changing concurrent Map, it has 
approximately the same performance consequence as when using 
Map.entrySet().toArray(). Either logic uses the estimated size to pre-size the 
array, but when the size is a moving target, it fails, so it has to do another 
copy at the end. My approach uses the estimated size to collect elements into 
the ready-made hash-table. When it fails, it has to do another copy, but in 
general case when the estimate is correct, it doesn't need an intermediary 
representation in the form of array, so it can be more optimal.


Regards, Peter

Re: RFR(xs): JDK-8201610: fix broken link in UnicastRemoteObject

[Lance Andersen]

+1

Had one of those myself recently :-)

> On Jun 25, 2018, at 7:44 PM, Stuart Marks  wrote:
> 
> Hi all,
> 
> Please review this small fix for a broken link.
> 
> Patch appended below.
> 
> Bug:
> 
>https://bugs.openjdk.java.net/browse/JDK-8201610
> 
> Thanks,
> 
> s'marks
> 
> 
> 
> diff -r 8b98dcf37891 -r 3da3bcc7b28b 
> src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java
> --- a/src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java 
> Mon Jun 25 16:21:14 2018 -0700
> +++ b/src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java 
> Mon Jun 25 16:43:57 2018 -0700
> @@ -126,7 +126,7 @@
>  * 
>  *
>  * The proxy's class is defined according to the specifications for the
> - * 
> + * 
>  * {@code Proxy}
>  * 
>  * class, using the class loader of the remote object's class.

 
  

 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: RFR(xs): JDK-8201610: fix broken link in UnicastRemoteObject

[Paul Sandoz]

+1
Paul.

> On Jun 25, 2018, at 4:44 PM, Stuart Marks  wrote:
> 
> Hi all,
> 
> Please review this small fix for a broken link.
> 
> Patch appended below.
> 
> Bug:
> 
>https://bugs.openjdk.java.net/browse/JDK-8201610
> 
> Thanks,
> 
> s'marks
> 
> 
> 
> diff -r 8b98dcf37891 -r 3da3bcc7b28b 
> src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java
> --- a/src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java 
> Mon Jun 25 16:21:14 2018 -0700
> +++ b/src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java 
> Mon Jun 25 16:43:57 2018 -0700
> @@ -126,7 +126,7 @@
>  * 
>  *
>  * The proxy's class is defined according to the specifications for the
> - * 
> + * 
>  * {@code Proxy}
>  * 
>  * class, using the class loader of the remote object's class.

RFR(xs): JDK-8201610: fix broken link in UnicastRemoteObject

[Stuart Marks]

Hi all,

Please review this small fix for a broken link.

Patch appended below.

Bug:

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

Thanks,

s'marks



diff -r 8b98dcf37891 -r 3da3bcc7b28b 
src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java
--- a/src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java	Mon 
Jun 25 16:21:14 2018 -0700
+++ b/src/java.rmi/share/classes/java/rmi/server/UnicastRemoteObject.java	Mon 
Jun 25 16:43:57 2018 -0700

@@ -126,7 +126,7 @@
  * 
  *
  * The proxy's class is defined according to the specifications for the
- * 
+ * 
  * {@code Proxy}
  * 
  * class, using the class loader of the remote object's class.

Re: RFR(s): 8203670: unmodifiable List iterator() implementations should not be ListIterators

[Ivan Gerasimov]

Hi Stuart!

Out of curiosity.  What if someone does something like

if (it instanceof ListIterator) {
// optimized for bidirectional access
} else {
// slower algorithm
}

previous(), nextIndex() and previousIndex() methods are not declared to 
be optional, so is it appropriate to throw UnsupportedOperationException 
from them?


Someone may assume that if an object can be cast to ListIterator 
interface, non-optional methods should not throw UOE.


With kind regards,

Ivan


On 6/25/18 3:06 PM, Stuart Marks wrote:

Hi all,

Please review this small changeset that ensures that calling 
iterator() on an unmodifiable List (from List.of, et. al.) returns an 
instance that cannot be downcast to ListIterator and used as such.


Bug:

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

Webrev:

http://cr.openjdk.java.net/~smarks/reviews/8203670/webrev.0/

Thanks,

s'marks



--
With kind regards,
Ivan Gerasimov

Re: RFR(JDK11/NIO) 8205058 throw CharacterCodingException --> Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for reading/writing a string from/to a file

[Joe Wang]

Hi Alan,

The test testMalformedRead and testMalformedWrite now expect an 
UnmappableCharacterException instead of IOE:


webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8205058/webrev01/

Thanks,
Joe

On 6/25/18, 9:48 AM, Joe Wang wrote:



On 6/24/18, 12:11 PM, Alan Bateman wrote:

On 20/06/2018 04:32, Joe Wang wrote:
Thanks Alan.  I created 8205058 to capture your suggestions. Please 
see below for more details.


Changed the internal APIs to throw CCE instead. In the same way as 
the previous changeset for 8201276, these methods are made specific 
for the use cases (though they are now for Files.read/writeString 
only) so that they are not mixed up with existing ones that may 
inadvertently affect other usages.


One thing to note is that MalformedInputException or 
UnmappableCharacterException will lose one piece of information in 
comparison to the existing IAE, that is where it happens (offset). 
Should there be an improvement in the future, we could consider add 
it back to this part of code.


JBS: https://bugs.openjdk.java.net/browse/JDK-8205058
webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8205058/webrev/


I see your point on MalformedInputException and 
UnmappableCharacterException so maybe we can submit a new issue to 
track the follow-on work.


Submitted: https://bugs.openjdk.java.net/browse/JDK-8205613



The update means a lot of duplication in StringCoding. Did you 
consider (or measure) having the private encode/decode methods take a 
parameter to indicate the exception handling? Sherman might have 
suggestions on this.


I did. I didn't like the code duplication at all. But it would be even 
messier if we reuse the code since the previous usage didn't declare 
checked exception, but did capture the position (offset) and length 
information, which means the new method while unnecessary to capture 
these information for Files read/writeString would still need to save 
them in case Exception occurs. Because of the complication, I thought 
you and Sherman would again prefer a specific methods than adding 
parameters (need fields as well).


Furthermore, in the first round of the review, Sherman mentioned that 
he's been working on an improvement in this area. But I'll wait till 
Sherman could comment on this.




In the tests, shouldn't testMalformedRead and testMalformedWrite be 
updated so that expectedExceptions lists the specify exception that 
is expected? If I read the update correctly then isn't checking for 
MalformedInputException and UnmappableCharacterException anywhere (it 
passes if IOException is thrown).


MalformedInputException and UnmappableCharacterException are 
implementation details, the tests only verified what the spec required 
(IOE).


-Joe



-Alan

RFR(s): 8203670: unmodifiable List iterator() implementations should not be ListIterators

[Stuart Marks]

Hi all,

Please review this small changeset that ensures that calling iterator() on an 
unmodifiable List (from List.of, et. al.) returns an instance that cannot be 
downcast to ListIterator and used as such.


Bug:

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

Webrev:

http://cr.openjdk.java.net/~smarks/reviews/8203670/webrev.0/

Thanks,

s'marks

Re: RFR: 8205184: Delegating Iterator implementations that don't delegate forEachRemaining()

[Martin Buchholz]

On Mon, Jun 25, 2018 at 11:17 AM, Paul Sandoz 
wrote:

> Looks good. Sometimes in these cases i reach for proxy rather than
> explicit code. It tends to be more robust to certain changes at the expense
> of reflective complexity.
>
>
Hmmm ... Never done that in a test before.


> You could probably reduce the test method names by removing the
> “forEachRemainingIsDelegated_” given that can be inferred from the class
> name.
>

test method names shortened.

Re: RFR 8195650 Method references to VarHandle accessors

[Karen Kinnear]

Looks good. Matches the existing JVMS. Thanks for the tests.

thanks,
Karen

> On Jun 19, 2018, at 8:08 PM, Paul Sandoz  wrote:
> 
> Hi,
> 
> Please review the following fix to ensure method references to VarHandle 
> signature polymorphic methods are supported at runtime (specifically the 
> method handle to a signature polymorphic method can be loaded from the 
> constant pool):
> 
>  http://cr.openjdk.java.net/~psandoz/jdk/JDK-8195650-varhandle-mref/webrev/ 
> 
> 
> I also added a “belts and braces” test to ensure a constant method handle to 
> MethodHandle::invokeBasic cannot be loaded if outside of the j.l.invoke 
> package.
> 
> Paul.
> 

Re: RFR 8195650 Method references to VarHandle accessors

[John Rose]

Good fix. Reviewed. 

> On Jun 25, 2018, at 9:11 AM, Paul Sandoz  wrote:
> 
> Gentle reminder.
> 
> I would like to get this reviews and pushed before the ramp down phase one 
> kicks in this week.
> 
> Paul.
> 
>> On Jun 19, 2018, at 5:08 PM, Paul Sandoz  wrote:
>> 
>> Hi,
>> 
>> Please review the following fix to ensure method references to VarHandle 
>> signature polymorphic methods are supported at runtime (specifically the 
>> method handle to a signature polymorphic method can be loaded from the 
>> constant pool):
>> 
>> http://cr.openjdk.java.net/~psandoz/jdk/JDK-8195650-varhandle-mref/webrev/ 
>> 
>> 
>> I also added a “belts and braces” test to ensure a constant method handle to 
>> MethodHandle::invokeBasic cannot be loaded if outside of the j.l.invoke 
>> package.
>> 
>> Paul.
>> 
> 

Re: RFR: 8148188: Enhance the security libraries to record events of interest

[Seán Coffey]

Many thanks for the review comments Erik. Replies inline.


On 24/06/2018 14:21, Erik Gahlin wrote:

Hi Sean,

Some of the changes in the webrev belongs to JDK-8203629 and should be 
removed for clarity.


Some initial comments:

default.jfc, profile.jfr:
The events should not have control="enable-exceptions". The purpose of 
the control attribute is so to provide parameterized configuration of 
events for JMC.  As it is now, the security events will be enabled 
when a user turns on the exception events.

Makes sense. I'll remove that parameter.


X509CertEvent:
You should use milliseconds since epoch to represent a date instead of 
a string value, i.e.


    @Label("Valid From")
    @Timestamp(Timestamp.MILLISECONDS_SINCE_EPOCH)
    public long validFrom;

    @Label("Valid Until")
    @Timestamp(Timestamp.MILLISECONDS_SINCE_EPOCH)
    public long validUntil;

The CertificateValidity class operates on Date Object values. I'll work 
with the Date.getTime() method then (and update the Logger formatting)

This following annotation adds little value

@Description("Details of Security Property")

I would either remove the annotation, or provide information that 
helps a user understand the event. For instance, what is X509, and 
what kind of certificates are we talking about?
Yes - that looks like the wrong Description. I'll review all of these 
fields.


@Category("Java Application")

I'm a bit worried that we will pollute the "Java Application" 
namespace if we add lots of JDK events in that category. We may want 
to add a new top level category "Java Development Kit", analogous to 
the "Java Virtual Machine" category, and put all security related 
events in subcategory, perhaps called "Security".

Yes - Open to suggestions. "Security" sounds like a good suggestion.


@Label("X509Cert")

The label should be human readable name, with spaces, title cased etc. 
I would recommend "X.509 Certificate". In general, avoid abbreviations 
like "certs" and instead use labels such as "Certificate Chain". The 
label should be short and not a full sentence.


For details see,
https://docs.oracle.com/javase/10/docs/api/jdk/jfr/Label.html

I think it would be good to separate testing of JFR and logging into 
different files / tests. I would prefer that the test name is the same 
as the event prefixed with "Test", i.e TestX509CertificateEvent, as 
this is the pattern used by other JFR tests.


I'll take a look at that pattern again. I've separated out the current 
tests into an (a) outer test to analyze the logger output and (b) the 
inner test which checks for JFR correctness. I did include extra logic 
to make sure that the EventHelper configuration was working correctly. 
"Events.assertField" looks very helpful. Thanks for the pointer.


Let me take on board the suggestions below and get a new webrev out on 
Tuesday.


regards,
Sean.


I reworked one of the tests to how I like to see it:

/*
 * @test
 * @key jfr
 * @library /test/lib
 * @run main/othervm jdk.jfr.event.security.TestX509CertificateEvent
 */
public class TestX509CertificateEvent {

    private static final String CERTIFICATE_1 = "...";
    private static final String CERTIFICATE_2 = "...";

    public static void main(String... args) throws CertificateException {

 Recording r = new Recording();
 r.enable(EventNames.X509Certificate).withoutStackTrace();
 r.start();

 CertificateFactory cf = CertificateFactory.getInstance("X.509");
 cf.generateCertificate(new 
ByteArrayInputStream(CERTIFICATE_1.getBytes()));
 cf.generateCertificate(new 
ByteArrayInputStream(CERTIFICATE_2.getBytes()));


 // Make sure only one event per certificate
 cf.generateCertificate(new 
ByteArrayInputStream(CERTIFICATE_1.getBytes()));
 cf.generateCertificate(new 
ByteArrayInputStream(CERTIFICATE_2.getBytes()));


 r.stop();

 List events = Events.fromRecording(r);
 Asserts.assertEquals(events.size(), 2, "Expected two X.509 
Certificate events");


 assertEvent(events, "1000", "SHA256withRSA",
    "CN=SSLCertificate, O=SomeCompany",
    "CN=Intermediate CA Cert, O=SomeCompany",
 "RSA", 2048);
 assertEvent(events, "1000", "SHA256withRSA",
    "CN=SSLCertificate, O=SomeCompany",
    "CN=Intermediate CA Cert, O=SomeCompany",
 "RSA", 2048);
    }

    private static void assertEvent(List events, String 
certID, String algId, String subject,

    String issuer, String keyType, int length) throws Exception {

    for (RecordedEvent e : events) {
    if (e.getString("serialNumber").equals(certID)) {
    Events.assertField(e, "algId").equal(algId);
    ...
    return;
    }
    }
    System.out.println(events);
    throw new Exception("Could not find event with Cert ID: " + 
certID);

    }
}

The reworked 

Re: Review Request: JDK-8205623: Replace use of Class::getPackage with Class::getPackageName

[Alan Bateman]

On 25/06/2018 19:46, mandy chung wrote:
This patch replaces the use of Class::getPackage with 
Class::getPackageName in jdk.internal.reflect.ReflectionFactory, 
sun.util.resources.BreakIteratorResourceBundle, and 
javax.xml.catalog.CatalogMessages.  Class::getPackageName avoids the 
overhead of constructing Package objects.


http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8205623/webrev.00/

ReflectionFactory::packageEquals does not specify if the Class object 
is an array class.  In the current implementation it returns true even 
if the input parameters are both array classes but in two different 
runtime package.  I added an assert instead of retaining the current 
behavior.

BreakIteratorResourceBundler and CatalogMessages look okay.

For ReflectionFactory.packageEquals then it might be cleaner to put the 
assert first but what you have is okay too.


-Alan

Re: RFR 8205547 : FileChannel/CleanerTest.java fails due to expected FD count

[Roger Riggs]

Thanks Paul, I'll add a comment.

On 6/25/2018 2:26 PM, Paul Sandoz wrote:



On Jun 25, 2018, at 11:08 AM, Roger Riggs > wrote:


Hi Paul,

Updated the webrev in place:
http://cr.openjdk.java.net/~rriggs/webrev-fd-count-wrong-8205547/

On 6/25/2018 1:41 PM, Paul Sandoz wrote:

Hi Roger,

Are you missing the throwing of an exception when the fdCount != fdCount0? (and 
relatedly i could not tell if the print statements were for temporary debugging 
and you intended to remove them)
That condition needed to be removed, the number of open file 
descriptors changes unpredictably
during the test.  The check for the reclamation of the 
'FileDescriptor' and 'closer' replaces

the simple count of open file descriptors.

In one case, I spotted a socket that showed up as open during the 
test (but the code does not use sockets).
The failures have only been seen using mach5 and are not otherwise 
reproducible.
I retained and improved the debugging information with some 
aspiration to determine

what file descriptor was appearing or disappearing.


Ah yes, i see now. Perhaps add a comment or something in the summary 
stating that failure will result in a time out? (No need for another 
review).




Since you check before hand for support of UnixOperatingSystemMXBean there 
appears no need for the method getFdCount can you can reuse the local variable 
unixMxBean. (If the check fails it suggests the @requires is incorrect, 
implying the test should fail.)
Right, I updated this case to look like the other 
java/io/FileXXXStream/UnreferencedXXXClosesFd tests

that did not have the @requires or the pretest.
The direct use of unixMxBean is fine; I reverted the addition of 
getFdCount().


I'll keep the instanceofUnixOperatingSystemMXBean as belt and suspenders.



Ok.

Paul.

Review Request: JDK-8205623: Replace use of Class::getPackage with Class::getPackageName

[mandy chung]

This patch replaces the use of Class::getPackage with 
Class::getPackageName in jdk.internal.reflect.ReflectionFactory, 
sun.util.resources.BreakIteratorResourceBundle, and 
javax.xml.catalog.CatalogMessages.  Class::getPackageName avoids the 
overhead of constructing Package objects.


http://cr.openjdk.java.net/~mchung/jdk11/webrevs/8205623/webrev.00/

ReflectionFactory::packageEquals does not specify if the Class object is 
an array class.  In the current implementation it returns true even if 
the input parameters are both array classes but in two different runtime 
package.  I added an assert instead of retaining the current behavior.


Mandy

Re: RFR 8205547 : FileChannel/CleanerTest.java fails due to expected FD count

[Paul Sandoz]

> On Jun 25, 2018, at 11:08 AM, Roger Riggs  wrote:
> 
> Hi Paul,
> 
> Updated the webrev in place:
>   http://cr.openjdk.java.net/~rriggs/webrev-fd-count-wrong-8205547/ 
> 
> 
> On 6/25/2018 1:41 PM, Paul Sandoz wrote:
>> Hi Roger,
>> 
>> Are you missing the throwing of an exception when the fdCount != fdCount0? 
>> (and relatedly i could not tell if the print statements were for temporary 
>> debugging and you intended to remove them)
> That condition needed to be removed, the number of open file descriptors 
> changes unpredictably
> during the test.  The check for the reclamation of the 'FileDescriptor' and 
> 'closer' replaces
> the simple count of open file descriptors.
> 
> In one case, I spotted a socket that showed up as open during the test (but 
> the code does not use sockets).
> The failures have only been seen using mach5 and are not otherwise 
> reproducible.
> I retained and improved the debugging information with some aspiration to 
> determine 
> what file descriptor was appearing or disappearing.

Ah yes, i see now. Perhaps add a comment or something in the summary stating 
that failure will result in a time out? (No need for another review).


>> 
>> Since you check before hand for support of UnixOperatingSystemMXBean there 
>> appears no need for the method getFdCount can you can reuse the local 
>> variable unixMxBean. (If the check fails it suggests the @requires is 
>> incorrect, implying the test should fail.)
> Right, I updated this case to look like the other 
> java/io/FileXXXStream/UnreferencedXXXClosesFd tests
> that did not have the @requires or the pretest.
> The direct use of unixMxBean is fine; I reverted the addition of getFdCount().
> 
> I'll keep the instanceofUnixOperatingSystemMXBean as belt and suspenders.
> 

Ok.

Paul.

Re: RFR: 8205184: Delegating Iterator implementations that don't delegate forEachRemaining()

[Paul Sandoz]

Looks good. Sometimes in these cases i reach for proxy rather than explicit 
code. It tends to be more robust to certain changes at the expense of 
reflective complexity.

You could probably reduce the test method names by removing the 
“forEachRemainingIsDelegated_” given that can be inferred from the class name.

Paul.


> On Jun 21, 2018, at 8:06 PM, Martin Buchholz  wrote:
> 
> 8205184: Delegating Iterator implementations that don't delegate
> forEachRemaining()
> http://cr.openjdk.java.net/~martin/webrevs/jdk/delegate-forEachRemaining/
> https://bugs.openjdk.java.net/browse/JDK-8205184

Re: RFR 8205547 : FileChannel/CleanerTest.java fails due to expected FD count

[Roger Riggs]

Hi Paul,

Updated the webrev in place:
  http://cr.openjdk.java.net/~rriggs/webrev-fd-count-wrong-8205547/

On 6/25/2018 1:41 PM, Paul Sandoz wrote:

Hi Roger,

Are you missing the throwing of an exception when the fdCount != fdCount0? (and 
relatedly i could not tell if the print statements were for temporary debugging 
and you intended to remove them)
That condition needed to be removed, the number of open file descriptors 
changes unpredictably
during the test.  The check for the reclamation of the 'FileDescriptor' 
and 'closer' replaces

the simple count of open file descriptors.

In one case, I spotted a socket that showed up as open during the test 
(but the code does not use sockets).
The failures have only been seen using mach5 and are not otherwise 
reproducible.
I retained and improved the debugging information with some aspiration 
to determine

what file descriptor was appearing or disappearing.


Since you check before hand for support of UnixOperatingSystemMXBean there 
appears no need for the method getFdCount can you can reuse the local variable 
unixMxBean. (If the check fails it suggests the @requires is incorrect, 
implying the test should fail.)
Right, I updated this case to look like the other 
java/io/FileXXXStream/UnreferencedXXXClosesFd tests

that did not have the @requires or the pretest.
The direct use of unixMxBean is fine; I reverted the addition of 
getFdCount().


I'll keep the instanceofUnixOperatingSystemMXBean as belt and suspenders.

Thanks, Roger



Paul.



On Jun 25, 2018, at 8:24 AM, Roger Riggs  wrote:

Please review a test improvement to avoid unexpected file opens/closes during 
the file channel CleanerTest.
The test now follows the pattern of the other 
java/io/File*/UnreferencedXXXClosesFd tests by waiting
for the cleaner and file descriptors to be reclaimed by GC.

Webrev:
   http://cr.openjdk.java.net/~rriggs/webrev-fd-count-wrong-8205547/

Thanks, Roger

Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 Instructions

[Vladimir Kozlov]

I forgot to reply to your answers.

On 6/22/18 2:49 PM, Kamath, Smita wrote:

Hi Vladimir,

Please see my answers to your questions as below:

1) One question so: why you have own copy of base64 charsets and not using one 
in library:
I am using vpgatherdd instruction to fetch multiple values from base64 array in 
a single instruction with a vector index. Vpgatherdd instruction works on 32 
bit array and so I need to define base64 charset in a 32 bit array. I have 
given reference to gather instruction below.


As was discussed in an other e-mail lets keep your copy.



2) Some indents in new and old Assembler::emit_operand() are off. In new 
Assembler::emit_operand() is better use } else { instead of 'return' in one 
branch.
I'll make the necessary changes and send an updated webrev.

3) This is first time I see that XMM register can be used for index in address. 
Is it true? Can you point to Intel's document which describes it.
I am using vpgatherdd instruction which requires index vector with scale. It 
uses VSIB addressing where the index register is a zmm register.
Please refer to reference manual, volume 2c, page 2211: 
https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf
Also see, section 2.3.12, page 524 for VSIB memory addressing information.


Got it. Thanks for document reference.



4)  Please, add tests to test/hotspot/jtreg/compiler/intrinsics/base64 (may be 
similar to sha or AES in compiler/codegen/aes) to make sure that all flags, 
intrinsic is used and it produces correct result.
I will add test cases as per your suggestion.


Thanks,
Vladimir



Please let me know if you have additional questions.

Thanks,
Smita

-Original Message-
From: Vladimir Kozlov [mailto:vladimir.koz...@oracle.com]
Sent: Friday, June 22, 2018 12:29 PM
To: Kamath, Smita 
Cc: hotspot compiler 
Subject: Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 
Instructions

Hi Smita,

I CCing to Libs to review code changes in Base64.java.

Looks like you changed all need place to implement intrinsic.
One question so: why you have own copy of base64 charsets and not using one in 
library:

   private int encode0(byte[] src, int off, int end, byte[] dst) {
   char[] base64 = isURL ? toBase64URL : toBase64;

Some indents in new and old Assembler::emit_operand() are off.

In new Assembler::emit_operand() is better use } else { instead of 'return' in 
one branch.

This is first time I see that XMM register can be used for index in address. Is 
it true? Can you point to Intel's document which describes it.

What testing you did?

Please, add tests to test/hotspot/jtreg/compiler/intrinsics/base64 (may be 
similar to sha or AES in compiler/codegen/aes) to make sure that all flags, 
intrinsic is used and it produces correct result.

I know there is test/jdk/java/util/Base64/ tests but they may not trigger 
intrinsic usage. But you can use them as starting point for new tests.

Thanks,
Vladimir

On 6/22/18 11:47 AM, Kamath, Smita wrote:

Hi Vladimir,

I'd like to contribute an optimization for Base64 Encoding Algorithm
using AVX512 Instructions. This optimization shows 1.5x improvement on
x86_64 platform(SKL).

Link to Bug: https://bugs.openjdk.java.net/browse/JDK-8205528

Link to webrev:
http://cr.openjdk.java.net/~vdeshpande/Base64/webrev.00/

For testing the implementation, I have run tests under
test/jdk/java/util/Base64/ and also ran
test/jdk/com/sun/jndi/ldap/Base64Test.java

I have also run jtreg.

Please review and let me know if you have any comments.

Thanks and Regards,

Smita

Re: RFR 8205547 : FileChannel/CleanerTest.java fails due to expected FD count

[Paul Sandoz]

Hi Roger,

Are you missing the throwing of an exception when the fdCount != fdCount0? (and 
relatedly i could not tell if the print statements were for temporary debugging 
and you intended to remove them)

Since you check before hand for support of UnixOperatingSystemMXBean there 
appears no need for the method getFdCount can you can reuse the local variable 
unixMxBean. (If the check fails it suggests the @requires is incorrect, 
implying the test should fail.)

Paul.


> On Jun 25, 2018, at 8:24 AM, Roger Riggs  wrote:
> 
> Please review a test improvement to avoid unexpected file opens/closes during 
> the file channel CleanerTest.
> The test now follows the pattern of the other 
> java/io/File*/UnreferencedXXXClosesFd tests by waiting
> for the cleaner and file descriptors to be reclaimed by GC.
> 
> Webrev:
>   http://cr.openjdk.java.net/~rriggs/webrev-fd-count-wrong-8205547/
> 
> Thanks, Roger
> 

Re: RFR: jsr166 integration 2018-06

[Martin Buchholz]

Committed except for

http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html

8203662: remove increment of modCount from ArrayList and Vector replaceAll()
http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/replaceAll/index.html
https://bugs.openjdk.java.net/browse/JDK-8203662

Re: RFR(JDK11/NIO) 8205058 throw CharacterCodingException --> Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for reading/writing a string from/to a file

[Joe Wang]

On 6/24/18, 12:11 PM, Alan Bateman wrote:

On 20/06/2018 04:32, Joe Wang wrote:
Thanks Alan.  I created 8205058 to capture your suggestions. Please 
see below for more details.


Changed the internal APIs to throw CCE instead. In the same way as 
the previous changeset for 8201276, these methods are made specific 
for the use cases (though they are now for Files.read/writeString 
only) so that they are not mixed up with existing ones that may 
inadvertently affect other usages.


One thing to note is that MalformedInputException or 
UnmappableCharacterException will lose one piece of information in 
comparison to the existing IAE, that is where it happens (offset). 
Should there be an improvement in the future, we could consider add 
it back to this part of code.


JBS: https://bugs.openjdk.java.net/browse/JDK-8205058
webrevs: http://cr.openjdk.java.net/~joehw/jdk11/8205058/webrev/


I see your point on MalformedInputException and 
UnmappableCharacterException so maybe we can submit a new issue to 
track the follow-on work.


Submitted: https://bugs.openjdk.java.net/browse/JDK-8205613



The update means a lot of duplication in StringCoding. Did you 
consider (or measure) having the private encode/decode methods take a 
parameter to indicate the exception handling? Sherman might have 
suggestions on this.


I did. I didn't like the code duplication at all. But it would be even 
messier if we reuse the code since the previous usage didn't declare 
checked exception, but did capture the position (offset) and length 
information, which means the new method while unnecessary to capture 
these information for Files read/writeString would still need to save 
them in case Exception occurs. Because of the complication, I thought 
you and Sherman would again prefer a specific methods than adding 
parameters (need fields as well).


Furthermore, in the first round of the review, Sherman mentioned that 
he's been working on an improvement in this area. But I'll wait till 
Sherman could comment on this.




In the tests, shouldn't testMalformedRead and testMalformedWrite be 
updated so that expectedExceptions lists the specify exception that is 
expected? If I read the update correctly then isn't checking for 
MalformedInputException and UnmappableCharacterException anywhere (it 
passes if IOException is thrown).


MalformedInputException and UnmappableCharacterException are 
implementation details, the tests only verified what the spec required 
(IOE).


-Joe



-Alan

Re: [11]RFR 8196213: sun/security/tools/jarsigner/warnings/NoTimestampTest.java test fails on ar_SA locale.

[Naoto Sato]

Looks good.

Naoto

On 6/25/18 12:04 AM, Dora Zhou wrote:

Hello,

Please help review the fix for JDK-8196213. Thank you.
Set default locale to en-US so that the output display the date using 
Gregorian Calendar and Latn numbering system(0~9).


Bug: https://bugs.openjdk.java.net/browse/JDK-8196213
Webrev: http://cr.openjdk.java.net/~ljiang/8196213/webrev/ 



Regards,
Dora

Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 Instructions

[Paul Sandoz]

> On Jun 22, 2018, at 5:17 PM, Vladimir Kozlov  
> wrote:
> 
> On 6/22/18 3:58 PM, Paul Sandoz wrote:
>> Hi Smita,
>> I am ok with it if Vladimir is :-) One slight concern is this may be biasing 
>> towards the x86 implementation of the intrinsic.
> 
> Looking on code and it will be a lot of changes in Base64.java. I am concern 
> about that late in JDK 11.
> 
> I think we should keep duplicated code for x86 intrinsic as Smita suggested. 
> And we can return to this when/if we intrinsify Decoder too.
> 

Agreed.

Paul.

>> I dunno if an int[] table is as useful for an AARCH64 intrinsic.
> 
> We should ask RH to check.
> 
> But I think SPARC is better operating on 32-bit values than 16-bit (at least 
> it was issue before).
> 
> Vladimir
> 

Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 Instructions

[Xueming Shen]

Hi Kamath,

Instead of throwing an aiobe, should the generateImplEncode() be like

void generateImplEncode(byte[] src, int sp, int sl, byte[] dst, int dp) {
if (sp < sl) {
implEncode(src, sp, sl, dst, dp, ...);
}
}

Any benefit of separating it into its own method?

Thanks,
Sherman

On 6/22/18, 2:49 PM, Kamath, Smita wrote:

Hi Vladimir,

Please see my answers to your questions as below:

1) One question so: why you have own copy of base64 charsets and not using one 
in library:
I am using vpgatherdd instruction to fetch multiple values from base64 array in 
a single instruction with a vector index. Vpgatherdd instruction works on 32 
bit array and so I need to define base64 charset in a 32 bit array. I have 
given reference to gather instruction below.

2) Some indents in new and old Assembler::emit_operand() are off. In new 
Assembler::emit_operand() is better use } else { instead of 'return' in one 
branch.
I'll make the necessary changes and send an updated webrev.

3) This is first time I see that XMM register can be used for index in address. 
Is it true? Can you point to Intel's document which describes it.
I am using vpgatherdd instruction which requires index vector with scale. It 
uses VSIB addressing where the index register is a zmm register.
Please refer to reference manual, volume 2c, page 2211: 
https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf
Also see, section 2.3.12, page 524 for VSIB memory addressing information.

4)  Please, add tests to test/hotspot/jtreg/compiler/intrinsics/base64 (may be 
similar to sha or AES in compiler/codegen/aes) to make sure that all flags, 
intrinsic is used and it produces correct result.
I will add test cases as per your suggestion.

Please let me know if you have additional questions.

Thanks,
Smita

-Original Message-
From: Vladimir Kozlov [mailto:vladimir.koz...@oracle.com]
Sent: Friday, June 22, 2018 12:29 PM
To: Kamath, Smita
Cc: hotspot compiler
Subject: Re: RFR(S) JDK-8205528: Base64 Encode Algorithm using AVX512 
Instructions

Hi Smita,

I CCing to Libs to review code changes in Base64.java.

Looks like you changed all need place to implement intrinsic.
One question so: why you have own copy of base64 charsets and not using one in 
library:

   private int encode0(byte[] src, int off, int end, byte[] dst) {
   char[] base64 = isURL ? toBase64URL : toBase64;

Some indents in new and old Assembler::emit_operand() are off.

In new Assembler::emit_operand() is better use } else { instead of 'return' in 
one branch.

This is first time I see that XMM register can be used for index in address. Is 
it true? Can you point to Intel's document which describes it.

What testing you did?

Please, add tests to test/hotspot/jtreg/compiler/intrinsics/base64 (may be 
similar to sha or AES in compiler/codegen/aes) to make sure that all flags, 
intrinsic is used and it produces correct result.

I know there is test/jdk/java/util/Base64/ tests but they may not trigger 
intrinsic usage. But you can use them as starting point for new tests.

Thanks,
Vladimir

On 6/22/18 11:47 AM, Kamath, Smita wrote:

Hi Vladimir,

I'd like to contribute an optimization for Base64 Encoding Algorithm
using AVX512 Instructions. This optimization shows 1.5x improvement on
x86_64 platform(SKL).

Link to Bug: https://bugs.openjdk.java.net/browse/JDK-8205528

Link to webrev:
http://cr.openjdk.java.net/~vdeshpande/Base64/webrev.00/

For testing the implementation, I have run tests under
test/jdk/java/util/Base64/ and also ran
test/jdk/com/sun/jndi/ldap/Base64Test.java

I have also run jtreg.

Please review and let me know if you have any comments.

Thanks and Regards,

Smita

Re: RFR 8195650 Method references to VarHandle accessors

[Paul Sandoz]

Gentle reminder.

I would like to get this reviews and pushed before the ramp down phase one 
kicks in this week.

Paul.

> On Jun 19, 2018, at 5:08 PM, Paul Sandoz  wrote:
> 
> Hi,
> 
> Please review the following fix to ensure method references to VarHandle 
> signature polymorphic methods are supported at runtime (specifically the 
> method handle to a signature polymorphic method can be loaded from the 
> constant pool):
> 
>  http://cr.openjdk.java.net/~psandoz/jdk/JDK-8195650-varhandle-mref/webrev/ 
> 
> 
> I also added a “belts and braces” test to ensure a constant method handle to 
> MethodHandle::invokeBasic cannot be loaded if outside of the j.l.invoke 
> package.
> 
> Paul.
> 

Re: RFR: jsr166 integration 2018-06

[Paul Sandoz]

> On Jun 24, 2018, at 1:29 PM, Doug Lea  wrote:
> 
> On 06/22/2018 01:28 PM, Paul Sandoz wrote:
> 
>>> 8203864: Execution error in Java's Timsort
>>> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/TimSort/index.html
>>> https://bugs.openjdk.java.net/browse/JDK-8203864
>>> 
>> 
>> 406 if (n > 0 && runLen[n-1] <= runLen[n] + runLen[n+1] ||
>> 407 n > 1 && runLen[n-2] <= runLen[n] + runLen[n-1]) {
>> 408 if (runLen[n - 1] < runLen[n + 1])
>> 409 n--;
>> 
>> Minor comment, would be good if consistent style is used for the index 
>> expressions. I have a marginal preference to retaining ordering from a 
>> “picture this in my head" kind of thing, specifically:
>> 
>>  runLen[n - 2] <= runLen[n - 1] + runLen[n]
> 
> I marginally agree, but I'm keeping it this way just for boring
> consistency with published versions.
> 

Ok, consistency overrules!

Paul.

Re: [11]RFR 8194152: sun/security/tools/jarsigner/AltProvider.java failed on de-DE locale

[Naoto Sato]

Looks good.

Naoto

On 6/25/18 12:37 AM, Dora Zhou wrote:

Hi Naoto,

Thanks a lot for the review.
I have made suggested changes, Kindly have a look at: 
http://cr.openjdk.java.net/~ljiang/8194152/webrev.01/ 



Regards,
Dora


From: naoto.s...@oracle.com
To: dan.z.z...@oracle.com, i18n-...@openjdk.java.net, 
core-libs-dev@openjdk.java.net, security-...@openjdk.java.net
Sent: Saturday, June 23, 2018 1:23:50 AM GMT +08:00 Beijing / 
Chongqing / Hong Kong / Urumqi
Subject: Re:  [11]RFR 8194152: 
sun/security/tools/jarsigner/AltProvider.java failed on de-DE locale


Hi Dora,

You could move those two lines that sets the locale to en-US before the
for-loop, so that if "args" contains -J-Duser.language/country then it
can override the default en-US.

The JIRA issue needs noreg-self label.

Naoto

On 6/22/18 1:26 AM, Dora Zhou wrote:

Hello,

Please help review the fix for JDK-8194152. Thank you.

The test compares output with expected error messages in English, set
locale to en-US so that the output are not translated into other 
languages.


Bug: https://bugs.openjdk.java.net/browse/JDK-8194152
Webrev: http://cr.openjdk.java.net/~ljiang/8194152/webrev/


Regards,
Dora

RFR 8205547 : FileChannel/CleanerTest.java fails due to expected FD count

[Roger Riggs]

Please review a test improvement to avoid unexpected file opens/closes 
during the file channel CleanerTest.
The test now follows the pattern of the other 
java/io/File*/UnreferencedXXXClosesFd tests by waiting

for the cleaner and file descriptors to be reclaimed by GC.

Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-fd-count-wrong-8205547/

Thanks, Roger

Re: RFR JDK-8205090 / JDK-8204930

[Brian Burkhalter]

Hi Patrick, Roger,

On Jun 25, 2018, at 7:22 AM, Roger Riggs  wrote:

> Looks fine,  thanks for the followup
> 
> @Brian, please sponsor 

Will do today.

Brian

Re: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

[Alan Bateman]

On 25/06/2018 15:29, Baesken, Matthias wrote:

Hi, do you consider  both  the file name and  line number as sensitive ?


There was a similar discussion on net-dev recently related to
leaking host names in exceptions. Something similar may be needed here


Do you know the outcome of this discussion ?


All the details are in JDK-8204233 and the associated CSR.

-Alan

RE: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

[Baesken, Matthias]

Hi, do you consider  both  the file name and  line number as sensitive ?

>
> There was a similar discussion on net-dev recently related to
> leaking host names in exceptions. Something similar may be needed here
>

Do you know the outcome of this discussion ?

Best regards, Matthias



> -Original Message-
> From: Alan Bateman [mailto:alan.bate...@oracle.com]
> Sent: Montag, 25. Juni 2018 16:17
> To: Baesken, Matthias ; core-libs-
> d...@openjdk.java.net
> Subject: Re: [RFR] 8205525 : Improve exception messages during manifest
> parsing of jar archives
> 
> On 25/06/2018 14:55, Baesken, Matthias wrote:
> > Hello, please review this small change  that  improve exception messages
> during manifest parsing of jar archives .
> >
> > Thanks, Matthias
> >
> >Bug :
> >
> > https://bugs.openjdk.java.net/browse/JDK-8205525
> >
> >Webrev :
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8205525/
> This potentially leaks sensitive information and will require a security
> review. There was a similar discussion on net-dev recently related to
> leaking host names in exceptions. Something similar may be needed here.
> 
> -Alan.

Re: RFR JDK-8205090 / JDK-8204930

[Roger Riggs]

Hi Patrick,

Looks fine,  thanks for the followup

@Brian, please sponsor

Thanks, Roger

On 6/25/2018 9:37 AM, Patrick Reinhart wrote:

Can anyone take this?

-Patrick

Am 23.06.2018 um 02:26 schrieb Brian Burkhalter 
mailto:brian.burkhal...@oracle.com>>:



Hi Patrick,

I reviewed the CSR with some minor changes as you will have seen. The 
webrev looks OK. I’ve not been following this thread that closely so 
perhaps someone else should also review it. I can be the committer if 
need be.


Thanks,

Brian

On Jun 22, 2018, at 11:21 AM, Patrick Reinhart > wrote:


The JDK 11 ramp down fase is coming soon and I would like to have 
that fixed before that. Please review the CSR and the proposed webrev:


http://cr.openjdk.java.net/~reinhapa/reviews/8204930/webrev 



Thanks for your replies and a sponsoring committer

Re: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

[Alan Bateman]

On 25/06/2018 14:55, Baesken, Matthias wrote:

Hello, please review this small change  that  improve exception messages during 
manifest parsing of jar archives .

Thanks, Matthias

   Bug :

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

   Webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8205525/
This potentially leaks sensitive information and will require a security 
review. There was a similar discussion on net-dev recently related to 
leaking host names in exceptions. Something similar may be needed here.


-Alan.

[RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

[Baesken, Matthias]

Hello, please review this small change  that  improve exception messages during 
manifest parsing of jar archives .

Thanks, Matthias

  Bug :

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

  Webrev :

http://cr.openjdk.java.net/~mbaesken/webrevs/8205525/

Re: RFR JDK-8205090 / JDK-8204930

[Patrick Reinhart]

Can anyone take this?

-Patrick 

> Am 23.06.2018 um 02:26 schrieb Brian Burkhalter :
> 
> Hi Patrick,
> 
> I reviewed the CSR with some minor changes as you will have seen. The webrev 
> looks OK. I’ve not been following this thread that closely so perhaps someone 
> else should also review it. I can be the committer if need be.
> 
> Thanks,
> 
> Brian
> 
>> On Jun 22, 2018, at 11:21 AM, Patrick Reinhart  wrote:
>> 
>> The JDK 11 ramp down fase is coming soon and I would like to have that fixed 
>> before that. Please review the CSR and the proposed webrev:
>> 
>> http://cr.openjdk.java.net/~reinhapa/reviews/8204930/webrev
>> 
>> Thanks for your replies and a sponsoring committer
> 

Re: RFR 8202292 : test/jdk/java/io/FileOutputStream/UnreferencedFOSClosesFd.java fails with "raw fd count wrong"

[Roger Riggs]

Created new issue: https://bugs.openjdk.java.net/browse/JDK-8205610

Thanks for the suggestion David

On 6/25/2018 1:00 AM, David Holmes wrote:

Hi Roger,

On 23/06/2018 12:49 AM, Roger Riggs wrote:

Hi Brian, Mandy,

Listing the open file descriptors can be a useful utility. Originally 
I thought it was a temporary and throw away code.
To be a utility it needed to be a bit more general (sending the 
output to a particular stream).

Thanks for the suggestions.


You might want to look at the code Leo used for a hotspot test 
recently. It covers more locations for lsof and also supports pfiles 
for Solaris:


http://hg.openjdk.java.net/jdk/jdk/rev/9ba6f5dfbe56
http://hg.openjdk.java.net/jdk/jdk/rev/1372f66e0a17

But there can be other locations like /usr/local/bin for lsof on 
Solaris too.


David


Please review:
http://cr.openjdk.java.net/~rriggs/webrev-count-wrong-8202292/

Thanks, Roger

On 6/21/2018 3:25 PM, Brian Burkhalter wrote:

Hi Roger,

Are the three versions of listProcFD() identical? Is so, could this 
method be put in test/lib/jdk/test/lib/util/FileUtils.java instead?


Thanks,

Brian

On Jun 21, 2018, at 11:59 AM, Roger Riggs > wrote:


Please review a test improvement to avoid spurious errors caused by 
concurrent changes in open files during the test.
The other existing conditions in the test are sufficient to verify 
the correct finalize behavior.
Some additional diagnostics are added and the tests removed from 
the problem list.


Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-count-wrong-8202292/index.html 
 



Issue:
https://bugs.openjdk.java.net/browse/JDK-8202292

Re: free(): corrupted unsorted chunks

[Oliver Kohll]

Hi Bernd,

Many thanks for your suggestions. It looks like the native APR library was
the issue, at least it hasn't crashed yet after removing that library

https://tomcat.apache.org/tomcat-8.0-doc/apr.html (the libapr1 package in
Ubuntu)

and re-configuring the /etc/tomcat8/server.xml to use NIO for SSL,
replacing org.apache.coyote.http11.Http11AprProtocol with
org.apache.coyote.http11.Http11NioProtocol.

I appreciate your help, it saved my bacon!

Oliver

On 21 June 2018 at 23:34:44, Oliver Kohll (oli...@agilechilli.com) wrote:

Aha, I wonder if it's configured with the APR native library for SSL. I
will check out that and those things.

Thanks
Oliver

On 21 June 2018 at 21:43:24, Bernd Eckenfels (e...@zusammenkunft.net) wrote:

Are you using any native libraries in this VM, is there a hs_err or System
core file? Can you run „ldd“ on the java launcher binary. Can you try a
OpenJDK binary distribution independent of the OS compiled version?

Gruss
Bernd
-- 
http://bernd.eckenfels.net

Re: [11]RFR 8194152: sun/security/tools/jarsigner/AltProvider.java failed on de-DE locale

[Dora Zhou]

Hi Naoto,

Thanks a lot for the review.
I have made suggested changes, Kindly have a look at: 
http://cr.openjdk.java.net/~ljiang/8194152/webrev.01/ 



Regards,
Dora


From: naoto.s...@oracle.com
To: dan.z.z...@oracle.com, i18n-...@openjdk.java.net, 
core-libs-dev@openjdk.java.net, security-...@openjdk.java.net
Sent: Saturday, June 23, 2018 1:23:50 AM GMT +08:00 Beijing / Chongqing / Hong 
Kong / Urumqi
Subject: Re:  [11]RFR 8194152: 
sun/security/tools/jarsigner/AltProvider.java failed on de-DE locale

Hi Dora,

You could move those two lines that sets the locale to en-US before the
for-loop, so that if "args" contains -J-Duser.language/country then it
can override the default en-US.

The JIRA issue needs noreg-self label.

Naoto

On 6/22/18 1:26 AM, Dora Zhou wrote:

Hello,

Please help review the fix for JDK-8194152. Thank you.

The test compares output with expected error messages in English, set
locale to en-US so that the output are not translated into other languages.

Bug: https://bugs.openjdk.java.net/browse/JDK-8194152
Webrev: http://cr.openjdk.java.net/~ljiang/8194152/webrev/


Regards,
Dora

[11]RFR 8196213: sun/security/tools/jarsigner/warnings/NoTimestampTest.java test fails on ar_SA locale.

[Dora Zhou]

Hello,

Please help review the fix for JDK-8196213. Thank you.
Set default locale to en-US so that the output display the date using 
Gregorian Calendar and Latn numbering system(0~9).


Bug: https://bugs.openjdk.java.net/browse/JDK-8196213
Webrev: http://cr.openjdk.java.net/~ljiang/8196213/webrev/ 



Regards,
Dora