Re: Additional Date-Time Formats

2022-01-20 Thread Joe Wang

Hi Naoto,

The javadoc points to LDML, it seems to me though it might be useful to 
add more information similar to that for the ofPattern methods, what's 
under the "Patterns for Formatting and Parsing" section, so that for at 
least the common use cases we could rely on the javadoc without having 
to consult the LDML specification. Some comparison with the ofPattern 
methods might also be helpful.


Just my 2 cents.

Thanks,
Joe

On 1/20/22 1:46 PM, Naoto Sato wrote:

Hello,

I am proposing a couple of new factory methods in 
java.time.format.DateTimeFormatter that produce flexible localized 
date/time formats, other than the existing pre-defined 
(FULL/LONG/MEDIUM/SHORT) styles. For example, if the user wants a year 
and month only string, such as the title for the calendar, currently 
they would have to use DateTimeFormatter.ofPattern() with explicit 
pattern argument, such as "MMM y". This is problematic because it is 
correct in locales such as US, but not correct in other locales.


So, I propose methods that are parallel to ofPattern(), which take 
pattern template. This is based on the CLDR's skeleton: 
https://www.unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems


Detailed design can be found in the draft CSR: 
https://bugs.openjdk.java.net/browse/JDK-8243445


Comments are welcome.

Naoto





Re: RFR: 8279508: Auto-vectorize Math.round API [v2]

2022-01-20 Thread Sandhya Viswanathan
On Wed, 19 Jan 2022 17:38:25 GMT, Jatin Bhateja  wrote:

>> Summary of changes:
>> - Intrinsify Math.round(float) and Math.round(double) APIs.
>> - Extend auto-vectorizer to infer vector operations on encountering scalar 
>> IR nodes for above intrinsics.
>> - Test creation using new IR testing framework.
>> 
>> Following are the performance number of a JMH micro included with the patch 
>> 
>> Test System: Intel(R) Xeon(R) Platinum 8380 CPU @ 2.30GHz (Icelake Server)
>> 
>>   |   | BASELINE AVX2 | WithOpt AVX2 | Gain (opt/baseline) | Baseline AVX3 | 
>> Withopt AVX3 | Gain (opt/baseline)
>> -- | -- | -- | -- | -- | -- | -- | --
>> Benchmark | ARRAYLEN | Score (ops/ms) | Score (ops/ms) |   | Score (ops/ms) 
>> | Score (ops/ms) |  
>> FpRoundingBenchmark.test_round_double | 1024 | 518.532 | 1364.066 | 
>> 2.630630318 | 512.908 | 4292.11 | 8.368186887
>> FpRoundingBenchmark.test_round_double | 2048 | 270.137 | 830.986 | 
>> 3.076165057 | 273.159 | 2459.116 | 9.002507697
>> FpRoundingBenchmark.test_round_float | 1024 | 752.436 | 7780.905 | 
>> 10.34095259 | 752.49 | 9506.694 | 12.63364829
>> FpRoundingBenchmark.test_round_float | 2048 | 389.499 | 4113.046 | 
>> 10.55983712 | 389.63 | 4863.673 | 12.48279907
>> 
>> Kindly review and share your feedback.
>> 
>> Best Regards,
>> Jatin
>
> Jatin Bhateja has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8279508: Adding a test for scalar intrinsification.

The JVM currently initializes the x86 mxcsr to round to nearest even, see below 
in stubGenerator_x86_64.cpp:
// Round to nearest (even), 64-bit mode, exceptions masked
StubRoutines::x86::_mxcsr_std = 0x1F80;
The above works for Math.rint which is specified to be round to nearest even.
Please see:
https://www.intel.com/content/www/us/en/developer/articles/technical/intel-sdm.html
 : section 4.8.4

The rounding mode needed for Math.round is round to positive infinity which 
needs a different x86 mxcsr initialization(0x5F80).

-

PR: https://git.openjdk.java.net/jdk/pull/7094


Re: RFR: JDK-8280373: Update Xalan serializer / SystemIDResolver to align with JDK-8270492

2022-01-20 Thread Joe Wang
On Thu, 20 Jan 2022 10:55:59 GMT, Matthias Baesken  wrote:

> After 8270492
> https://github.com/openjdk/jdk/commit/78b2c8419bc69436873e6fc9c542480949d140c5
> has been pushed, we should adjust 
> src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/utils/SystemIDResolver.java
>  getAbsoluteURI to what has been done in 8270492 to 
> src/java.xml/share/classes/com/sun/org/apache/xml/internal/utils/SystemIDResolver.java

Thanks Alan for the reminder. The change looks good. Please add a copyright 
header and the LastModified tag as the other class did.

-

PR: https://git.openjdk.java.net/jdk/pull/7155


Re: RFR: JDK-8280168 Add Objects.toDefaultString [v5]

2022-01-20 Thread Joe Darcy
On Thu, 20 Jan 2022 23:20:37 GMT, Joe Darcy  wrote:

>> While it is strongly recommend to not use the default toString for a class, 
>> at times it is the least-bad alternative. When that alternative needs to be 
>> used, it would be helpful to have the implementation already available, such 
>> as in Objects.toDefaultString(). This method is analagous to 
>> System.identityHashCode.
>> 
>> Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8280184
>
> Joe Darcy has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Appease jcheck.

Expanding the scope of the proposed change, perhaps outside of a good 
cost/benefit ratio, there are now two new methods:

toDefaultString(Object o) // uses o.hashCode
toIdentityString(Object o) // uses System.identityHashCode(o)

The revision refactors two occurrences of the logic of toDefaultString with 
calls to the new method.

The toIdentityString method has the sometimes helpful property that no 
overridden methods of the object are called in constructing the string.

I'll revise the CSR once the final set of methods and their spec for this fix 
is determined.

-

PR: https://git.openjdk.java.net/jdk/pull/7139


Re: RFR: JDK-8280168 Add Objects.toDefaultString [v5]

2022-01-20 Thread Joe Darcy
> While it is strongly recommend to not use the default toString for a class, 
> at times it is the least-bad alternative. When that alternative needs to be 
> used, it would be helpful to have the implementation already available, such 
> as in Objects.toDefaultString(). This method is analagous to 
> System.identityHashCode.
> 
> Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8280184

Joe Darcy has updated the pull request incrementally with one additional commit 
since the last revision:

  Appease jcheck.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7139/files
  - new: https://git.openjdk.java.net/jdk/pull/7139/files/48b596e9..0f6f0786

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7139=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7139=03-04

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7139.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7139/head:pull/7139

PR: https://git.openjdk.java.net/jdk/pull/7139


Re: RFR: JDK-8280168 Add Objects.toDefaultString [v4]

2022-01-20 Thread Joe Darcy
> While it is strongly recommend to not use the default toString for a class, 
> at times it is the least-bad alternative. When that alternative needs to be 
> used, it would be helpful to have the implementation already available, such 
> as in Objects.toDefaultString(). This method is analagous to 
> System.identityHashCode.
> 
> Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8280184

Joe Darcy has updated the pull request incrementally with one additional commit 
since the last revision:

  Add toIdentityString

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7139/files
  - new: https://git.openjdk.java.net/jdk/pull/7139/files/a854766b..48b596e9

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7139=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7139=02-03

  Stats: 66 lines in 4 files changed: 62 ins; 0 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7139.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7139/head:pull/7139

PR: https://git.openjdk.java.net/jdk/pull/7139


Withdrawn: 8277155: Compress and expand vector operations

2022-01-20 Thread Paul Sandoz
On Wed, 24 Nov 2021 19:20:08 GMT, Paul Sandoz  wrote:

> Add two new cross-lane vector operations, `compress` and `expand`.
> 
> An example of such usage might be code that selects elements from array `a` 
> and stores those selected elements in array `z`:
> 
> 
> int[] a = ...;
> 
> int[] z = ...;
> int ai = 0, zi = 0;
> while (ai < a.length) {
> IntVector av = IntVector.fromArray(SPECIES, a, ai);
> // query over elements of vector av
> // returning a mask marking elements of interest
> VectorMask m = interestingBits(av, ...);
> IntVector zv = av.compress(m);
> zv.intoArray(z, zi, m.compress());
> ai += SPECIES.length();
> zi += m.trueCount();
> }
> 
> 
> (There's also a more sophisticated version using `unslice` to coalesce 
> matching elements with non-masked stores.)
> 
> Given RDP 1 for 18 is getting close, 2021/12/09, we may not get this reviewed 
> in time and included in [JEP 417](https://openjdk.java.net/jeps/417). Still I 
> think I think it worth starting the review now (the CSR is marked 
> provisional).

This pull request has been closed without being integrated.

-

PR: https://git.openjdk.java.net/jdk/pull/6545


Re: RFR: 8277155: Compress and expand vector operations

2022-01-20 Thread Paul Sandoz
On Wed, 24 Nov 2021 19:20:08 GMT, Paul Sandoz  wrote:

> Add two new cross-lane vector operations, `compress` and `expand`.
> 
> An example of such usage might be code that selects elements from array `a` 
> and stores those selected elements in array `z`:
> 
> 
> int[] a = ...;
> 
> int[] z = ...;
> int ai = 0, zi = 0;
> while (ai < a.length) {
> IntVector av = IntVector.fromArray(SPECIES, a, ai);
> // query over elements of vector av
> // returning a mask marking elements of interest
> VectorMask m = interestingBits(av, ...);
> IntVector zv = av.compress(m);
> zv.intoArray(z, zi, m.compress());
> ai += SPECIES.length();
> zi += m.trueCount();
> }
> 
> 
> (There's also a more sophisticated version using `unslice` to coalesce 
> matching elements with non-masked stores.)
> 
> Given RDP 1 for 18 is getting close, 2021/12/09, we may not get this reviewed 
> in time and included in [JEP 417](https://openjdk.java.net/jeps/417). Still I 
> think I think it worth starting the review now (the CSR is marked 
> provisional).

Withdrawing and will create a new PR corresponding to a new incubating JEP.

-

PR: https://git.openjdk.java.net/jdk/pull/6545


Additional Date-Time Formats

2022-01-20 Thread Naoto Sato

Hello,

I am proposing a couple of new factory methods in 
java.time.format.DateTimeFormatter that produce flexible localized 
date/time formats, other than the existing pre-defined 
(FULL/LONG/MEDIUM/SHORT) styles. For example, if the user wants a year 
and month only string, such as the title for the calendar, currently 
they would have to use DateTimeFormatter.ofPattern() with explicit 
pattern argument, such as "MMM y". This is problematic because it is 
correct in locales such as US, but not correct in other locales.


So, I propose methods that are parallel to ofPattern(), which take 
pattern template. This is based on the CLDR's skeleton: 
https://www.unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems


Detailed design can be found in the draft CSR: 
https://bugs.openjdk.java.net/browse/JDK-8243445


Comments are welcome.

Naoto



RFR: 8251466: test/java/io/File/GetXSpace.java fails on Windows with mapped network drives.

2022-01-20 Thread Andrey Turbanov
Test `test/java/io/File/GetXSpace.java` always failed on my machine with this 
output:

--System.out:(12/489)--
--- Testing df
C:/Programs/cygwin64 292848636 49695320 243153316 17% /
D: 59672 59672 0 100% /cygdrive/d


SecurityManager = null
C:/Programs/cygwin64:
  df total= 299877003264 free = 0 usable = 248988995584
  getX total= 299877003264 free = 248988995584 usable = 248988995584
::
  df total= 61104128 free = 0 usable = 0
  getX total= 0 free = 0 usable = 0
--System.err:(23/1617)--
java.nio.file.InvalidPathException: Illegal char <:> at index 0: :
at java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92)
at java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:232)
at java.base/java.io.File.toPath(File.java:2396)
at GetXSpace.compare(GetXSpace.java:223)
at GetXSpace.testDF(GetXSpace.java:403)
at GetXSpace.main(GetXSpace.java:437)
at 
java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
at java.base/java.lang.reflect.Method.invoke(Method.java:577)
at 
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
at java.base/java.lang.Thread.run(Thread.java:833) 

Parsing of df output is incorrect. It skips first symbols after matching of 
line.
https://github.com/openjdk/jdk/blob/293fb46f7cd28f2a08055e3eb8ec9459d64e9688/test/jdk/java/io/File/GetXSpace.java#L160

It means that after matching first line:

C:/Programs/cygwin64 292848636 49695320 243153316 17% /


It skips first symbols of next line - symbol `D` and then proceeds to match 
_corrupted_ line:

: 59672 59672 0 100% /cygdrive/d


Problems affects only Windows, because only in Windows case first group of 
matcher is used:

https://github.com/openjdk/jdk/blob/293fb46f7cd28f2a08055e3eb8ec9459d64e9688/test/jdk/java/io/File/GetXSpace.java#L155-L156


Testing:
* Windows x64 - always failed before. No errors after fix
* Linux x64

-

Commit messages:
 - 8251466: test/java/io/File/GetXSpace.java fails on Windows with mapped 
network drives.

Changes: https://git.openjdk.java.net/jdk/pull/7170/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7170=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8251466
  Stats: 6 lines in 1 file changed: 0 ins; 1 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7170.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7170/head:pull/7170

PR: https://git.openjdk.java.net/jdk/pull/7170


Re: LambdaMetafactory requires full privilege access, but doesn't seem to actually restrict functionality

2022-01-20 Thread Michael Kuhlmann

Hi Steven!

On 1/19/22 2:23 AM, Steven Schlansker wrote:

Interestingly, that wasn't what I found.  At least as of Java 14, the 
Metafactory generation approach (as limited as my first pass was) was 
competitive with CGLib hand-generated code in many (but not all) cases, and 
significantly faster than Method.invoke:
https://github.com/FasterXML/jackson-modules-base/tree/2.14/blackbird/benchmarks
Unfortunately I don't have a MethodHandle.invokeExact comparison in that 
benchmark. I should go back and finish that work, and re-run it all on 17. If 
we can call non-static MethodHandles with little overhead, then any reason to 
use the Metafactory here goes away.


Very interesting! Thank you for sharing it.
It's true that for non-static MethodHandles it's a similar problem as 
for generic lambdas: Hotspot can't easily inline the call. I wouldn't 
have expected that MethodHandles are even slower than lambas, but 
honestly I never measured it.





Really, I'm curious if this could be an approach for Jackson. Or if not, what 
could be the obstacles.


I think the problem here is just slightly different: your approach will copy 
from one Java object to another Java object, but what we are doing is reacting 
to a token stream and trying to bind it to Java objects with as little overhead 
as possible.


So I assume your token handler is getting the caller (be it a handle, a 
lambda or whatever) from some key-value store (maybe a Map) and 
injecting the value into it.


What you can try is using MethodHandles::exactInvoker combined with 
foldArguments, where the token-to-handle mapper is already injected. 
The handle would just expect the token key and the value as arguments. 
Maybe this would perform better then the non-static setter handles.


I did this for another use case where the target handle was the 
dynamicInvoker of a MutableCallSite. The handle then lazily generated 
its own final handle on the first call and set it into its CallSite, so 
that all subsequent calls ran directly into the generated handle. It was 
working great and a lot of fun to code, but again, I never measured the 
performance in detail.


I wish you good luck!
Michael


Re: RFR: 8280166: Extend java/lang/instrument/GetObjectSizeIntrinsicsTest.java test cases

2022-01-20 Thread Serguei Spitsyn
On Tue, 18 Jan 2022 18:33:29 GMT, Aleksey Shipilev  wrote:

> While working on JDK-8280003, I noticed that 
> java/lang/instrument/GetObjectSizeIntrinsicsTest.java does not test arrays 
> with more than 1-byte size elements, and no large arrays (past 4G limit) are 
> tested either. It would be better to add those test cases. 
> 
> Additional testing:
>  - [x] Linux x86_64 fastdebug, affected test still passes
>  - [x] Linux x86_32 fastdebug, affected test still passes
>  - [x] Linux AArch64 fastdebug, affected test still passes
>  - [x] Linux PPC64 fastdebug, affected test still passes

Looks okay to me.
Thanks,
Serguei

-

Marked as reviewed by sspitsyn (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7132


Integrated: 8277535: Remove redundant Stream.distinct()/sorted() steps

2022-01-20 Thread Andrey Turbanov
On Mon, 27 Sep 2021 11:20:53 GMT, Andrey Turbanov  wrote:

> 1. Stream.distinct() is redundant before toSet() collector. Duplicates will 
> be collapsed by Collector.
> 2. Stream.sorted() is redundant before toMap() collector. Keys will be 
> shuffled by Collector (it's a HashMap in current implementation)

This pull request has now been integrated.

Changeset: 3419ff7b
Author:Andrey Turbanov 
URL:   
https://git.openjdk.java.net/jdk/commit/3419ff7ba70b778906249dd5ab3a91998ca5a864
Stats: 6 lines in 3 files changed: 0 ins; 4 del; 2 mod

8277535: Remove redundant Stream.distinct()/sorted() steps

Reviewed-by: prappo

-

PR: https://git.openjdk.java.net/jdk/pull/5714


Proposed JEP: Safer Process Launch by ProcessBuilder and Runtime.exec

2022-01-20 Thread Roger Riggs
A JEP to Improve safety of process launch by ProcessBuilder and 
Runtime.exec on Windows[1].


Argument encoding errors have been problematic on Windows systems due to
improperly quoted command arguments.

The idea is to tighten up quoting and encoding of command line arguments.

Comments appreciated,  Roger

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


Re: RFR: JDK-8280373: adjust other SystemIDResolver.java after 8270492

2022-01-20 Thread Alan Bateman
On Thu, 20 Jan 2022 10:55:59 GMT, Matthias Baesken  wrote:

> After 8270492
> https://github.com/openjdk/jdk/commit/78b2c8419bc69436873e6fc9c542480949d140c5
> has been pushed, we should adjust 
> src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/utils/SystemIDResolver.java
>  getAbsoluteURI to what has been done in 8270492 to 
> src/java.xml/share/classes/com/sun/org/apache/xml/internal/utils/SystemIDResolver.java

It would be useful if Joe Wang can review this, just in case the security 
changes did not touch this code for some reason.

-

PR: https://git.openjdk.java.net/jdk/pull/7155


Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v3]

2022-01-20 Thread Daniel Fuchs
On Thu, 20 Jan 2022 10:58:27 GMT, Michael McMahon  wrote:

>> Hi,
>> 
>> This change adds Channel Binding Token (CBT) support to HTTPS 
>> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, 
>> Kerberos) authentication scheme. When enabled, the implementation 
>> preemptively includes a CBT with authentication requests over Kerberos. The 
>> feature is enabled as follows:
>> 
>> A system property "jdk.spnego.cbt" is defined which can have the values 
>> "never" (default), which means the feature is disabled, "always", which 
>> means the CBT is included for all https Negotiate authentications, or it can 
>> take the form "domain:a,b.c,*.d.com" which is a comma separated list of 
>> domains/hosts where the feature is enabled, and disabled everywhere else. In 
>> the given example, the CBT would be included in authentication requests for 
>> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its 
>> sub-domains.
>> 
>> A test will be added separately to the implementation.
>> 
>> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>> 
>> Thanks,
>> Michael
>
> Michael McMahon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   removed sasl module dependency and added SaslException cause

src/java.base/share/classes/java/net/doc-files/net-properties.html line 220:

> 218:  This controls the generation and sending of TLS channel binding tokens 
> (CBT) when Kerberos 
> 219: or the Negotiate authentication scheme using Kerberos are 
> employed over HTTPS with 
> 220: {@code HttpURLConnection}. There are three possible settings:

Should it be `{@code HttpsURLConnection}`?
(BTW - can we use {@code } here ? Would be worth checking the generated doc)

src/java.base/share/classes/sun/net/www/http/HttpClient.java line 189:

> 187: } else {
> 188: logError("Unexpected value for \"jdk.https.negotiate.cbt\" 
> system property");
> 189: return s;

Should this return either "always" or "never" instead? It seems that junk 
values will be treated as "always". It would be better to make it clear here.

src/java.base/share/classes/sun/security/util/ChannelBindingException.java line 
31:

> 29:  * Thrown by TlsChannelBinding if an error occurs
> 30:  */
> 31: public class ChannelBindingException extends Exception {

Should this extend `GeneralSecurityException` instead? Or should we just remove 
this class and throw plain `GeneralSecurityException` in `TlsChannelBinding` ?

src/java.naming/share/classes/com/sun/jndi/ldap/sasl/LdapSasl.java line 143:

> 141: tlsCB = TlsChannelBinding.create(cert);
> 142: } catch (ChannelBindingException e) {
> 143: throw new SaslException(e.getMessage());

Why is there a difference compared to line 133?

-

PR: https://git.openjdk.java.net/jdk/pull/7065


Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v3]

2022-01-20 Thread Michael Osipov
On Thu, 20 Jan 2022 10:58:27 GMT, Michael McMahon  wrote:

>> Hi,
>> 
>> This change adds Channel Binding Token (CBT) support to HTTPS 
>> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, 
>> Kerberos) authentication scheme. When enabled, the implementation 
>> preemptively includes a CBT with authentication requests over Kerberos. The 
>> feature is enabled as follows:
>> 
>> A system property "jdk.spnego.cbt" is defined which can have the values 
>> "never" (default), which means the feature is disabled, "always", which 
>> means the CBT is included for all https Negotiate authentications, or it can 
>> take the form "domain:a,b.c,*.d.com" which is a comma separated list of 
>> domains/hosts where the feature is enabled, and disabled everywhere else. In 
>> the given example, the CBT would be included in authentication requests for 
>> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its 
>> sub-domains.
>> 
>> A test will be added separately to the implementation.
>> 
>> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
>> 
>> Thanks,
>> Michael
>
> Michael McMahon has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   removed sasl module dependency and added SaslException cause

src/java.naming/share/classes/com/sun/jndi/ldap/sasl/LdapSasl.java line 133:

> 131: 
> (String)env.get(TlsChannelBinding.CHANNEL_BINDING_TYPE));
> 132: } catch (ChannelBindingException e) {
> 133: throw new SaslException(e.getMessage(), e);

Why not ust pass the exception if the API allows? This looks like message 
duplication.

-

PR: https://git.openjdk.java.net/jdk/pull/7065


Re: RFR: JDK-8280373: adjust other SystemIDResolver.java after 8270492

2022-01-20 Thread Yuri Nesterenko
On Thu, 20 Jan 2022 10:55:59 GMT, Matthias Baesken  wrote:

> After 8270492
> https://github.com/openjdk/jdk/commit/78b2c8419bc69436873e6fc9c542480949d140c5
> has been pushed, we should adjust 
> src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/utils/SystemIDResolver.java
>  getAbsoluteURI to what has been done in 8270492 to 
> src/java.xml/share/classes/com/sun/org/apache/xml/internal/utils/SystemIDResolver.java

LGTM!

-

Marked as reviewed by yan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7155


RFR: JDK-8280373: adjust other SystemIDResolver.java after 8270492

2022-01-20 Thread Matthias Baesken
After 8270492
https://github.com/openjdk/jdk/commit/78b2c8419bc69436873e6fc9c542480949d140c5
has been pushed, we should adjust 
src/java.xml/share/classes/com/sun/org/apache/xml/internal/serializer/utils/SystemIDResolver.java
 getAbsoluteURI to what has been done in 8270492 to 
src/java.xml/share/classes/com/sun/org/apache/xml/internal/utils/SystemIDResolver.java

-

Commit messages:
 - JDK-8280373

Changes: https://git.openjdk.java.net/jdk/pull/7155/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7155=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8280373
  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7155.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7155/head:pull/7155

PR: https://git.openjdk.java.net/jdk/pull/7155


Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v2]

2022-01-20 Thread Michael McMahon
On Wed, 19 Jan 2022 22:25:43 GMT, Weijun Wang  wrote:

>> Michael McMahon has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   changes after first review round
>
> src/java.naming/share/classes/com/sun/jndi/ldap/sasl/LdapSasl.java line 133:
> 
>> 131: 
>> (String)env.get(TlsChannelBinding.CHANNEL_BINDING_TYPE));
>> 132: } catch (ChannelBindingException e) {
>> 133: throw new SaslException(e.getMessage());
> 
> How about setting `e` as cause of new exception? In `TlsChannelBinding.java` 
> the when the original exception was thrown (the 2nd throws) there was a cause.

Agreed.

> src/java.security.jgss/share/classes/module-info.java line 36:
> 
>> 34: module java.security.jgss {
>> 35: requires java.naming;
>> 36: requires java.security.sasl;
> 
> Can this be removed now?

Yes, well spotted!

-

PR: https://git.openjdk.java.net/jdk/pull/7065


Re: RFR: 8279842: HTTPS Channel Binding support for Java GSS/Kerberos [v3]

2022-01-20 Thread Michael McMahon
> Hi,
> 
> This change adds Channel Binding Token (CBT) support to HTTPS 
> (java.net.HttpsURLConnection) when used with the Negotiate (SPNEGO, Kerberos) 
> authentication scheme. When enabled, the implementation preemptively includes 
> a CBT with authentication requests over Kerberos. The feature is enabled as 
> follows:
> 
> A system property "jdk.spnego.cbt" is defined which can have the values 
> "never" (default), which means the feature is disabled, "always", which means 
> the CBT is included for all https Negotiate authentications, or it can take 
> the form "domain:a,b.c,*.d.com" which is a comma separated list of 
> domains/hosts where the feature is enabled, and disabled everywhere else. In 
> the given example, the CBT would be included in authentication requests for 
> hosts "a", "b.c" and all hosts under the domain "d.com" and all of its 
> sub-domains.
> 
> A test will be added separately to the implementation.
> 
> Bug report: https://bugs.openjdk.java.net/browse/JDK-8279842
> 
> Thanks,
> Michael

Michael McMahon has updated the pull request incrementally with one additional 
commit since the last revision:

  removed sasl module dependency and added SaslException cause

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7065/files
  - new: https://git.openjdk.java.net/jdk/pull/7065/files/f2ee58ec..fd56b5e3

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7065=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7065=01-02

  Stats: 2 lines in 2 files changed: 0 ins; 1 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7065.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7065/head:pull/7065

PR: https://git.openjdk.java.net/jdk/pull/7065


Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger [v7]

2022-01-20 Thread kabutz
On Sat, 15 Jan 2022 18:03:33 GMT, Paul Sandoz  wrote:

>> kabutz has updated the pull request incrementally with one additional commit 
>> since the last revision:
>> 
>>   Changed depth type to byte to save 8 bytes on each RecursiveSquare instance
>
> test/jdk/java/math/BigInteger/BigIntegerParallelMultiplyTest.java line 64:
> 
>> 62: BigInteger fib = fibonacci(n, BigInteger::multiply);
>> 63: System.out.printf("fibonacci(%d) = %d%n", n, fib);
>> 64: }
> 
> I think we can remove this and the loop block at #70-80, since we have the 
> performance test. After that we are good.

Done.

-

PR: https://git.openjdk.java.net/jdk/pull/6409


Re: RFR: JDK-8277175 : Add a parallel multiply method to BigInteger [v8]

2022-01-20 Thread kabutz
> BigInteger currently uses three different algorithms for multiply. The simple 
> quadratic algorithm, then the slightly better Karatsuba if we exceed a bit 
> count and then Toom Cook 3 once we go into the several thousands of bits. 
> Since Toom Cook 3 is a recursive algorithm, it is trivial to parallelize it. 
> I have demonstrated this several times in conference talks. In order to be 
> consistent with other classes such as Arrays and Collection, I have added a 
> parallelMultiply() method. Internally we have added a parameter to the 
> private multiply method to indicate whether the calculation should be done in 
> parallel.
> 
> The performance improvements are as should be expected. Fibonacci of 100 
> million (using a single-threaded Dijkstra's sum of squares version) completes 
> in 9.2 seconds with the parallelMultiply() vs 25.3 seconds with the 
> sequential multiply() method. This is on my 1-8-2 laptop. The final 
> multiplications are with very large numbers, which then benefit from the 
> parallelization of Toom-Cook 3. Fibonacci 100 million is a 347084 bit number.
> 
> We have also parallelized the private square() method. Internally, the 
> square() method defaults to be sequential.
> 
> Some benchmark results, run on my 1-6-2 server:
> 
> 
> Benchmark  (n)  Mode  Cnt  Score  
> Error  Units
> BigIntegerParallelMultiply.multiply100ss4 51.707 
> ±   11.194  ms/op
> BigIntegerParallelMultiply.multiply   1000ss4988.302 
> ±  235.977  ms/op
> BigIntegerParallelMultiply.multiply  1ss4  24662.063 
> ± 1123.329  ms/op
> BigIntegerParallelMultiply.parallelMultiply100ss4 49.337 
> ±   26.611  ms/op
> BigIntegerParallelMultiply.parallelMultiply   1000ss4527.560 
> ±  268.903  ms/op
> BigIntegerParallelMultiply.parallelMultiply  1ss4   9076.551 
> ± 1899.444  ms/op
> 
> 
> We can see that for larger calculations (fib 100m), the execution is 2.7x 
> faster in parallel. For medium size (fib 10m) it is 1.873x faster. And for 
> small (fib 1m) it is roughly the same. Considering that the fibonacci 
> algorithm that we used was in itself sequential, and that the last 3 
> calculations would dominate, 2.7x faster should probably be considered quite 
> good on a 1-6-2 machine.

kabutz has updated the pull request incrementally with one additional commit 
since the last revision:

  Removed unnecessary output from the test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6409/files
  - new: https://git.openjdk.java.net/jdk/pull/6409/files/94c2d665..25e8c082

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6409=07
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6409=06-07

  Stats: 16 lines in 1 file changed: 0 ins; 16 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6409.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6409/head:pull/6409

PR: https://git.openjdk.java.net/jdk/pull/6409