jdk11u build failure on Windows

[Vyom Tiwari]

Hi,
I am facing the build issue with  OpenJDK11(jdk11u). I am trying to build
jdk11u on Windows and I am  getting the below error.


./src/java.base/windows/native/libnet/net_util_md.c(792): error C2065:
'TCP_INITIAL_RTO_NO_SYN_RETRANSMISSIONS': undeclared identifier
make[3]: *** [Lib-java.base.gmk:44:
/cygdrive/d/source/mirrors_github_jdk11u/build/windows-x86_64-normal-server-release/support/native/java.base/libnet/net_util_md.obj]
Error 1
make[3]: *** Waiting for unfinished jobs
make[2]: *** [make/Main.gmk:215: java.base-libs] Error 2

ERROR: Build failed for target 'images' in configuration
'windows-x86_64-normal-server-release' (exit code 2)
Stopping sjavac server
###

I found that the back-port of  JDK-8250521
  is causing this failure.

My  environment details are as follows.
OS: Windows Server 2016 Standard, Visual Studio professional 2017 version
15.2(26430.14)

When I updated my Visual Studio to 15.9.42(which contains Windows 10
SDK(10.0.17134.0)) the problem went away.

My question is do we have a minimum supported Visual Studio(15.9.42) to
build jdk11u(11.0.14) ?. If this is not the case then I would like to fix
this build failure.

Thanks,
Vyom

Re: RFR: 8253155: Minor cleanups and Javadoc fixes for LdapDnsProvider of java.naming

[Vyom Tiwari]

Hi Christoph,
Changes look ok to me.

On Tue, Sep 15, 2020 at 1:26 PM Christoph Langer 
wrote:

> There are some little flaws in LdapDNSProvider and auxilliary classes,
> mostly in Javadoc.
>
> In detail:
> src/java.naming/share/classes/com/sun/jndi/ldap/DefaultLdapDnsProvider.java:
> Unnecessary import
> src/java.naming/share/classes/com/sun/jndi/ldap/LdapDnsProviderService.java:
> typo
> src/java.naming/share/classes/javax/naming/ldap/spi/LdapDnsProvider.java:
> Whitespace
> src/java.naming/share/classes/javax/naming/ldap/spi/LdapDnsProviderResult.java:
> Spelling of "ldap" -> should be
> capitalized
>
> -
>
> Commit messages:
>  - JDK-8253155
>
> Changes: https://git.openjdk.java.net/jdk/pull/168/files
>  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=168=00
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8253155
>   Stats: 21 lines in 4 files changed: 3 ins; 7 del; 11 mod
>   Patch: https://git.openjdk.java.net/jdk/pull/168.diff
>   Fetch: git fetch https://git.openjdk.java.net/jdk pull/168/head:pull/168
>
> PR: https://git.openjdk.java.net/jdk/pull/168
>


-- 
Thanks,
Vyom

Re: RFR 8252248: __SIGRTMAX is not declared in musl libc

[Vyom Tiwari]

+1
Vyom

On Fri, Aug 28, 2020 at 8:24 PM Alexander Scherbatiy <
alexander.scherba...@bell-sw.com> wrote:

> On 28.08.2020 17:40, Alan Bateman wrote:
>
> > On 28/08/2020 15:32, Alexander Scherbatiy wrote:
> >>
> >>   I run the java/nio/channels tests with the fix. There are five
> >> failed tests that fail with and without the fix:
> >> java/nio/channels/DatagramChannel/AdaptorMulticasting.java
> >> java/nio/channels/DatagramChannel/MulticastSendReceiveTests.java
> >> java/nio/channels/DatagramChannel/PromiscuousIPv6.java
> >> java/nio/channels/DatagramChannel/Loopback.java
> >> java/nio/channels/FileChannel/FileExtensionAndMap.java
> >>
> >> The FileExtensionAndMap.java has clear fail message: "Error. Test
> >> ignored: This test has huge disk space requirements".
> > The DatagramChannel failures are probably because of firewall or
> > iptables issues too.  The FileExtensionAndMap test has @ignore so it
> > will be skipped, maybe you are running jtreg directly and aren't use
> > -ignore:quiet?
>Yes, I run the jtreg tests directly without the -ignore:quiet option.
>
>Thanks,
>Alexander.
> >
> > In any case, I think the changes look okay.
> >
> > -Alan
>


-- 
Thanks,
Vyom

Re: RFR[testbug]: 8251189: com/sun/jndi/ldap/LdapDnsProviderTest.java failed due to timeout

[Vyom Tiwari]

+1
Vyom

On Tue, Aug 11, 2020 at 4:39 PM Daniel Fuchs 
wrote:

> Hi Aleksei,
>
> Looks good to me!
>
> best regards,
>
> -- daniel
>
> On 06/08/2020 13:44, Aleks Efimov wrote:
> > Hi,
> >
> > LdapDnsProviderTest was seen failing due to the timeout caused by the
> > blocked bind operation. That could happen if there is a local process
> > listening on the port specified in the test URL.
> > To make LdapProviderTest.ProviderTest.call fail the LDAP connect timeout
> > property needs to be set to greater than 0 value. That will help
> > runLocalHostTestWithRandomPort to fail with the timeout exception if the
> > port is busy and try another random port.
> > Also, the test output has been modified not to print stack traces for
> > the expected exceptions.
> >
> > Webrev: http://cr.openjdk.java.net/~aefimov/8251189/00/index.html
> > JBS: https://bugs.openjdk.java.net/browse/JDK-8251189
> >
> > The test was executed 100+ times alongside to other LDAP tests and was
> > not seen failing with the
> >   proposed changes.
> >
> > With Best Regards,
> > Aleksei
> >
>
>

-- 
Thanks,
Vyom

Re: RFR (JDK 15) 8251276: two jdeps files have extra whitespace in copyright header

[Vyom Tiwari]

LGTM
Vyom

On Fri, Aug 7, 2020 at 10:31 AM 
wrote:

> Please review.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8251276
>
> Webrev: http://cr.openjdk.java.net/~sundar/8251276/webrev.00/index.html
>
> Thanks,
>
> -Sundar
>
>

-- 
Thanks,
Vyom

Re: RFR(S) 8242504: Enhance the system clock to nanosecond precision

[Vyom Tiwari]

Hi David,

we can remove the redundant local variable(jlong result) from if block as
follows.

return jlong(ts.tv_sec) * MILLIUNITS + jlong(ts.tv_nsec) /
NANOUNITS_PER_MILLIUNIT;

Vyom


On Tue, May 26, 2020 at 10:29 AM David Holmes 
wrote:

> bug: https://bugs.openjdk.java.net/browse/JDK-8242504
> webrev: http://cr.openjdk.java.net/~dholmes/8242504/webrev/
>
> This work was contributed by Mark Kralj-Taylor:
>
>
> https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-April/038975.html
>
> On the hotspot side we change the Linux implementations of
> javaTimeMillis() and javaTimeSystemUTC() so that they use
> clock_gettime(CLOCK_REALTIME) instead of gettimeofday(). In keeping with
> our conditional use of this API I added a new guard
>
> os::Posix::supports_clock_gettime()
>
> and refined the existing supports_monotonic_clock() so that we can still
> use CLOCK_REALTIME if CLOCK_MONOTONIC does not exist. In the future
> (hopefully very near future) we will simply assume these APIs always exist.
>
> On the core-libs side the existing test:
>
> test/jdk/java/time/test/java/time/TestClock_System.java
>
> is adjusted to track the precision in more detail.
>
> Finally Mark has added instantNowAsEpochNanos() to the benchmark
> previously known as
>
> test/micro/org/openjdk/bench/java/lang/Systems.java
>
> which I've renamed to SystemTime.java as Mark suggested. I agree having
> these three functions measured together makes sense.
>
> Testing:
>- test/jdk/java/time/test/java/time/TestClock_System.java
>- test/micro/org/openjdk/bench/java/lang/SystemTime.java
>- Tiers 1-3
>
> Thanks,
> David
>


-- 
Thanks,
Vyom

Re: RFR [LDAP]: 8062947: Fix exception message to correctly represent LDAP connection failure

[Vyom Tiwari]

LGTM as well.
Vyom

On Thu, May 7, 2020 at 3:01 PM Chris Yin  wrote:

> +1
>
> Thanks,
> Chris
>
> > On 6 May 2020, at 6:35 PM, Daniel Fuchs  wrote:
> >
> > Hi Aleksei,
> >
> > Looks good to me.
> >
> > best regards,
> >
> > -- daniel
> >
> > On 05/05/2020 18:23, Aleks Efimov wrote:
> >> "LDAP response read timed out, timeout used:-1ms.":
> >> https://bugs.openjdk.java.net/browse/JDK-8062947
> >> The following fix tries to tackle this issue:
> >> http://cr.openjdk.java.net/~aefimov/8062947/00
> >
>
>

-- 
Thanks,
Vyom

Re: [15] RFR: 8244152: Remove unnecessary hash map resize in LocaleProviderAdapters

[Vyom Tiwari]

LGTM
Vyom

On Thu, Apr 30, 2020 at 3:50 AM  wrote:

> Hello,
>
> Please review this small fix to the following issue:
>
> https://bugs.openjdk.java.net/browse/JDK-8244152
>
> The proposed changeset is located at:
>
> https://cr.openjdk.java.net/~naoto/8244152/webrev.00/
>
> The hash map used there didn't have initial capacity, even though the
> exact numbers are known.
>
> Naoto
>


-- 
Thanks,
Vyom

Re: RFR 15 8243099: Adding ADQ support to JDK

[Vyom Tiwari]

Hi Vladimir,

In LinuxSocketOptions.c we have lot's of duplicate code, can you please
reuse "socketOptionSupported" & "handleError" as follows, this we remove
lot's of duplicate code.
Thanks,
Vyom

diff -r b6b4506a7827 src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c
--- a/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c Fri Apr 24
15:28:57 2020 +0800
+++ b/src/jdk.net/linux/native/libextnet/LinuxSocketOptions.c Fri Apr 24
13:37:36 2020 +0530
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights
reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,10 @@
 #include "jni_util.h"
 #include "jdk_net_LinuxSocketOptions.h"

+#ifndef SO_INCOMING_NAPI_ID
+#define SO_INCOMING_NAPI_ID56
+#endif
+
 /*
  * Class: jdk_net_LinuxSocketOptions
  * Method:setQuickAck
@@ -44,15 +48,7 @@
 int rv;
 optval = (on ? 1 : 0);
 rv = setsockopt(fd, SOL_SOCKET, TCP_QUICKACK, , sizeof
(optval));
-if (rv < 0) {
-if (errno == ENOPROTOOPT) {
-JNU_ThrowByName(env, "java/lang/UnsupportedOperationException",
-"unsupported socket option");
-} else {
-JNU_ThrowByNameWithLastError(env, "java/net/SocketException",
-"set option TCP_QUICKACK failed");
-}
-}
+handleError(env, rv, "set option TCP_QUICKACK failed");
 }

 /*
@@ -65,15 +61,7 @@
 int on;
 socklen_t sz = sizeof (on);
 int rv = getsockopt(fd, SOL_SOCKET, TCP_QUICKACK, , );
-if (rv < 0) {
-if (errno == ENOPROTOOPT) {
-JNU_ThrowByName(env, "java/lang/UnsupportedOperationException",
-"unsupported socket option");
-} else {
-JNU_ThrowByNameWithLastError(env, "java/net/SocketException",
-"get option TCP_QUICKACK failed");
-}
-}
+handleError(env, rv, "get option TCP_QUICKACK failed");
 return on != 0;
 }

@@ -83,31 +71,18 @@
  * Signature: ()Z
  */
 JNIEXPORT jboolean JNICALL
Java_jdk_net_LinuxSocketOptions_quickAckSupported0
-(JNIEnv *env, jobject unused) {
-int one = 1;
-int rv, s;
-s = socket(PF_INET, SOCK_STREAM, 0);
-if (s < 0) {
-return JNI_FALSE;
-}
-rv = setsockopt(s, SOL_SOCKET, TCP_QUICKACK, (void *) , sizeof
(one));
-if (rv != 0 && errno == ENOPROTOOPT) {
-rv = JNI_FALSE;
-} else {
-rv = JNI_TRUE;
-}
-close(s);
-return rv;
+(JNIEnv *env, jobject unused) {
+return socketOptionSupported(SOL_SOCKET, TCP_QUICKACK);
 }

-static jint socketOptionSupported(jint sockopt) {
+static jint socketOptionSupported(jint level, jint optname) {
 jint one = 1;
 jint rv, s;
 s = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP);
 if (s < 0) {
 return 0;
 }
-rv = setsockopt(s, SOL_TCP, sockopt, (void *) , sizeof (one));
+rv = setsockopt(s, level, optname, (void *) , sizeof (one));
 if (rv != 0 && errno == ENOPROTOOPT) {
 rv = 0;
 } else {
@@ -135,8 +110,8 @@
  */
 JNIEXPORT jboolean JNICALL
Java_jdk_net_LinuxSocketOptions_keepAliveOptionsSupported0
 (JNIEnv *env, jobject unused) {
-return socketOptionSupported(TCP_KEEPIDLE) &&
socketOptionSupported(TCP_KEEPCNT)
-&& socketOptionSupported(TCP_KEEPINTVL);
+return socketOptionSupported(SOL_TCP, TCP_KEEPIDLE) &&
socketOptionSupported(SOL_TCP, TCP_KEEPCNT)
+&& socketOptionSupported(SOL_TCP, TCP_KEEPINTVL);
 }

 /*
@@ -213,3 +188,27 @@
 handleError(env, rv, "get option TCP_KEEPINTVL failed");
 return optval;
 }
+
+/*
+ * Class: jdk_net_LinuxSocketOptions
+ * Method:IncomingNapiIdSupported0
+ * Signature: (I)I;
+ */
+JNIEXPORT jboolean JNICALL
Java_jdk_net_LinuxSocketOptions_IncomingNapiIdSupported0
+(JNIEnv *env, jobject unused) {
+return socketOptionSupported(SOL_SOCKET, SO_INCOMING_NAPI_ID);
+}
+
+/*
+ * Class: jdk_net_LinuxSocketOptions
+ * Method:getIncomingNapiId0
+ * Signature: (I)I;
+ */
+JNIEXPORT jint JNICALL Java_jdk_net_LinuxSocketOptions_getIncomingNapiId0
+(JNIEnv *env, jobject unused, jint fd) {
+jint optval, rv;
+socklen_t sz = sizeof (optval);
+rv = getsockopt(fd, SOL_SOCKET, SO_INCOMING_NAPI_ID, , );
+handleError(env, rv, "get option SO_INCOMING_NAPI_ID failed");
+return optval;
+}

On Fri, Apr 24, 2020 at 12:43 AM Ivanov, Vladimir A <
vladimir.a.iva...@intel.com> wrote:

> Thanks a lot to Chris and Alan for detailed comments.
> The updated version of patch available at
> http://cr.openjdk.java.net/~sviswanathan/Vladimir/8243099/webrev.01/
>
> Changes:
> 1. in native code the common pattern was used for the 'getsockopt' call.
> 2. condition to define SO_INCOMING_NAPI_ID was added.
> 3. the DatagarmSocket was added to the 

Re: [15] RFR: 8243138: Enhance BaseLdapServer to support starttls extended request

[Vyom Tiwari]

Hi Chris,
change looks good to me.
Vyom

On Wed, Apr 22, 2020 at 12:59 PM Chris Yin  wrote:

> Hello
>
> Please review following change for enhancement to
> com/sun/jndi/ldap/lib/BaseLdapServer.java, thanks
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8243138
> Webrev: http://cr.openjdk.java.net/~xyin/8243138/webrev.00/
>
> There is requirement to test starttls extended op against dummy ldap
> server, but unfortunately, current BaseLdapSever logic cannot handle it
> correctly since the starttls request is quite special to use existed socket
> for tls negotiate. This enhancement will provide the possibility to wrap
> current socket connection and overwrite in-place processing input/output
> stream, certainly, that will not affect existed tests which depends on
> BaseLdapServer. Run existed dependency tests on 4 platforms for total 200
> times, all green.
>
> Thanks,
> Chris



-- 
Thanks,
Vyom

Re: [15] RFR: 8242614: cleanup duplicated test ldap server in some com/sun/jndi/ldap/ tests

[Vyom Tiwari]

Hi Chris,

Changes looks good to me.

Thanks,
Vyom

On Tue, Apr 21, 2020 at 1:14 PM Chris Yin  wrote:

> Hello
>
> Please review following changes to cleanup duplicated test ldap server in
> some com/sun/jndi/ldap/ tests, thanks
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8242614
> Webrev: http://cr.openjdk.java.net/~xyin/8242614/webrev.00/
>
> This is the first part to cleanup straightforward duplicated test ldap
> server which was embedded in test itself. In the past, ldap tests may copy
> or implemented a dummy server in test itself to support specific test
> logic, so we have many duplicated dummy server logic across tests. Now
> there is already a common test ldap server skeleton available, it’s time to
> cleanup those duplicated logic to reduce possible future maintenance works.
> Run modified tests on 4 platforms for total 200 times, all green.
>
> Thanks,
> Chris



-- 
Thanks,
Vyom

Re: RFR(S): 8242848: Improve performance of InflaterOutputStream.write()

[Vyom Tiwari]

Hi Volker,

Latest changes looks good to me. I notices the copyright in new test
Streams.java is "

Copyright (c) 2020, Amazon and/or its affiliates." instead is regular
Oracle copyright.

Thanks,
Vyom

On Fri, Apr 17, 2020 at 4:23 PM Volker Simonis 
wrote:

> Thanks everybody for looking at this change!
>
> Please find an updated webrev (with a new test and micro-benchmark)
> and my answers to your comments below:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2020/8242848.01/
>
> On Thu, Apr 16, 2020 at 6:40 AM Vyom Tiwari  wrote:
> > Thanks for doing this, i think the below code change is not required .
> >
> > Please do let me know if i am not reading it correctly.
> >
> > if (inf.finished() || (len == 0)/* no more input */) {
> >
> > If you check the native code "inf.finished() will be true only of the
> corresponding native call inflate(strm, Z_PARTIAL_FLUSH) returns
> "Z_STREAM_END".
> >
> > After your code change  write may return even if finished() is not true.
>
> Yes, that's true, but that's what we must do if there's no more input
> available. Before my change this break on "len < 1" was in the "if
> (inf.needsInput())" branch.
>
> On Thu, Apr 16, 2020 at 8:22 AM Thomas Stüfe 
> wrote:
> >  252 // Check the decompressor
> >  253 if (inf.finished() || (len == 0)/* no more input
> */) {
> >  254 break;
> >  255 }
> >
> > Not sure but I think this is wrong because now you bypass the followup
> handling of inf.needsDirectory.
> >
> > Inflater.inflate() returns 0 if either needsInput or needsDirectory. We
> have to ignore needsInput since we have no input anymore, but
> needsDirectory has to be handled, no?
>
> You're absolutely right Thomas. Thanks for catching this! I've moved
> the check for "needsDictionary" in front of the "finished() || len ==
> 0" check.
>
> Unfortunately there is not very good test coverage for zip with preset
> dictionaries (jtreg and submit repo passed without problems). So I
> added a new test for this use case to "
> test/jdk/java/util/zip/DeflateIn_InflateOut.java".
>
> On Thu, Apr 16, 2020 at 8:48 AM Thomas Stüfe 
> wrote:
> >
> > As for increasing the buffer size, it makes sense but IMHO needs a CSR
> and a release note.
>
> I don't think so. This is an internal, implementation specific setting
> which has never been exposed or documented before so I don't see why
> we should document it now. But let's discuss this separately in the
> corresponding JBS issue (see below).
>
> On Thu, Apr 16, 2020 at 1:18 PM Claes Redestad
>  wrote:
> >
> > Hi Volker,
> >
> > On 2020-04-15 19:48, Volker Simonis wrote:
> > > While doing this change, I've also realized that all the streams in
> > > java.util.zip (i.e. DeflaterInputStream, GZIPInputStream,
> > > GZIPOutputStream, InflaterInputStream, DeflaterOutputStream) use an
> > > internal byte buffer of 512 bytes by default. Looking at the benchmark
> > > attached to JDK-8242864, I think that increasing this default to a
> > > bigger size (e.g. 4096 bytes) will considerably speed up (up to 50%)
> > > read and write operations on these streams when they are created with
> > > the default buffer size. I think the value "512" is a legacy of old
> > > times when memory was more precious:)  so  I've opened "JDK-8242864:
> > > Increase the default, internal buffer size of the Streams in
> > > java.util.zip" to track that as as separate issue. Do you think it
> > > makes sense to increase that default value?
> >
> > Seems reasonable. 8192 seems to be the buffer size we've been converging
> > on elsewhere (see InputStream, BufferedInputStream, Files, ..). I also
>
> That seems reasonable. Alan commented on the JBS issue so we can
> continue the discussion there.
>
> > found an instance of 8096, which is either a typo or a clever
> > optimization to keep the array + array object header fit snugly within
> > 8Kb. You chose. :-)
> >
>
> I like how you try to be positive :)
>
> > >
> > > Thank you and best regards,
> > > Volker
> > >
> > > PS: do you think it makes sense to contribute the benchmark attached
> > > to JDK-8242864 to the code-tools/mh-jdk-microbenchmarks [1] project?
> > >
> > > [1]
> http://openjdk.java.net/projects/code-tools/jmh-jdk-microbenchmarks/
> >
> > I'd definitely welcome the micro as part of the patch under
> > test/micro/org/openjdk

Re: RFR(S): 8242848: Improve performance of InflaterOutputStream.write()

[Vyom Tiwari]

Hi Volker,

Thanks for doing this, i think the below code change is not required .

Please do let me know if i am not reading it correctly.

if (inf.finished() || (len == 0)/* no more input */) {

If you check the native code "inf.finished() will be true only of the
corresponding native call inflate(strm, Z_PARTIAL_FLUSH) returns
"Z_STREAM_END".

After your code change  write may return even if finished() is not true.


Thanks,

Vyom


On Wed, Apr 15, 2020 at 11:19 PM Volker Simonis 
wrote:

> Hi,
>
> can I please have a review for the following simple performance
> enhancement:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2020/8242848/
> https://bugs.openjdk.java.net/browse/JDK-8242864
>
> InflaterOutputStream has an internal buffer which is used for the
> inflated data. This buffer can be configured to a custom size (the
> default is 512 bytes) by using the specialized
> "InflaterOutputStream(OutputStream out, Inflater infl, int bufLen)"
> constructor. A bigger buffer, results in fewer native calls to
> "write()" on the associated OutputStream and better overall
> performance.
>
> Unfortunately "InflaterOutputStream.write(byte[] b, int off, int len)"
> unnecessarily chunks its own byte input buffer "b" into pieces of
> maximum 512 bytes before feeding them to the underlying Inflater. This
> unnecessarily increases the number of native calls to
> Inflater.inflate() and limits the benefit of the configurable internal
> buffer to a size of ~(512 * compression-ratio) bytes.
>
> Instead, we could easily pass the complete input buffer "b" as input
> to the underlying Inflater. This simplifies the code and results in up
> to ~25% performance improvements for bigger internal buffers (see the
> JMH benchmark attached to the bug).
>
> Regarding the implementation, I initially wanted to completely remove
> the "for (;;)" loop after removing the chunking of the input buffer.
> But I think it is still necessary, because an InflaterOutputStream can
> be created with a custom Inflater which already has some input data.
> In that case, the Inflater will first consume its initial input data
> in the first "for" loop iteration while "write()"s input buffer "b"
> will only be consumed in the second "for" loop iteration.
>
> While doing this change, I've also realized that all the streams in
> java.util.zip (i.e. DeflaterInputStream, GZIPInputStream,
> GZIPOutputStream, InflaterInputStream, DeflaterOutputStream) use an
> internal byte buffer of 512 bytes by default. Looking at the benchmark
> attached to JDK-8242864, I think that increasing this default to a
> bigger size (e.g. 4096 bytes) will considerably speed up (up to 50%)
> read and write operations on these streams when they are created with
> the default buffer size. I think the value "512" is a legacy of old
> times when memory was more precious :) so  I've opened "JDK-8242864:
> Increase the default, internal buffer size of the Streams in
> java.util.zip" to track that as as separate issue. Do you think it
> makes sense to increase that default value?
>
> Thank you and best regards,
> Volker
>
> PS: do you think it makes sense to contribute the benchmark attached
> to JDK-8242864 to the code-tools/mh-jdk-microbenchmarks [1] project?
>
> [1] http://openjdk.java.net/projects/code-tools/jmh-jdk-microbenchmarks/
>


-- 
Thanks,
Vyom

Re: [15] RFR: 8214694: cleanup rawtypes warnings in open jndi tests

[Vyom Tiwari]

Hi Chris,

Latest changes look good to me. I can see that there are couple of unused
imports in files(DeadServerTimeoutSSLTest.java) but unused imports are
separate issue.

Thanks,
Vyom

On Fri, Mar 27, 2020 at 2:48 PM Chris Yin  wrote:

> Hi, Vyom
>
> On 27 Mar 2020, at 12:08 PM, Vyom Tiwari  wrote:
>
> Hi Chris,
>
> I have one question to you, is there is any specific reason for using
> wildcard(?) ?.
>
>
> Thank you for reviewing and comments. I just replaced most of the
> wildcard(?) with specified type as precise as they could be in latest
> webrev.01, the rest of them may fall into below scenarios.
>
> 1. API return value or parameter with wildcard(?), such as Hashtable
> in test/jdk/com/sun/jndi/dns/EnvTests/AddInherited.java
> 2. Cannot find the precise type from code, such as ScheduledFuture
> in test/jdk/com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java
>
> In your change we can avoid  "?" at most of the places. Please see the
> below methods signatures.
>
> ###
> public NamingEnumeration listBindings(Name name)  throws
> NamingException;
> public NamingEnumeration list(Name name)  throws
> NamingException;
> public NamingEnumerationsearch(Name name,  Attributes
> matchingAttributes,
>
>
> String[] attributesToReturn)  throws NamingException;
> #
>
>
> Thank you for the detailed signatures info, yes, now all fixed in the
> latest webrev http://cr.openjdk.java.net/~xyin/8214694/webrev.01/
>
> Regards,
> Chris
>
>
> thanks,
> Vyom
>
> On Wed, Mar 25, 2020 at 1:28 PM Chris Yin  wrote:
>
>> Hello
>>
>> Please review following simple changes to cleanup raw types warning for
>> open jndi tests (under test/jdk/com/sun/jndi and test/jdk/javax/naming),
>> thanks
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8214694
>> Webrev: http://cr.openjdk.java.net/~xyin/8214694/webrev.00/
>>
>>
>> The changes should be straightforward, only fix raw types warnings, no
>> test logic change, no code optimization or cleanup. Minor change to each
>> test file, just a little surprised about the affected tests count, hope
>> this covers all. Run related jndi tests on 4 platforms for total 200 times,
>> all passed.
>>
>> Thanks,
>> Chris
>
>
>
> --
> Thanks,
> Vyom
>
>
>

-- 
Thanks,
Vyom

Re: [15] RFR: 8214694: cleanup rawtypes warnings in open jndi tests

[Vyom Tiwari]

Hi Chris,

I have one question to you, is there is any specific reason for using
wildcard(?) ?.
In your change we can avoid  "?" at most of the places. Please see the
below methods signatures.

###
public NamingEnumeration listBindings(Name name)  throws
NamingException;
public NamingEnumeration list(Name name)  throws
NamingException;
public NamingEnumerationsearch(Name name,  Attributes
matchingAttributes,


String[] attributesToReturn)  throws NamingException;
#

thanks,
Vyom

On Wed, Mar 25, 2020 at 1:28 PM Chris Yin  wrote:

> Hello
>
> Please review following simple changes to cleanup raw types warning for
> open jndi tests (under test/jdk/com/sun/jndi and test/jdk/javax/naming),
> thanks
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8214694
> Webrev: http://cr.openjdk.java.net/~xyin/8214694/webrev.00/
>
>
> The changes should be straightforward, only fix raw types warnings, no
> test logic change, no code optimization or cleanup. Minor change to each
> test file, just a little surprised about the affected tests count, hope
> this covers all. Run related jndi tests on 4 platforms for total 200 times,
> all passed.
>
> Thanks,
> Chris



-- 
Thanks,
Vyom

Re: [15] RFR: 8202117: com/sun/jndi/ldap/RemoveNamingListenerTest.java fails intermittently: Connection reset

[Vyom Tiwari]

looks good to me.
Vyom

On Mon, Mar 16, 2020 at 12:59 PM Chris Yin  wrote:

> Thanks for reviewing, Daniel, Vyom.
>
>
> Hi, Daniel
>
> I modified the test as you suggested to cover the potential issue with
> URIBuilder, many thanks. Updated webrev as below:
>
> http://cr.openjdk.java.net/~xyin/8202117/webrev.01/
>
> Regards,
> Chris
>
>
> > On 13 Mar 2020, at 7:51 PM, Daniel Fuchs 
> wrote:
> >
> > Hi Chris,
> >
> > This looks fine to me too. Thanks for taking care of this issue.
> >
> > A potential issue I see is that the test might fail if
> > "localhost" does not resolve to the loopback address - but you
> > would likely get a "Connection refused" in that case.
> >
> > One possibility to get that out of the way could be to use
> > the URIBuilder:
> >
> > * @library lib/ /test/lib
> >
> >   ...
> >
> >   URI providerURI = URIBuilder.newBuilder()
> >.scheme("ldap")
> >.loopback()
> >.port(server.getLocalPort())
> >.path("/o=example")
> >.build();
> >   ...
> >
> >  59 env.put(Context.PROVIDER_URL, providerURI.toString());
> >
> > best regards,
> >
> > -- daniel
> >
> >
> > On 13/03/2020 08:28, Chris Yin wrote:
> >> Hello,
> >> Please review following changes to try to fix intermittent failure of
> test com/sun/jndi/ldap/RemoveNamingListenerTest.java, thanks
> >> Bug: https://bugs.openjdk.java.net/browse/JDK-8202117
> >> Webrev: http://cr.openjdk.java.net/~xyin/8202117/webrev.00/
> >> According to failure logs, test already run done and give a pass
> message, but test framework caught “java.lang.RuntimeException:
> java.net.SocketException: Connection reset” from other thread during test
> end, go through the test code, LDAPServerHandler thread may throw such
> exception in specific case. This change will replace test itself
> implemented TestLDAPServer/LDAPServerHandler with customized BaseLdapServer
> to fix the corner. I had run the changed test on 4 platforms for total 600
> times, no failure observed.
> >> Thanks,
> >> Chris
> >
>
>

-- 
Thanks,
Vyom

Re: [15] RFR: 8202117: com/sun/jndi/ldap/RemoveNamingListenerTest.java fails intermittently: Connection reset

[Vyom Tiwari]

Hi Chris,

Thanks for taking care of this issue, changes looks OK to me.

I am happy to see that we are removing the lot's of duplicate "DummyServer"
from individual test cases and using the "BaseLdapServer" test-server
instead.

Thanks,
Vyom

On Fri, Mar 13, 2020 at 1:59 PM Chris Yin  wrote:

> Hello,
>
> Please review following changes to try to fix intermittent failure of test
> com/sun/jndi/ldap/RemoveNamingListenerTest.java, thanks
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8202117
> Webrev: http://cr.openjdk.java.net/~xyin/8202117/webrev.00/
>
>
> According to failure logs, test already run done and give a pass message,
> but test framework caught “java.lang.RuntimeException:
> java.net.SocketException: Connection reset” from other thread during test
> end, go through the test code, LDAPServerHandler thread may throw such
> exception in specific case. This change will replace test itself
> implemented TestLDAPServer/LDAPServerHandler with customized BaseLdapServer
> to fix the corner. I had run the changed test on 4 platforms for total 600
> times, no failure observed.
>
> Thanks,
> Chris



-- 
Thanks,
Vyom

Re: RFR 8240524: Removed warnings from test classes

[Vyom Tiwari]

Hi Vipin,

Test code changes looks good to me as well except copyright  changes, which
will be fix at the time of pushing the code by Christoph.

Thanks,
Vyom

On Fri, Mar 6, 2020 at 12:06 PM Langer, Christoph 
wrote:

> Hi Vipin,
>
> this all looks correct to me.
>
> When changing the copyright headers, you have to keep the first (initial)
> year and only update the second year. If this doesn’t exist, you’ll have to
> add it, e.g.
> Copyright (c) 1999, Oracle -> Copyright (c) 1999, 2020, Oracle
>
> I can fix that for you though, when sponsoring, no need for new webrev.
>
> Can I have a second review, please?
>
> Thanks
> Christoph
>
>
>
> From: Vipin Sharma 
> Sent: Donnerstag, 5. März 2020 18:27
> To: Langer, Christoph ;
> core-libs-dev@openjdk.java.net
> Subject: RFR 8240524: Removed warnings from test classes
>
> Hi All,
>
> Please review patch to remove warnings from test classes.
>
> Bug : https://bugs.openjdk.java.net/browse/JDK-8240524
> Webrev : http://cr.openjdk.java.net/~clanger/webrevs/8240524.0/
>
>
> Change description:
>
> Class: test/jdk/java/lang/Boolean/GetBoolean.java
> Fixed following warning:
> 1. "Exception 'java.lang.Exception' is never thrown in the method”
>
> Class: test/jdk/java/lang/Boolean/MakeBooleanComparable.java
> Fixed following warnings:
> 1. Explicit type argument Boolean can be replaced with <>
> 2. C-style array declaration of parameter 'args'
>
> Class: test/jdk/java/lang/Boolean/ParseBoolean.java
> Fixed following warning:
> 1. Exception 'java.lang.Exception' is never thrown in the method
>
>
> Regards,
> Vipin
>


-- 
Thanks,
Vyom

RFR 8238579: HttpsURLConnection drops the timeout and hangs forever in read

[Vyom Tiwari]

Hi All,

Please find the below fix  which resolves the issue(
https://bugs.openjdk.java.net/browse/JDK-8238579).

"HttpURLConnection.writeRequests()" retry in case of any write failure,
during retry it creates new HttpsClient  without connectTimeout &
readTimeout. Below fix sets the connect & read timeout.

Thanks,
Vyom

diff -r 7e6165c9c606
src/java.base/share/classes/sun/net/www/protocol/https/AbstractDelegateHttpsURLConnection.java
---
a/src/java.base/share/classes/sun/net/www/protocol/https/AbstractDelegateHttpsURLConnection.java
Thu Feb 13 17:14:45 2020 -0800
+++
b/src/java.base/share/classes/sun/net/www/protocol/https/AbstractDelegateHttpsURLConnection.java
Fri Feb 14 10:11:06 2020 +0530
@@ -87,10 +87,15 @@
  */
 public void setNewClient (URL url, boolean useCache)
 throws IOException {
+int readTimeout = getReadTimeout();
 http = HttpsClient.New (getSSLSocketFactory(),
 url,
 getHostnameVerifier(),
-useCache, this);
+ null,
+ -1,
+useCache,
+ getConnectTimeout(), this);
+ http.setReadTimeout(readTimeout);
 ((HttpsClient)http).afterConnect();
 }

@@ -132,10 +137,14 @@
 boolean useCache) throws IOException {
 if (connected)
 return;
-http = HttpsClient.New (getSSLSocketFactory(),
-url,
-getHostnameVerifier(),
-proxyHost, proxyPort, useCache, this);
+int readTimeout = getReadTimeout();
+http = HttpsClient.New(getSSLSocketFactory(),
+url,
+getHostnameVerifier(),
+proxyHost, proxyPort,
+useCache,
+getConnectTimeout(), this);
+http.setReadTimeout(readTimeout);
 connected = true;
 }

Re: [teststabilization] RFR: 8141685: com/sun/jndi/ldap/InvalidLdapFilters.java initializes context failed

[Vyom Tiwari]

Hi Daniel,
Ok, thanks for the update.
Vyom


On Sun, Dec 8, 2019 at 4:19 PM Daniel Fuchs  wrote:

> Hi Vyom,
>
> On 07/12/2019 03:36, Vyom Tiwari wrote:
> > Changes looks OK to me, one minor bit you  need to update @bug  tag
> > before pushing.
>
> We don't update @bug for test bugs (when the fix is in the
> the test itself).
>
> best regards,
>
> -- daniel
>


-- 
Thanks,
Vyom

Re: [teststabilization] RFR: 8141685: com/sun/jndi/ldap/InvalidLdapFilters.java initializes context failed

[Vyom Tiwari]

Hi Aleks,

Changes looks OK to me, one minor bit you  need to update @bug  tag before
pushing.

Thanks,
Vyom

On Fri, Dec 6, 2019 at 11:41 PM Daniel Fuchs 
wrote:

> Looks good to me Aleksei!
>
> best regards,
>
> -- daniel
>
> On 06/12/2019 17:33, Aleks Efimov wrote:
> > Hi,
> >
> > InvalidLdapFilters test has been seen failing intermittently with
> > timeouts. The cause of the observed failures could be the binding of
> > test server to the wildcard address.
> > Proposed fix binds the test server to the loopback address and uses the
> > test library to construct LDAP provider URL.
> > The modified test has been executed ~100 times alongside to other
> > networking and JNDI tests: No failures have been observed.
> >
> > Webrev: http://cr.openjdk.java.net/~aefimov/8141685/00
> > JBS: https://bugs.openjdk.java.net/browse/JDK-8141685
> >
> > With Best Regards,
> > Aleksei
> >
>
>

-- 
Thanks,
Vyom

Re: RFR: 8234964: failure_handler: gather more environment information on Windows, Solaris and Linux

[Vyom Tiwari]

Hi Julia,

changes looks good to me.
Thanks,
Vyom

On Mon, Dec 2, 2019 at 4:24 PM Julia Boes  wrote:

> Hi,
>
>
> > to make it more consistent w/ other tools, I'd move all ifconfig (incl.
> one on macOS) to 'net' category, i.e. rename them to net.ifconfig, this
> will require also moving all netstat.* on macOS and solaris to 'net' as
> well. I don't insist on it, though.
>
> Good point, I made those changes.
>
>
> > Changes looks OK to me although i have one question, in case of
> > Solaris you use explicit path to ifconfig(ifconfig.app=/sbin/ifconfig).
> > Does Solaris by default doesn't set the /sbin folder to user's 'PATH'
> > environment variable ?
>
> That's right, /sbin is not on the PATH on the Solaris machines of the
> test farm, which can be confirmed under the 'env' section of the
> environment information output.
>
>
> Updated webrev:
> http://cr.openjdk.java.net/~jboes/webrevs/8234964/webrev.01/
>
>
> Regards,
>
> Julia
>
>
>

-- 
Thanks,
Vyom

Re: RFR: 8234964: failure_handler: gather more environment information on Windows, Solaris and Linux

[Vyom Tiwari]

Hi Julia,

Changes looks OK to me although i have one question, in case of Solaris you
use explicit path to ifconfig(ifconfig.app=/sbin/ifconfig).

Does Solaris by default doesn't set the /sbin folder to user's 'PATH'
environment variable ?.

Thanks,
Vyom

On Fri, Nov 29, 2019 at 12:18 AM Igor Ignatyev 
wrote:

> Hi Julia,
>
> looks good to me.
>
> to make it more consistent w/ other tools, I'd move all ifconfig (incl.
> one on macOS) to 'net' category, i.e. rename them to net.ifconfig, this
> will require also moving all netstat.* on macOS and solaris to 'net' as
> well. I don't insist on it, though.
>
> -- Igor
>
> > On Nov 28, 2019, at 10:15 AM, Julia Boes  wrote:
> >
> > Hi,
> >
> > In the case of test failure, the environment information of 'ifconfig
> -a' is already gathered on macOS systems.
> >
> > The following enhancement allows the same information to be gathered on
> Linux, Solaris and Windows systems (in the latter case 'ipconfig /all').
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8234964
> >
> > Webrev: http://cr.openjdk.java.net/~jboes/webrevs/8234964/webrev.00/
> >
> > Regards,
> >
> > Julia
> >
>
>

-- 
Thanks,
Vyom

Re: RFR [14/java.xml] 8233686: XML transformer uses excessive amount of memory

[Vyom Tiwari]

Hi Joe,

Fix looks OK to me , but i am not able to understand how come
"NamespaceMappings"  instance can increase memory uses from (20MB to 140MB
).

Current scope of "ns"  is  "case Node.ELEMENT_NODE:"  block and
"NamespaceMapping"  seems to be very lightweight class.

Thanks,
Vyom

On Fri, Nov 8, 2019 at 12:33 AM Joe Wang  wrote:

> Please review a quick fix that reduces unnecessary object allocations.
>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8233686
> webrev: http://cr.openjdk.java.net/~joehw/jdk14/8233686/webrev/
>
> Thanks,
> Joe
>
>

-- 
Thanks,
Vyom

Re: 8229022: BufferedReader performance can be improved by using StringBuilder

[Vyom Tiwari]

Hi Brian,
Looks good to me.
thanks,
Vyom

On Wed, Oct 2, 2019 at 5:44 AM Brian Burkhalter 
wrote:

> While the performance improvement that I measured for this proposed change
> [1] using JMH was only in the 2% to 8% range as opposed to the 13% claimed
> in the issue description, given the simplicity of the change [2] it is
> probably worth doing. All use of the StringBuilder is within a synchronized
> block within a single package-scope method, so there should be no problem
> with access by multiple threads.
>
> Thanks,
>
> Brian
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8229022
> [2] diff
>
> --- a/src/java.base/share/classes/java/io/BufferedReader.java
> +++ b/src/java.base/share/classes/java/io/BufferedReader.java
> @@ -314,7 +314,7 @@
>   * @throws IOException  If an I/O error occurs
>   */
>  String readLine(boolean ignoreLF, boolean[] term) throws IOException {
> -StringBuffer s = null;
> +StringBuilder s = null;
>  int startChar;
>
>  synchronized (lock) {
> @@ -372,7 +372,7 @@
>  }
>
>  if (s == null)
> -s = new StringBuffer(defaultExpectedLineLength);
> +s = new StringBuilder(defaultExpectedLineLength);
>  s.append(cb, startChar, i - startChar);
>  }
>  }
>
>

-- 
Thanks,
Vyom

Re: RFR (14) [testbug]: runs zero test, 8230002 and 8230010

[Vyom Tiwari]

Hi Frank,

Changes looks good to me.

Thanks,
Vyom

On Tue, Aug 27, 2019 at 2:37 PM Frank Yuan  wrote:

> Hi all
>
>
>
> We found 2 jaxp tests, which didn't run indeed.
>
>
>
> Bugs:
>
> https://bugs.openjdk.java.net/browse/JDK-8230002
>
> Annotation @Test was missed in TestNG test.
>
>
>
> https://bugs.openjdk.java.net/browse/JDK-8230010
>
> The old test was left during it was converted to TestNG test.
>
>
>
> Webrev:
>
> http://cr.openjdk.java.net/~fyuan/8230002_8230010/webrev.00/
>
>
>
> Thanks
>
> Frank
>
>

-- 
Thanks,
Vyom

Re: [PATCH] JDK-8228580 DnsClient TCP socket timeout

[Vyom Tiwari]

Hi Milan,

Your test need the corresponding "TcpTimeout.dns" file to run successfully,
I believe you forgot to add with your patch.
please check the existing tests in the same folder if you need any
additional information.

Thanks,
Vyom

On Wed, Aug 21, 2019 at 10:48 PM Milan Mimica 
wrote:

> Hi Pavel
>
> Updated the patch with the jtreg test.
> The test hangs when the fix is not applied. I don't know why
> main/timeout=20 does not work for me.
>
> On Tue, 20 Aug 2019 at 00:08, Pavel Rappo  wrote:
> >
> > Thanks for doing that. I've only skimmed through the patch and I’d
> recommend that no matter if the changes are adequate or not there should be
> a test [1]:
> >
> > "...A unit test, written for the jtreg test harness, that validates your
> change. The test should fail when run against a build without the change
> and succeed when run against a build with the change. Unit tests aren't
> always practical, especially for changes such as performance improvements
> or fixes to bugs that are highly platform-dependent, but if practical then
> a unit test is required."
> >
> > Please consider adding it to the patch while the changes are being
> (preliminary) reviewed.
> >
> > -Pavel
> >
> >
> ---
> > [1] https://openjdk.java.net/contribute/
> >
> > > On 19 Aug 2019, at 17:20, Milan Mimica  wrote:
> > >
> > > Hello list
> > >
> > > Please find the attached patch that fixes JDK-8228580.
> > >
> > > It works the similar way UDP timeout does: calculate the initial
> > > timeout from retry attempt, and account for duration of every blocking
> > > call on the TCP socket.
> > >
> > > I am listed as Author[1] on "jdk" project.
> > >
> > > [1] https://openjdk.java.net/census#mmimica
> > >
> > >
> > > --
> > > Milan Mimica
> > > 
> >
>
>
> --
> Milan Mimica
>


-- 
Thanks,
Vyom