Re: RFR: 8285521: Minor improvements in java.net.URI [v2]

[Сергей Цыпанов]

> - use `String.equalsIgnoreCase()` instead of hand-written code relying on 
> `String.charAt()`
> - use `String.compareToIgnoreCase()` instead of hand-written code relying on 
> `String.charAt()`
> - drop branches that are never executed
> - drop unused argument from `URI.resolvePath()`
> - simplify String-related operations

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  8285521: Revert wrong changes

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8397/files
  - new: https://git.openjdk.java.net/jdk/pull/8397/files/408a3a8e..be41ccf9

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

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

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

Re: RFR: 8285521: Minor improvements in java.net.URI

[Сергей Цыпанов]

On Tue, 26 Apr 2022 07:02:55 GMT, Сергей Цыпанов  wrote:

> - use `String.equalsIgnoreCase()` instead of hand-written code relying on 
> `String.charAt()`
> - use `String.compareToIgnoreCase()` instead of hand-written code relying on 
> `String.charAt()`
> - drop branches that are never executed
> - drop unused argument from `URI.resolvePath()`
> - simplify String-related operations

Ok, I'll revert these changes then.

-

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

Re: RFR: 8285521: Minor improvements in java.net.URI

[Сергей Цыпанов]

On Thu, 19 May 2022 12:19:25 GMT, ExE Boss  wrote:

>> - use `String.equalsIgnoreCase()` instead of hand-written code relying on 
>> `String.charAt()`
>> - use `String.compareToIgnoreCase()` instead of hand-written code relying on 
>> `String.charAt()`
>> - drop branches that are never executed
>> - drop unused argument from `URI.resolvePath()`
>> - simplify String-related operations
>
> The `String.equalsIgnoreCase(…)` and `String.compareToIgnoreCase(…)` changes 
> are incorrect, as the `String.*IgnoreCase(…)` methods compare all **Unicode** 
> code points case‑insensitively using **Unicode** rules for the current or 
> specified locale, whereas the **URI** specification does case‑insensitive 
> comparison only for characters in the **US‑ASCII** range [[RFC3986]].
> 
> 
> 
> https://github.com/openjdk/jdk/blob/408a3a8e29006798071cd6f185e415bc2bc62282/src/java.base/share/classes/java/net/URI.java#L1825-L1830
>  
> https://github.com/openjdk/jdk/blob/408a3a8e29006798071cd6f185e415bc2bc62282/src/java.base/share/classes/java/net/URI.java#L1832-L1844
> 
> [RFC3986]: https://datatracker.ietf.org/doc/html/rfc3986

@ExE-Boss 

> String.*IgnoreCase(…) methods compare all Unicode code points 
> case‑insensitively using Unicode rules for the current or specified locale, 
> whereas the URI specification does case‑insensitive comparison only for 
> characters in the US‑ASCII range

Aren't all the items of US-ASCII range belong to Unicode regardless of locale?

-

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

Re: RFR: 8285521: Minor improvements in java.net.URI

[Сергей Цыпанов]

On Tue, 26 Apr 2022 07:02:55 GMT, Сергей Цыпанов  wrote:

> - use `String.equalsIgnoreCase()` instead of hand-written code relying on 
> `String.charAt()`
> - use `String.compareToIgnoreCase()` instead of hand-written code relying on 
> `String.charAt()`
> - drop branches that are never executed
> - drop unused argument from `URI.resolvePath()`
> - simplify String-related operations

Btw, I see that `java.net.URI` duplicates some methods from 
`sun.net.www.ParseUtil`. Should we consolidate them e.g. in a way of reusing 
functionality of `sun.net.www.ParseUtil` in `java.net.URI`?

-

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

Re: RFR: 8285521: Minor improvements in java.net.URI

[Сергей Цыпанов]

On Tue, 26 Apr 2022 11:35:10 GMT, Daniel Fuchs  wrote:

>> - use `String.equalsIgnoreCase()` instead of hand-written code relying on 
>> `String.charAt()`
>> - use `String.compareToIgnoreCase()` instead of hand-written code relying on 
>> `String.charAt()`
>> - drop branches that are never executed
>> - drop unused argument from `URI.resolvePath()`
>> - simplify String-related operations
>
> src/java.base/share/classes/java/net/URI.java line 1921:
> 
>> 1919: return sn - tn;
>> 1920: int val = 0;
>> 1921: int n = Math.min(sn, tn);
> 
> Can we drop this change? I wouldn't like `java.net.URI` to gratuitously 
> trigger the loading of the Math class.
> For the rest of the proposed changes, I will need to study them carefully and 
> that may take some time.
> Thanks!

@dfuch I've looked into class loading order and I see this:

...
[0.179s][info][class,init] 38 Initializing 
'java/util/ImmutableCollections$MapN'(no method) (0x00080013d4b8)
[0.180s][info][class,init] 39 Initializing 'java/lang/StringLatin1' 
(0x000800021cb8)
[0.180s][info][class,init] 40 Initializing 'java/lang/Math' (0x000800022828)
[0.180s][info][class,init] 41 Initializing 'java/lang/Number'(no method) 
(0x000800030080)
[0.180s][info][class,init] 42 Initializing 'java/lang/Float' 
(0x00080002fe10)
...
[0.234s][info][class,init] 194 Initializing 
'java/lang/module/ModuleDescriptor$Requires$Modifier' (0x0008001def98)
[0.234s][info][class,init] 195 Initializing 
'java/lang/module/ModuleDescriptor$Provides'(no method) (0x0008001dd800)
[0.235s][info][class,init] 196 Initializing 'java/net/URI' (0x000800142ff0)
[0.236s][info][class,init] 197 Initializing 'java/net/URI$1'(no method) 
(0x0008001f4368)
[0.236s][info][class,init] 198 Initializing 
'jdk/internal/module/SystemModuleFinders$2'(no method) (0x0008001d4790)
[0.236s][info][class,init] 199 Initializing 
'jdk/internal/module/SystemModuleFinders$3'(no method) (0x0008001d50b0)

I.e. `Math` is loaded far before `URI`. Am I missing something?

-

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

RFR: 8285521: Minor improvements in java.net.URI

[Сергей Цыпанов]

- use `String.equalsIgnoreCase()` instead of hand-written code relying on 
`String.charAt()`
- use `String.compareToIgnoreCase()` instead of hand-written code relying on 
`String.charAt()`
- drop branches that are never executed
- drop unused argument from `URI.resolvePath()`
- simplify String-related operations

-

Commit messages:
 - 8285521: Minor improvements in java.net.URI
 - 8285521: Minor improvements in java.net.URI

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

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

Integrated: 8276166: Remove dead code from MimeTable and MimeEntry

[Сергей Цыпанов]

On Fri, 29 Oct 2021 11:20:57 GMT, Сергей Цыпанов  wrote:

> There are unused methods/constructors in mentioned classes that can be safely 
> removed.

This pull request has now been integrated.

Changeset: cf977e88
Author:Sergey Tsypanov 
Committer: Julia Boes 
URL:   
https://git.openjdk.java.net/jdk/commit/cf977e88ecc64b549f332efe01578fca9f435060
Stats: 44 lines in 2 files changed: 0 ins; 41 del; 3 mod

8276166: Remove dead code from MimeTable and MimeEntry

Reviewed-by: dfuchs

-

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

Re: RFR: 8276166: Remove dead code from MimeTable and MimeEntry

[Сергей Цыпанов]

On Wed, 19 Jan 2022 16:55:44 GMT, Julia Boes  wrote:

>> Not now
>
> Happy to /sponsor once you /integrate, @stsypanov.

@FrauBoes thanks!

-

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

Re: RFR: 8276166: Remove dead code from MimeTable and MimeEntry

[Сергей Цыпанов]

On Fri, 29 Oct 2021 11:20:57 GMT, Сергей Цыпанов  wrote:

> There are unused methods/constructors in mentioned classes that can be safely 
> removed.

Not now

-

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

Integrated: 8277868: Use Comparable.compare() instead of surrogate code

[Сергей Цыпанов]

On Fri, 26 Nov 2021 12:46:59 GMT, Сергей Цыпанов  wrote:

> Instead of something like
> 
> long x;
> long y;
> return (x < y) ? -1 : ((x == y) ? 0 : 1);
> 
> we can use `return Long.compare(x, y);`
> 
> All replacements are done with IDE.

This pull request has now been integrated.

Changeset: 20db7800
Author:Sergey Tsypanov 
Committer: Roger Riggs 
URL:   
https://git.openjdk.java.net/jdk/commit/20db7800a657b311eeac504a2bbae4adbc209dbf
Stats: 77 lines in 12 files changed: 2 ins; 54 del; 21 mod

8277868: Use Comparable.compare() instead of surrogate code

Reviewed-by: rriggs, aivanov

-

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

Re: RFR: 8277868: Use Comparable.compare() instead of surrogate code [v3]

[Сергей Цыпанов]

On Tue, 7 Dec 2021 12:01:27 GMT, Alexey Ivanov  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8277868: Inline local var
>
> src/java.base/share/classes/java/util/Calendar.java line 3420:
> 
>> 3418: private int compareTo(long t) {
>> 3419: long thisTime = getMillisOf(this);
>> 3420: return Long.compare(thisTime, t);
> 
> Probably, in this case `thisTime` variable can also be dropped.

Inlined

> src/java.base/share/classes/java/util/Date.java line 977:
> 
>> 975: long thisTime = getMillisOf(this);
>> 976: long anotherTime = getMillisOf(anotherDate);
>> 977: return Long.compare(thisTime, anotherTime);
> 
> Looks like local variables can also be dropped here as each value is used 
> once.

Inlined

-

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

Re: RFR: 8277868: Use Comparable.compare() instead of surrogate code [v4]

[Сергей Цыпанов]

> Instead of something like
> 
> long x;
> long y;
> return (x < y) ? -1 : ((x == y) ? 0 : 1);
> 
> we can use `return Long.compare(x, y);`
> 
> All replacements are done with IDE.

Сергей Цыпанов has updated the pull request incrementally with two additional 
commits since the last revision:

 - 8277868: Revert irrelevant changes
 - 8277868: Inline local vars

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6575/files
  - new: https://git.openjdk.java.net/jdk/pull/6575/files/1b3a5d4b..bb06bf2f

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

  Stats: 9 lines in 3 files changed: 0 ins; 3 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6575.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6575/head:pull/6575

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

Re: RFR: 8277868: Use Comparable.compare() instead of surrogate code [v2]

[Сергей Цыпанов]

On Mon, 6 Dec 2021 17:48:37 GMT, Phil Race  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8277868: Use Integer.signum() in BasicTableUI
>
> src/java.desktop/share/classes/sun/java2d/Spans.java line 322:
> 
>> 320: float otherStart = otherSpan.getStart();
>> 321: 
>> 322: return Float.compare(mStart, otherStart);
> 
> We don't need the variable any more, do we ?

inlined

-

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

Re: RFR: 8277868: Use Comparable.compare() instead of surrogate code [v3]

[Сергей Цыпанов]

> Instead of something like
> 
> long x;
> long y;
> return (x < y) ? -1 : ((x == y) ? 0 : 1);
> 
> we can use `return Long.compare(x, y);`
> 
> All replacements are done with IDE.

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  8277868: Inline local var

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6575/files
  - new: https://git.openjdk.java.net/jdk/pull/6575/files/6744a562..1b3a5d4b

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

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

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

Re: RFR: 8277868: Use Comparable.compare() instead of surrogate code [v2]

[Сергей Цыпанов]

On Mon, 6 Dec 2021 17:46:22 GMT, Phil Race  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8277868: Use Integer.signum() in BasicTableUI
>
> src/java.desktop/share/classes/java/awt/geom/Line2D.java line 115:
> 
>> 113:  */
>> 114: public double getX1() {
>> 115: return x1;
> 
> What do these changes have to do with the subject of the PR ?

Just a tiny clean-up in on of affected files. Do you want this to be reverted?

-

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

Re: RFR: 8277868: Use Comparable.compare() instead of surrogate code [v2]

[Сергей Цыпанов]

> Instead of something like
> 
> long x;
> long y;
> return (x < y) ? -1 : ((x == y) ? 0 : 1);
> 
> we can use `return Long.compare(x, y);`
> 
> All replacements are done with IDE.

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  8277868: Use Integer.signum() in BasicTableUI

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6575/files
  - new: https://git.openjdk.java.net/jdk/pull/6575/files/8fa8242a..6744a562

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

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

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

Re: RFR: 8277868: Use Comparable.compare() instead of surrogate code [v2]

[Сергей Цыпанов]

On Sat, 27 Nov 2021 22:50:55 GMT, Michael Bien  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8277868: Use Integer.signum() in BasicTableUI
>
> src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTableUI.java line 
> 249:
> 
>> 247: 
>> 248: private static int sign(int num) {
>> 249: return Integer.compare(num, 0);
> 
> => Integer.signum(num)

Done!

-

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

RFR: 8277868: Use Comparable.compare() instead of surrogate code

[Сергей Цыпанов]

Instead of something like

long x;
long y;
return (x < y) ? -1 : ((x == y) ? 0 : 1);

we can use `return Long.compare(x, y);`

All replacements are done with IDE.

-

Commit messages:
 - 8277868: Use Comparable.compare() instead of surrogate code

Changes: https://git.openjdk.java.net/jdk/pull/6575/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6575=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8277868
  Stats: 70 lines in 12 files changed: 2 ins; 44 del; 24 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6575.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6575/head:pull/6575

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

Re: RFR: 8276166: Remove dead code from MimeTable and MimeEntry

[Сергей Цыпанов]

On Fri, 29 Oct 2021 11:20:57 GMT, Сергей Цыпанов  wrote:

> There are unused methods/constructors in mentioned classes that can be safely 
> removed.

Let's wait a bit

-

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

RFR: 8276166: Remove dead code from MimeTable and MimeEntry

[Сергей Цыпанов]

There are unused methods/constructors in mentioned classes that can be safely 
removed.

-

Commit messages:
 - 8276166: Remove dead code from MimeTable and MimeEntry

Changes: https://git.openjdk.java.net/jdk/pull/6169/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6169=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8276166
  Stats: 45 lines in 2 files changed: 0 ins; 41 del; 4 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6169.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6169/head:pull/6169

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

Integrated: 8267840: Improve URLStreamHandler.parseURL()

[Сергей Цыпанов]

On Fri, 18 Jun 2021 07:31:14 GMT, Сергей Цыпанов 
 wrote:

> There is an optimization opportunity for the widespread use-case when a 
> resource is read from classpath using 
> `getClass().getClassLoader().getResource()` or 
> `getClass().getClassLoader().getResourceAsStream()`.
> 
> Pay attention to lines starting from 261. In case I run something like
> 
> var props = 
> getClass().getClassLoader().getResource("/application.properties");
> 
> I get into the if-else block starting from 251 and here 'separator' variable 
> is an empty String. In this case we can skip 'separator' from concatenation 
> chain and use `String.concat()` as there are only two items concatenated.
> 
> In the opposite case `separator` variable is `"/"` and at the same time `ind` 
> variable is `-1`. This means that expression `path.substring(0, ind + 1)` 
> always returns an empty String and again can be excluded from concatenation 
> chain allowing usage of `String.concat()` which allows to dodge utilization 
> of `StringBuilder` (here `StringConcatFactory` is not available, see 
> https://github.com/openjdk/jdk/pull/3627)
> 
> In the next else-block, starting from 274, again, `String.concat()` is 
> applicable.
> 
> In another if-else block, starting from 277, when id is 0 again 
> path.substring(0, ind) returns empty String making concatenation pointless 
> and avoidable.
> 
> There are also some other minor clean-ups possible regarding constant 
> conditions (lines 252 and 161).
> 
> The change allows to reduce significantly resource look-up costs for a very 
> wide-spread case:
> 
> @State(Scope.Benchmark)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class ClassGetResourceBenchmark {
> private final Class clazz = getClass();
> 
> @Benchmark
> public URL getResource() {
> return clazz.getResource("/application.properties");
> }
> }
> 
> The change allows to reduce memory consumption significantly:
> 
> before
> 
> Benchmark   Mode  
> Cnt Score Error   Units
> ClassGetResourceBenchmark.getResource   avgt  
> 100  1649,367 ±   5,904   ns/op
> ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt  
> 100   619,204 ±   2,413  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt  
> 100  1339,232 ±   4,909B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt  
> 100   627,192 ±  74,972  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt  
> 100  1356,681 ± 162,354B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt  
> 100 0,119 ±   0,100  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt  
> 100 0,257 ±   0,217B/op
> ClassGetResourceBenchmark.getResource:·gc.count avgt  
> 100   128,000counts
> ClassGetResourceBenchmark.getResource:·gc.time  avgt  
> 100   227,000ms
> 
> after
> 
> Benchmark   Mode  
> Cnt Score Error   Units
> ClassGetResourceBenchmark.getResource   avgt  
> 100  1599,948 ±   4,115   ns/op
> ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt  
> 100   358,434 ±   0,922  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt  
> 100   752,016 ±   0,004B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt  
> 100   342,778 ±  76,490  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt  
> 100   719,264 ± 160,513B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt  
> 100 0,008 ±   0,005  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt  
> 100 0,017 ±   0,010B/op
> ClassGetResourceBenchmark.getResource:·gc.count avgt  
> 10070,000counts
> ClassGetResourceBenchmark.getResource:·gc.time  avgt  
> 100   151,000ms

This pull request has now been integrated.

Changeset: d7fc9e41
Author:Sergey Tsypanov 
Committer: Claes Redestad 
URL:   
https://git.openjdk.java.net/jdk/commit/d7fc9e4171efa4154951cf353df10f9bacbed7ab
Stats: 25 lines in 1 file changed: 3 ins; 8 del; 14 mod

8267840: Improve URLStreamHandler.parseURL()

Reviewed-by: dfuchs, redestad

-

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

Re: RFR: 8267840: Improve URLStreamHandler.parseURL()

[Сергей Цыпанов]

On Wed, 23 Jun 2021 10:03:48 GMT, Сергей Цыпанов 
 wrote:

>> Hi Sergey,
>> 
>> That exception means you're using an obsolete version of jtreg: you need 
>> jtreg-6+1 now.
>> 
>> best regards,
>> -- daniel
>
> @dfuch I've fixed the issue and retested the changes, tier1 is ok, in tier2 
> some IPv6 tests on my machine are failing, but they fail both on `master` and 
> `8267840` branches, so seem to be unrelated to the change.

> @stsypanov This pull request has been inactive for more than 4 weeks and will 
> be automatically closed if another 4 weeks passes without any activity. To 
> avoid this, simply add a new comment to the pull request. Feel free to ask 
> for assistance if you need help with progressing this pull request towards 
> integration!

Let's wait

-

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

Integrated: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

On Wed, 2 Jun 2021 12:10:46 GMT, Сергей Цыпанов 
 wrote:

> After I've renamed remove branch GitHub for some reason has closed original 
> https://github.com/openjdk/jdk/pull/2744, so I've decided to recreate it.

This pull request has now been integrated.

Changeset: 249d6418
Author:Sergey Tsypanov 
Committer: Claes Redestad 
URL:   
https://git.openjdk.java.net/jdk/commit/249d641889c6f9aed6957502d5fca9c74c9baceb
Stats: 47 lines in 9 files changed: 0 ins; 2 del; 45 mod

8263561: Re-examine uses of LinkedList

Reviewed-by: redestad

-

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

Re: RFR: 8263561: Re-examine uses of LinkedList [v6]

[Сергей Цыпанов]

> After I've renamed remove branch GitHub for some reason has closed original 
> https://github.com/openjdk/jdk/pull/2744, so I've decided to recreate it.

Сергей Цыпанов has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 11 commits:

 - Merge branch 'master' into 8263561
   
   # Conflicts:
   #src/java.base/share/classes/jdk/internal/util/jar/JarIndex.java
 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
   
   # Conflicts:
   #src/java.base/unix/classes/sun/net/dns/ResolverConfigurationImpl.java
 - Merge branch 'master' into purge-linked-list
 - 8263561: Use sized constructor where reasonable
 - 8263561: Use interface List instead of particular type where possible
 - 8263561: Rename requestList -> requests
 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/7cc1eb3e...dea42cac

-

Changes: https://git.openjdk.java.net/jdk/pull/4304/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4304=05
  Stats: 47 lines in 9 files changed: 0 ins; 2 del; 45 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4304.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4304/head:pull/4304

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

Re: RFR: 8263561: Re-examine uses of LinkedList [v5]

[Сергей Цыпанов]

> After I've renamed remove branch GitHub for some reason has closed original 
> https://github.com/openjdk/jdk/pull/2744, so I've decided to recreate it.

Сергей Цыпанов has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains 10 commits:

 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
   
   # Conflicts:
   #src/java.base/unix/classes/sun/net/dns/ResolverConfigurationImpl.java
 - Merge branch 'master' into purge-linked-list
 - 8263561: Use sized constructor where reasonable
 - 8263561: Use interface List instead of particular type where possible
 - 8263561: Rename requestList -> requests
 - 8263561: Re-examine uses of LinkedList

-

Changes: https://git.openjdk.java.net/jdk/pull/4304/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4304=04
  Stats: 48 lines in 9 files changed: 0 ins; 2 del; 46 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4304.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4304/head:pull/4304

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

Re: RFR: 8263561: Re-examine uses of LinkedList [v4]

[Сергей Цыпанов]

> After I've renamed remove branch GitHub for some reason has closed original 
> https://github.com/openjdk/jdk/pull/2744, so I've decided to recreate it.

Сергей Цыпанов has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains ten commits:

 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
   
   # Conflicts:
   #src/java.base/unix/classes/sun/net/dns/ResolverConfigurationImpl.java
 - Merge branch 'master' into purge-linked-list
 - 8263561: Use sized constructor where reasonable
 - 8263561: Use interface List instead of particular type where possible
 - 8263561: Rename requestList -> requests
 - 8263561: Re-examine uses of LinkedList

-

Changes: https://git.openjdk.java.net/jdk/pull/4304/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4304=03
  Stats: 48 lines in 9 files changed: 0 ins; 2 del; 46 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4304.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4304/head:pull/4304

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

Re: RFR: 8267840: Improve URLStreamHandler.parseURL() [v3]

[Сергей Цыпанов]

> There is an optimization opportunity for the widespread use-case when a 
> resource is read from classpath using 
> `getClass().getClassLoader().getResource()` or 
> `getClass().getClassLoader().getResourceAsStream()`.
> 
> Pay attention to lines starting from 261. In case I run something like
> 
> var props = 
> getClass().getClassLoader().getResource("/application.properties");
> 
> I get into the if-else block starting from 251 and here 'separator' variable 
> is an empty String. In this case we can skip 'separator' from concatenation 
> chain and use `String.concat()` as there are only two items concatenated.
> 
> In the opposite case `separator` variable is `"/"` and at the same time `ind` 
> variable is `-1`. This means that expression `path.substring(0, ind + 1)` 
> always returns an empty String and again can be excluded from concatenation 
> chain allowing usage of `String.concat()` which allows to dodge utilization 
> of `StringBuilder` (here `StringConcatFactory` is not available, see 
> https://github.com/openjdk/jdk/pull/3627)
> 
> In the next else-block, starting from 274, again, `String.concat()` is 
> applicable.
> 
> In another if-else block, starting from 277, when id is 0 again 
> path.substring(0, ind) returns empty String making concatenation pointless 
> and avoidable.
> 
> There are also some other minor clean-ups possible regarding constant 
> conditions (lines 252 and 161).
> 
> The change allows to reduce significantly resource look-up costs for a very 
> wide-spread case:
> 
> @State(Scope.Benchmark)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class ClassGetResourceBenchmark {
> private final Class clazz = getClass();
> 
> @Benchmark
> public URL getResource() {
> return clazz.getResource("/application.properties");
> }
> }
> 
> The change allows to reduce memory consumption significantly:
> 
> before
> 
> Benchmark   Mode  
> Cnt Score Error   Units
> ClassGetResourceBenchmark.getResource   avgt  
> 100  1649,367 ±   5,904   ns/op
> ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt  
> 100   619,204 ±   2,413  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt  
> 100  1339,232 ±   4,909B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt  
> 100   627,192 ±  74,972  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt  
> 100  1356,681 ± 162,354B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt  
> 100 0,119 ±   0,100  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt  
> 100 0,257 ±   0,217B/op
> ClassGetResourceBenchmark.getResource:·gc.count avgt  
> 100   128,000counts
> ClassGetResourceBenchmark.getResource:·gc.time  avgt  
> 100   227,000ms
> 
> after
> 
> Benchmark   Mode  
> Cnt Score Error   Units
> ClassGetResourceBenchmark.getResource   avgt  
> 100  1599,948 ±   4,115   ns/op
> ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt  
> 100   358,434 ±   0,922  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt  
> 100   752,016 ±   0,004B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt  
> 100   342,778 ±  76,490  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt  
> 100   719,264 ± 160,513B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt  
> 100 0,008 ±   0,005  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt  
> 100 0,017 ±   0,010B/op
> ClassGetResourceBenchmark.getResource:·gc.count avgt  
> 10070,000counts
> ClassGetResourceBenchmark.getResource:·gc.time  avgt  
> 100   151,000ms

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  8267840: Improve URLStreamHandler.parseURL() - remove unnecessary 
String.concat() call

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4526/files
  - new: https://git.openjdk.java.net/jdk/pull/4526/files/1a2ea299..2d2e1b59

Re: RFR: 8267840: Improve URLStreamHandler.parseURL() [v2]

[Сергей Цыпанов]

On Fri, 2 Jul 2021 13:57:47 GMT, Daniel Fuchs  wrote:

>> Сергей Цыпанов has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains three additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'master' into 8267840
>>  - Merge branch 'master' into 8267840
>>  - 8267840: Improve URLStreamHandler.parseURL()
>
> src/java.base/share/classes/java/net/URLStreamHandler.java line 275:
> 
>> 273: path = "/";
>> 274: } else {
>> 275: path = path.substring(0, ind).concat("/");
> 
> would that be equivalent to 
> 
> path = path.substring(0, ind + 1);
> 
> given that ind = path.lastIndexOf('/') ?

Right, fixed!

-

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

Re: RFR: 8263561: Re-examine uses of LinkedList [v3]

[Сергей Цыпанов]

> After I've renamed remove branch GitHub for some reason has closed original 
> https://github.com/openjdk/jdk/pull/2744, so I've decided to recreate it.

Сергей Цыпанов has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains eight commits:

 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
 - Merge branch 'master' into 8263561
   
   # Conflicts:
   #src/java.base/unix/classes/sun/net/dns/ResolverConfigurationImpl.java
 - Merge branch 'master' into purge-linked-list
 - 8263561: Use sized constructor where reasonable
 - 8263561: Use interface List instead of particular type where possible
 - 8263561: Rename requestList -> requests
 - 8263561: Re-examine uses of LinkedList

-

Changes: https://git.openjdk.java.net/jdk/pull/4304/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4304=02
  Stats: 48 lines in 9 files changed: 0 ins; 2 del; 46 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4304.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4304/head:pull/4304

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

Re: RFR: 8267840: Improve URLStreamHandler.parseURL() [v2]

[Сергей Цыпанов]

> There is an optimization opportunity for the widespread use-case when a 
> resource is read from classpath using 
> `getClass().getClassLoader().getResource()` or 
> `getClass().getClassLoader().getResourceAsStream()`.
> 
> Pay attention to lines starting from 261. In case I run something like
> 
> var props = 
> getClass().getClassLoader().getResource("/application.properties");
> 
> I get into the if-else block starting from 251 and here 'separator' variable 
> is an empty String. In this case we can skip 'separator' from concatenation 
> chain and use `String.concat()` as there are only two items concatenated.
> 
> In the opposite case `separator` variable is `"/"` and at the same time `ind` 
> variable is `-1`. This means that expression `path.substring(0, ind + 1)` 
> always returns an empty String and again can be excluded from concatenation 
> chain allowing usage of `String.concat()` which allows to dodge utilization 
> of `StringBuilder` (here `StringConcatFactory` is not available, see 
> https://github.com/openjdk/jdk/pull/3627)
> 
> In the next else-block, starting from 274, again, `String.concat()` is 
> applicable.
> 
> In another if-else block, starting from 277, when id is 0 again 
> path.substring(0, ind) returns empty String making concatenation pointless 
> and avoidable.
> 
> There are also some other minor clean-ups possible regarding constant 
> conditions (lines 252 and 161).
> 
> The change allows to reduce significantly resource look-up costs for a very 
> wide-spread case:
> 
> @State(Scope.Benchmark)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class ClassGetResourceBenchmark {
> private final Class clazz = getClass();
> 
> @Benchmark
> public URL getResource() {
> return clazz.getResource("/application.properties");
> }
> }
> 
> The change allows to reduce memory consumption significantly:
> 
> before
> 
> Benchmark   Mode  
> Cnt Score Error   Units
> ClassGetResourceBenchmark.getResource   avgt  
> 100  1649,367 ±   5,904   ns/op
> ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt  
> 100   619,204 ±   2,413  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt  
> 100  1339,232 ±   4,909B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt  
> 100   627,192 ±  74,972  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt  
> 100  1356,681 ± 162,354B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt  
> 100 0,119 ±   0,100  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt  
> 100 0,257 ±   0,217B/op
> ClassGetResourceBenchmark.getResource:·gc.count avgt  
> 100   128,000counts
> ClassGetResourceBenchmark.getResource:·gc.time  avgt  
> 100   227,000ms
> 
> after
> 
> Benchmark   Mode  
> Cnt Score Error   Units
> ClassGetResourceBenchmark.getResource   avgt  
> 100  1599,948 ±   4,115   ns/op
> ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt  
> 100   358,434 ±   0,922  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt  
> 100   752,016 ±   0,004B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt  
> 100   342,778 ±  76,490  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt  
> 100   719,264 ± 160,513B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt  
> 100 0,008 ±   0,005  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt  
> 100 0,017 ±   0,010B/op
> ClassGetResourceBenchmark.getResource:·gc.count avgt  
> 10070,000counts
> ClassGetResourceBenchmark.getResource:·gc.time  avgt  
> 100   151,000ms

Сергей Цыпанов has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains three additional 
commits since the last revision:

 - Merge branch 'master' into 8267840
 - Merge branch 'master' into 8267840
 - 8267840: Improve URLStreamHandler.pa

Re: RFR: 8267840: Improve URLStreamHandler.parseURL()

[Сергей Цыпанов]

On Tue, 22 Jun 2021 11:59:11 GMT, Daniel Fuchs  wrote:

>> There is an optimization opportunity for the widespread use-case when a 
>> resource is read from classpath using 
>> `getClass().getClassLoader().getResource()` or 
>> `getClass().getClassLoader().getResourceAsStream()`.
>> 
>> Pay attention to lines starting from 261. In case I run something like
>> 
>> var props = 
>> getClass().getClassLoader().getResource("/application.properties");
>> 
>> I get into the if-else block starting from 251 and here 'separator' variable 
>> is an empty String. In this case we can skip 'separator' from concatenation 
>> chain and use `String.concat()` as there are only two items concatenated.
>> 
>> In the opposite case `separator` variable is `"/"` and at the same time 
>> `ind` variable is `-1`. This means that expression `path.substring(0, ind + 
>> 1)` always returns an empty String and again can be excluded from 
>> concatenation chain allowing usage of `String.concat()` which allows to 
>> dodge utilization of `StringBuilder` (here `StringConcatFactory` is not 
>> available, see https://github.com/openjdk/jdk/pull/3627)
>> 
>> In the next else-block, starting from 274, again, `String.concat()` is 
>> applicable.
>> 
>> In another if-else block, starting from 277, when id is 0 again 
>> path.substring(0, ind) returns empty String making concatenation pointless 
>> and avoidable.
>> 
>> There are also some other minor clean-ups possible regarding constant 
>> conditions (lines 252 and 161).
>> 
>> The change allows to reduce significantly resource look-up costs for a very 
>> wide-spread case:
>> 
>> @State(Scope.Benchmark)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class ClassGetResourceBenchmark {
>> private final Class clazz = getClass();
>> 
>> @Benchmark
>> public URL getResource() {
>> return clazz.getResource("/application.properties");
>> }
>> }
>> 
>> The change allows to reduce memory consumption significantly:
>> 
>> before
>> 
>> Benchmark   Mode 
>>  Cnt Score Error   Units
>> ClassGetResourceBenchmark.getResource   avgt 
>>  100  1649,367 ±   5,904   ns/op
>> ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt 
>>  100   619,204 ±   2,413  MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt 
>>  100  1339,232 ±   4,909B/op
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt 
>>  100   627,192 ±  74,972  MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt 
>>  100  1356,681 ± 162,354B/op
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt 
>>  100 0,119 ±   0,100  MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt 
>>  100 0,257 ±   0,217B/op
>> ClassGetResourceBenchmark.getResource:·gc.count avgt 
>>  100   128,000counts
>> ClassGetResourceBenchmark.getResource:·gc.time  avgt 
>>  100   227,000ms
>> 
>> after
>> 
>> Benchmark   Mode 
>>  Cnt Score Error   Units
>> ClassGetResourceBenchmark.getResource   avgt 
>>  100  1599,948 ±   4,115   ns/op
>> ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt 
>>  100   358,434 ±   0,922  MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt 
>>  100   752,016 ±   0,004B/op
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt 
>>  100   342,778 ±  76,490  MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt 
>>  100   719,264 ± 160,513B/op
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt 
>>  100 0,008 ±   0,005  MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt 
>>  100 0,017 ±   0,010B/op
>> ClassGetResourceBenchmark.getResource:·gc.count avgt 
>>  10070,000counts
>> ClassGetResourceBenchmark.getResource:·gc.time  avgt 
>>  100   151,000ms
>
> Hi Sergey,
> 
> That exception means you're using an obsolete version of jtreg: you need 
> jtreg-6+1 now.
> 
> best regards,
> -- daniel

@dfuch I've fixed the issue and retested the changes, tier1 is ok, in tier2 
some IPv6 tests on my machine are failing, but they fail both on `master` and 
`8267840` branches, so seem to be unrelated to the change.

-

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

Re: RFR: 8267840: Improve URLStreamHandler.parseURL()

[Сергей Цыпанов]

On Fri, 18 Jun 2021 07:31:14 GMT, Сергей Цыпанов 
 wrote:

> There is an optimization opportunity for the widespread use-case when a 
> resource is read from classpath using 
> `getClass().getClassLoader().getResource()` or 
> `getClass().getClassLoader().getResourceAsStream()`.
> 
> Pay attention to lines starting from 261. In case I run something like
> 
> var props = 
> getClass().getClassLoader().getResource("/application.properties");
> 
> I get into the if-else block starting from 251 and here 'separator' variable 
> is an empty String. In this case we can skip 'separator' from concatenation 
> chain and use `String.concat()` as there are only two items concatenated.
> 
> In the opposite case `separator` variable is `"/"` and at the same time `ind` 
> variable is `-1`. This means that expression `path.substring(0, ind + 1)` 
> always returns an empty String and again can be excluded from concatenation 
> chain allowing usage of `String.concat()` which allows to dodge utilization 
> of `StringBuilder` (here `StringConcatFactory` is not available, see 
> https://github.com/openjdk/jdk/pull/3627)
> 
> In the next else-block, starting from 274, again, `String.concat()` is 
> applicable.
> 
> In another if-else block, starting from 277, when id is 0 again 
> path.substring(0, ind) returns empty String making concatenation pointless 
> and avoidable.
> 
> There are also some other minor clean-ups possible regarding constant 
> conditions (lines 252 and 161).
> 
> The change allows to reduce significantly resource look-up costs for a very 
> wide-spread case:
> 
> @State(Scope.Benchmark)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class ClassGetResourceBenchmark {
> private final Class clazz = getClass();
> 
> @Benchmark
> public URL getResource() {
> return clazz.getResource("/application.properties");
> }
> }
> 
> The change allows to reduce memory consumption significantly:
> 
> before
> 
> Benchmark   Mode  
> Cnt Score Error   Units
> ClassGetResourceBenchmark.getResource   avgt  
> 100  1649,367 ±   5,904   ns/op
> ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt  
> 100   619,204 ±   2,413  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt  
> 100  1339,232 ±   4,909B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt  
> 100   627,192 ±  74,972  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt  
> 100  1356,681 ± 162,354B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt  
> 100 0,119 ±   0,100  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt  
> 100 0,257 ±   0,217B/op
> ClassGetResourceBenchmark.getResource:·gc.count avgt  
> 100   128,000counts
> ClassGetResourceBenchmark.getResource:·gc.time  avgt  
> 100   227,000ms
> 
> after
> 
> Benchmark   Mode  
> Cnt Score Error   Units
> ClassGetResourceBenchmark.getResource   avgt  
> 100  1599,948 ±   4,115   ns/op
> ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt  
> 100   358,434 ±   0,922  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt  
> 100   752,016 ±   0,004B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt  
> 100   342,778 ±  76,490  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt  
> 100   719,264 ± 160,513B/op
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt  
> 100 0,008 ±   0,005  MB/sec
> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt  
> 100 0,017 ±   0,010B/op
> ClassGetResourceBenchmark.getResource:·gc.count avgt  
> 10070,000counts
> ClassGetResourceBenchmark.getResource:·gc.time  avgt  
> 100   151,000ms

Hi Daniel, tier1 and tier2 is ok in pipeline, but locally I'm facing

Test selection 'jdk:tier1', will run:
* jtreg:test/jdk:tier1

Running test 'jtreg:test/jdk:tier1'
Error: Unexpected exception occurred! java.lang.IllegalArgumentException: 6+1
java.lang.IllegalArgumentException: 6+1
at 

RFR: 8267840: Improve URLStreamHandler.parseURL()

[Сергей Цыпанов]

There is an optimization opportunity for the widespread use-case when a 
resource is read from classpath using 
`getClass().getClassLoader().getResource()` or 
`getClass().getClassLoader().getResourceAsStream()`.

Pay attention to lines starting from 261. In case I run something like

var props = getClass().getClassLoader().getResource("/application.properties");

I get into the if-else block starting from 251 and here 'separator' variable is 
an empty String. In this case we can skip 'separator' from concatenation chain 
and use `String.concat()` as there are only two items concatenated.

In the opposite case `separator` variable is `"/"` and at the same time `ind` 
variable is `-1`. This means that expression `path.substring(0, ind + 1)` 
always returns an empty String and again can be excluded from concatenation 
chain allowing usage of `String.concat()` which allows to dodge utilization of 
`StringBuilder` (here `StringConcatFactory` is not available, see 
https://github.com/openjdk/jdk/pull/3627)

In the next else-block, starting from 274, again, `String.concat()` is 
applicable.

In another if-else block, starting from 277, when id is 0 again 
path.substring(0, ind) returns empty String making concatenation pointless and 
avoidable.

There are also some other minor clean-ups possible regarding constant 
conditions (lines 252 and 161).

The change allows to reduce significantly resource look-up costs for a very 
wide-spread case:

@State(Scope.Benchmark)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
public class ClassGetResourceBenchmark {
private final Class clazz = getClass();

@Benchmark
public URL getResource() {
return clazz.getResource("/application.properties");
}
}

The change allows to reduce memory consumption significantly:

before

Benchmark   Mode  
Cnt Score Error   Units
ClassGetResourceBenchmark.getResource   avgt  
100  1649,367 ±   5,904   ns/op
ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt  
100   619,204 ±   2,413  MB/sec
ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt  
100  1339,232 ±   4,909B/op
ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt  
100   627,192 ±  74,972  MB/sec
ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt  
100  1356,681 ± 162,354B/op
ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt  
100 0,119 ±   0,100  MB/sec
ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt  
100 0,257 ±   0,217B/op
ClassGetResourceBenchmark.getResource:·gc.count avgt  
100   128,000counts
ClassGetResourceBenchmark.getResource:·gc.time  avgt  
100   227,000ms

after

Benchmark   Mode  
Cnt Score Error   Units
ClassGetResourceBenchmark.getResource   avgt  
100  1599,948 ±   4,115   ns/op
ClassGetResourceBenchmark.getResource:·gc.alloc.rateavgt  
100   358,434 ±   0,922  MB/sec
ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm   avgt  
100   752,016 ±   0,004B/op
ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space   avgt  
100   342,778 ±  76,490  MB/sec
ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm  avgt  
100   719,264 ± 160,513B/op
ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space   avgt  
100 0,008 ±   0,005  MB/sec
ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm  avgt  
100 0,017 ±   0,010B/op
ClassGetResourceBenchmark.getResource:·gc.count avgt  
10070,000counts
ClassGetResourceBenchmark.getResource:·gc.time  avgt  
100   151,000ms

-

Commit messages:
 - 8267840: Improve URLStreamHandler.parseURL()

Changes: https://git.openjdk.java.net/jdk/pull/4526/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4526=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8267840
  Stats: 25 lines in 1 file changed: 3 ins; 8 del; 14 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4526.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4526/head:pull/4526

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

RFR: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

After I've renamed remove branch GitHub for some reason has closed original 
https://github.com/openjdk/jdk/pull/2744, so I've decided to recreate it.

-

Commit messages:
 - Merge branch 'master' into 8263561
 - Merge branch 'master' into purge-linked-list
 - 8263561: Use sized constructor where reasonable
 - 8263561: Use interface List instead of particular type where possible
 - 8263561: Rename requestList -> requests
 - 8263561: Re-examine uses of LinkedList

Changes: https://git.openjdk.java.net/jdk/pull/4304/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4304=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263561
  Stats: 48 lines in 9 files changed: 0 ins; 2 del; 46 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4304.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4304/head:pull/4304

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

Withdrawn: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

On Fri, 26 Feb 2021 10:48:33 GMT, Сергей Цыпанов 
 wrote:

> The usage of `LinkedList` is senseless and can be replaced with either 
> `ArrayList` or `ArrayDeque` which are both more compact and effective.
> 
> jdk:tier1 and jdk:tier2 are both ok

This pull request has been closed without being integrated.

-

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

Re: RFR: 8263561: Re-examine uses of LinkedList [v3]

[Сергей Цыпанов]

> The usage of `LinkedList` is senseless and can be replaced with either 
> `ArrayList` or `ArrayDeque` which are both more compact and effective.
> 
> jdk:tier1 and jdk:tier2 are both ok

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  8263561: Use sized constructor where reasonable

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2744/files
  - new: https://git.openjdk.java.net/jdk/pull/2744/files/158006c6..40910fae

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

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

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

Re: RFR: 8263561: Re-examine uses of LinkedList [v2]

[Сергей Цыпанов]

> The usage of `LinkedList` is senseless and can be replaced with either 
> `ArrayList` or `ArrayDeque` which are both more compact and effective.
> 
> jdk:tier1 and jdk:tier2 are both ok

Сергей Цыпанов has refreshed the contents of this pull request, and previous 
commits have been removed. The incremental views will show differences compared 
to the previous content of the PR. The pull request contains three new commits 
since the last revision:

 - 8263561: Use interface List instead of particular type where possible
 - 8263561: Rename requestList -> requests
 - 8263561: Re-examine uses of LinkedList

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2744/files
  - new: https://git.openjdk.java.net/jdk/pull/2744/files/73029fe1..158006c6

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

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

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

Re: RFR: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

On Mon, 24 May 2021 09:25:08 GMT, liach  
wrote:

>> Should we then revert the changes to `JarIndex`?
>
> But don't people access these internal code at their own risk, as jdk may 
> change these code at any time without notice?

True, I just wonder whether it's OK to change internals when we know for sure 
that this breaks 3rd party code

-

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

Re: RFR: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

On Fri, 21 May 2021 15:12:20 GMT, Thiago Henrique Hüpner 
 wrote:

>>> IcedTea-Web seems to be using this method reflectively:
>>> https://github.com/AdoptOpenJDK/IcedTea-Web/blob/master/common/src/main/java/net/adoptopenjdk/icedteaweb/jdk89access/JarIndexAccess.java#L80
>> 
>> I assume this doesn't work with JDK 16, at least not without using 
>> --add-exports to export jdk.internal.util.jar.
>
> Just for completeness, they're using the add-exports:
> https://github.com/AdoptOpenJDK/IcedTea-Web/blob/master/launchers/itw-modularjdk.args#L19

Should we then revert the changes to `JarIndex`?

-

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

Re: RFR: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

On Fri, 21 May 2021 14:18:16 GMT, Daniel Fuchs  wrote:

>> The usage of `LinkedList` is senseless and can be replaced with either 
>> `ArrayList` or `ArrayDeque` which are both more compact and effective.
>> 
>> jdk:tier1 and jdk:tier2 are both ok
>
> src/java.base/share/classes/jdk/internal/util/jar/JarIndex.java line 264:
> 
>> 262: String jar = jarFiles[i];
>> 263: bw.write(jar + "\n");
>> 264: ArrayList jarlist = jarMap.get(jar);
> 
> Here again, jarList could probably be declared as `List`

Done

-

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

Re: RFR: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

On Fri, 21 May 2021 14:15:53 GMT, Daniel Fuchs  wrote:

>> The usage of `LinkedList` is senseless and can be replaced with either 
>> `ArrayList` or `ArrayDeque` which are both more compact and effective.
>> 
>> jdk:tier1 and jdk:tier2 are both ok
>
> src/java.base/share/classes/jdk/internal/util/jar/JarIndex.java line 155:
> 
>> 153:  */
>> 154: public List get(String fileName) {
>> 155: ArrayList jarFiles;
> 
> This could probably be declared as:
> 
> 
> List jarFiles;

Done

-

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

Re: RFR: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

On Fri, 26 Feb 2021 10:48:33 GMT, Сергей Цыпанов 
 wrote:

> The usage of `LinkedList` is senseless and can be replaced with either 
> `ArrayList` or `ArrayDeque` which are both more compact and effective.
> 
> jdk:tier1 and jdk:tier2 are both ok

Hi guys, any more comments here? Please ping me if I've missed something

-

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

Integrated: 8261880: Change nested classes in java.base to static nested classes where possible

[Сергей Цыпанов]

On Tue, 16 Feb 2021 14:30:58 GMT, Сергей Цыпанов 
 wrote:

> Non-static classes hold a link to their parent classes, which in many cases 
> can be avoided.

This pull request has now been integrated.

Changeset: 9425d3de
Author:Sergey Tsypanov 
Committer: Claes Redestad 
URL:   
https://git.openjdk.java.net/jdk/commit/9425d3de83fe8f4caddef03ffa3f4dd4de58f236
Stats: 15 lines in 11 files changed: 0 ins; 0 del; 15 mod

8261880: Change nested classes in java.base to static nested classes where 
possible

Reviewed-by: redestad

-

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

Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]

[Сергей Цыпанов]

On Thu, 20 May 2021 10:42:49 GMT, Claes Redestad  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8261880: Remove static from declarations of Holder nested classes
>
> Marked as reviewed by redestad (Reviewer).

@cl4es now you can sponsor :)

-

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

Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]

[Сергей Цыпанов]

On Wed, 17 Feb 2021 16:38:03 GMT, Сергей Цыпанов 
 wrote:

>> Non-static classes hold a link to their parent classes, which in many cases 
>> can be avoided.
>
> Сергей Цыпанов has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8261880: Remove static from declarations of Holder nested classes

Any more comments?

-

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

Re: RFR: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

On Mon, 15 Mar 2021 06:56:00 GMT, Alan Bateman  wrote:

>> Nothing outside of the JDK should be hacking into this private field of a 
>> non-exposed class, this should not be a concern here.
>
>> @AlanBateman so is it ok to keep `ArrayLists`?
> 
> One thing to look out for is usages of 2-arg add method that inserts at a 
> specific position. This shouldn't be a concern in URLClassPath.closeLoaders 
> (assuming this is this usage where the question arises).

@AlanBateman looks like `List.add(o, i)` is not used at all in scope of these 
changes.

-

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

RFR: 8264896: Remove redundant '& 0xFF' from int-to-byte cast

[Сергей Цыпанов]

When we do
byte b1 = (byte) (value & 0xFF);
we keep from int only 1 lower byte and exactly the same can be achieved with 
plain cast. See the test below:
public class Main {
  public static void main(String[] args) throws Exception {
IntStream.range(Integer.MIN_VALUE, Integer.MAX_VALUE).forEach(value -> {
  byte b1 = (byte) (value & 0xFF);
  byte b2 = (byte) value;
  if (b1 != b2) {
throw new RuntimeException("" + value);
  }
});
}

Also tier1 and tier2 are both OK.

-

Commit messages:
 - Merge branch 'master' into bitwise
 - Remove redundant '& 0xFF' from int-to-byte cast

Changes: https://git.openjdk.java.net/jdk/pull/2826/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2826=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264896
  Stats: 50 lines in 14 files changed: 0 ins; 0 del; 50 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2826.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2826/head:pull/2826

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

Integrated: 8263560: Remove needless wrapping with BufferedInputStream

[Сергей Цыпанов]

On Sat, 13 Mar 2021 22:29:23 GMT, Сергей Цыпанов 
 wrote:

> In some cases wrapping of `InputStream` with `BufferedInputStream` is 
> redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does 
> not require any buffer having one within.
> 
> Other cases are related to reading either a byte or short `byte[]`: in both 
> cases `BufferedInputStream.fill()` will be called resulting in load of much 
> bigger amount of data (8192 by default) than required.

This pull request has now been integrated.

Changeset: 1a681fa7
Author:Sergey Tsypanov 
Committer: Aleksey Shipilev 
URL:   https://git.openjdk.java.net/jdk/commit/1a681fa7
Stats: 12 lines in 2 files changed: 2 ins; 5 del; 5 mod

8263560: Remove needless wrapping with BufferedInputStream

Reviewed-by: prr, alanb, dfuchs, serb

-

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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v6]

[Сергей Цыпанов]

> In some cases wrapping of `InputStream` with `BufferedInputStream` is 
> redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does 
> not require any buffer having one within.
> 
> Other cases are related to reading either a byte or short `byte[]`: in both 
> cases `BufferedInputStream.fill()` will be called resulting in load of much 
> bigger amount of data (8192 by default) than required.

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  Revert HttpClient

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2992/files
  - new: https://git.openjdk.java.net/jdk/pull/2992/files/ff25cc4d..af4fcce4

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

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

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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]

[Сергей Цыпанов]

On Mon, 15 Mar 2021 15:04:49 GMT, Daniel Fuchs  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Fix error message
>
> src/java.base/share/classes/sun/net/www/http/HttpClient.java line 434:
> 
>> 432: int r = tmpbuf.read();
>> 433: if (r == -1) {
>> 434: logFinest("HttpClient.available(): " +
> 
> There are some subtle things going on there. Using a `BufferedInputStream` 
> ensures that all the bytes available on the socket will be read, up to the 
> buffer capacity. Can you revert this change? I'd rather that this clean up be 
> handled separately. I have logged 
> https://bugs.openjdk.java.net/browse/JDK-8263599.

Sure, I'll revert this.

-

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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]

[Сергей Цыпанов]

On Mon, 15 Mar 2021 11:41:07 GMT, Alan Bateman  wrote:

>> Done
>
>> Done
> 
> I think I'd prefer if the exception message would say that the JMOD is 
> invalid or that the JMOD file is truncated (because I don't think anyone 
> seeing this exception will know anything about the header).

Fixed

-

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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v5]

[Сергей Цыпанов]

> In some cases wrapping of `InputStream` with `BufferedInputStream` is 
> redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does 
> not require any buffer having one within.
> 
> Other cases are related to reading either a byte or short `byte[]`: in both 
> cases `BufferedInputStream.fill()` will be called resulting in load of much 
> bigger amount of data (8192 by default) than required.

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  Fix error message

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2992/files
  - new: https://git.openjdk.java.net/jdk/pull/2992/files/f69c8ff4..ff25cc4d

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

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

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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v4]

[Сергей Цыпанов]

> In some cases wrapping of `InputStream` with `BufferedInputStream` is 
> redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does 
> not require any buffer having one within.
> 
> Other cases are related to reading either a byte or short `byte[]`: in both 
> cases `BufferedInputStream.fill()` will be called resulting in load of much 
> bigger amount of data (8192 by default) than required.

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  Fix formatting

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2992/files
  - new: https://git.openjdk.java.net/jdk/pull/2992/files/a016d2ac..f69c8ff4

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

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

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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v3]

[Сергей Цыпанов]

On Sun, 14 Mar 2021 22:48:18 GMT, Sergey Bylokhov  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Use InputStream.readNBytes() and fix JLinkNegativeTest
>
> src/java.desktop/share/classes/sun/awt/image/ByteArrayImageSource.java line 
> 54:
> 
>> 52: 
>> 53: protected ImageDecoder getDecoder() {
>> 54: InputStream is = new ByteArrayInputStream(imagedata, 
>> imageoffset, imagelength);
> 
> This file use 80 chars per line code style.

Fixed.

-

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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v3]

[Сергей Цыпанов]

> In some cases wrapping of `InputStream` with `BufferedInputStream` is 
> redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does 
> not require any buffer having one within.
> 
> Other cases are related to reading either a byte or short `byte[]`: in both 
> cases `BufferedInputStream.fill()` will be called resulting in load of much 
> bigger amount of data (8192 by default) than required.

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  Use InputStream.readNBytes() and fix JLinkNegativeTest

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2992/files
  - new: https://git.openjdk.java.net/jdk/pull/2992/files/12505d05..a016d2ac

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

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

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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v3]

[Сергей Цыпанов]

On Sun, 14 Mar 2021 17:40:27 GMT, Alan Bateman  wrote:

>> It turned out, that with this latest update some of tier2_tools tests are 
>> failing, e.g. `JLinkNegativeTest`:
>> test JLinkNegativeTest.testMalformedJmod(): failure
>> java.lang.AssertionError: Output does not fit regexp: Error: 
>> java.io.IOException: Invalid JMOD file
>>  at tests.Result.assertFailure(Result.java:70)
>>  at JLinkNegativeTest.testMalformedJmod(JLinkNegativeTest.java:201)
>>  at 
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native 
>> Method)
>>  at 
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
>>  at 
>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>  at java.base/java.lang.reflect.Method.invoke(Method.java:568)
>>  at 
>> org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85)
>>  at org.testng.internal.Invoker.invokeMethod(Invoker.java:639)
>>  at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821)
>>  at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1131)
>>  at 
>> org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:125)
>>  at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108)
>>  at org.testng.TestRunner.privateRun(TestRunner.java:773)
>>  at org.testng.TestRunner.run(TestRunner.java:623)
>>  at org.testng.SuiteRunner.runTest(SuiteRunner.java:357)
>>  at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352)
>>  at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310)
>>  at org.testng.SuiteRunner.run(SuiteRunner.java:259)
>>  at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
>>  at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
>>  at org.testng.TestNG.runSuitesSequentially(TestNG.java:1185)
>>  at org.testng.TestNG.runSuitesLocally(TestNG.java:1110)
>>  at org.testng.TestNG.run(TestNG.java:1018)
>>  at 
>> com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94)
>>  at 
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native 
>> Method)
>>  at 
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
>>  at 
>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>  at java.base/java.lang.reflect.Method.invoke(Method.java:568)
>>  at 
>> com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298)
>>  at java.base/java.lang.Thread.run(Thread.java:831)
>> ...
>> java.lang.module.FindException: java.io.IOException: Invalid JMOD file: 
>> /home/s.tsypanov/IdeaProjects/jdk-github/build/linux-x86_64-server-release/test-support/jtreg_test_jdk_tier2/scratch/3/jmods/hacked4.jmod
>>  at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:253)
>>  at 
>> java.base/jdk.internal.module.ModulePath.scanNextEntry(ModulePath.java:190)
>>  at java.base/jdk.internal.module.ModulePath.find(ModulePath.java:154)
>>  at 
>> java.base/java.lang.module.Resolver.findWithBeforeFinder(Resolver.java:842)
>>  at java.base/java.lang.module.Resolver.resolve(Resolver.java:119)
>>  at 
>> java.base/java.lang.module.Configuration.resolve(Configuration.java:421)
>>  at 
>> java.base/java.lang.module.Configuration.resolve(Configuration.java:255)
>>  at 
>> jdk.jlink/jdk.tools.jlink.internal.JlinkTask.limitFinder(JlinkTask.java:545)
>>  at 
>> jdk.jlink/jdk.tools.jlink.internal.JlinkTask.newModuleFinder(JlinkTask.java:466)
>>  at 
>> jdk.jlink/jdk.tools.jlink.internal.JlinkTask.initJlinkConfig(JlinkTask.java:374)
>>  at jdk.jlink/jdk.tools.jlink.internal.JlinkTask.run(JlinkTask.java:267)
>>  at jdk.jlink/jdk.tools.jlink.internal.Main.run(Main.java:54)
>>  at 
>> jdk.jlink/jdk.tools.jlink.internal.Main$JlinkToolProvider.run(Main.java:65)
>>  at tests.JImageGenerator$JLinkTask.call(JImageGenerator.java:715)
>>  at tests.Helper.generateDefaultImage(Helper.java:257)
>>  at tests.Helper.generateDefaultImage(Helper.java:244)
>>  at JLinkNegativeTest.testSectionsAreFiles(JLinkNegativeTest.java:307)
>>  at 
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native 
>> Method)
>>  at 
>> java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
>>  at 
>> java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>>  at java.base/java.lang.reflect.Method.invoke(Method.java:568)
>>  at 
>> org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85)
>>  at org.testng.internal.Invoker.invokeMethod(Invoker.java:639)
>>  at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821)
>>

Re: RFR: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

On Sun, 14 Mar 2021 17:26:07 GMT, Alan Bateman  wrote:

>> Looks like it's never specified in JavaDoc which particular implementation 
>> of List is used in fields of affected classes, so it's quite odd to me that 
>> someone would rely on that when using reflection. But your point about 
>> backward compatibility is reasonable, so I'll revert mentioned changes.
>
> Nothing outside of the JDK should be hacking into this private field of a 
> non-exposed class, this should not be a concern here.

@AlanBateman so is it ok to keep `ArrayLists`?

-

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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v2]

[Сергей Цыпанов]

On Sun, 14 Mar 2021 11:16:13 GMT, Alan Bateman  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Revert usage of InputStream.readNBytes()
>
> src/java.base/share/classes/jdk/internal/jmod/JmodFile.java line 58:
> 
>> 56: byte[] magic = in.readNBytes(4);
>> 57: if (magic.length != 4) {
>> 58: throw new IOException("Header expected to be of length 
>> 4, but was " + magic.length);
> 
> Thanks for the update. If magic != 4 then it means the file is not a JMOD 
> file or that it has been truncated.

It turned out, that with this latest update some of tier2_tools tests are 
failing, e.g. `JLinkNegativeTest`:
test JLinkNegativeTest.testMalformedJmod(): failure
java.lang.AssertionError: Output does not fit regexp: Error: 
java.io.IOException: Invalid JMOD file
at tests.Result.assertFailure(Result.java:70)
at JLinkNegativeTest.testMalformedJmod(JLinkNegativeTest.java:201)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at 
org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:85)
at org.testng.internal.Invoker.invokeMethod(Invoker.java:639)
at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:821)
at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1131)
at 
org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:125)
at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:108)
at org.testng.TestRunner.privateRun(TestRunner.java:773)
at org.testng.TestRunner.run(TestRunner.java:623)
at org.testng.SuiteRunner.runTest(SuiteRunner.java:357)
at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:352)
at org.testng.SuiteRunner.privateRun(SuiteRunner.java:310)
at org.testng.SuiteRunner.run(SuiteRunner.java:259)
at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:86)
at org.testng.TestNG.runSuitesSequentially(TestNG.java:1185)
at org.testng.TestNG.runSuitesLocally(TestNG.java:1110)
at org.testng.TestNG.run(TestNG.java:1018)
at 
com.sun.javatest.regtest.agent.TestNGRunner.main(TestNGRunner.java:94)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at 
com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298)
at java.base/java.lang.Thread.run(Thread.java:831)
...
java.lang.module.FindException: java.io.IOException: Invalid JMOD file: 
/home/s.tsypanov/IdeaProjects/jdk-github/build/linux-x86_64-server-release/test-support/jtreg_test_jdk_tier2/scratch/3/jmods/hacked4.jmod
at java.base/jdk.internal.module.ModulePath.scan(ModulePath.java:253)
at 
java.base/jdk.internal.module.ModulePath.scanNextEntry(ModulePath.java:190)
at java.base/jdk.internal.module.ModulePath.find(ModulePath.java:154)
at 
java.base/java.lang.module.Resolver.findWithBeforeFinder(Resolver.java:842)
at java.base/java.lang.module.Resolver.resolve(Resolver.java:119)
at 
java.base/java.lang.module.Configuration.resolve(Configuration.java:421)
at 
java.base/java.lang.module.Configuration.resolve(Configuration.java:255)
at 
jdk.jlink/jdk.tools.jlink.internal.JlinkTask.limitFinder(JlinkTask.java:545)
at 
jdk.jlink/jdk.tools.jlink.internal.JlinkTask.newModuleFinder(JlinkTask.java:466)
at 
jdk.jlink/jdk.tools.jlink.internal.JlinkTask.initJlinkConfig(JlinkTask.java:374)
at jdk.jlink/jdk.tools.jlink.internal.JlinkTask.run(JlinkTask.java:267)
at jdk.jlink/jdk.tools.jlink.internal.Main.run(Main.java:54)
at 
jdk.jlink/jdk.tools.jlink.internal.Main$JlinkToolProvider.run(Main.java:65)
at tests.JImageGenerator$JLinkTask.call(JImageGenerator.java:715)
at tests.Helper.generateDefaultImage(Helper.java:257)
at tests.Helper.generateDefaultImage(Helper.java:244)
at JLinkNegativeTest.testSectionsAreFiles(JLinkNegativeTest.java:307)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream [v2]

[Сергей Цыпанов]

> In some cases wrapping of `InputStream` with `BufferedInputStream` is 
> redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does 
> not require any buffer having one within.
> 
> Other cases are related to reading either a byte or short `byte[]`: in both 
> cases `BufferedInputStream.fill()` will be called resulting in load of much 
> bigger amount of data (8192 by default) than required.

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  Revert usage of InputStream.readNBytes()

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2992/files
  - new: https://git.openjdk.java.net/jdk/pull/2992/files/4c608737..12505d05

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

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

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

Re: RFR: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

On Sun, 14 Mar 2021 15:02:03 GMT, liach  
wrote:

>> src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 220:
>> 
>>> 218: return Collections.emptyList();
>>> 219: }
>>> 220: List result = new ArrayList<>();
>> 
>> We'd better be cautious about this replacement since its 
>> [caller](https://github.com/openjdk/jdk/blob/73029fe10a8a814a8c5f5221f2e667fd14a5b379/src/java.base/share/classes/java/net/URLClassLoader.java#L363)
>>  will remove the first element of this array, that's one of the scenarios 
>> where LinkedList usually has better performance than ArrayList.
>> 
>> Just IMHO, I suggest replacing them only if there is a performance 
>> improvement(e.g. benchmark reports). Changing field types will break users' 
>> existing application code, they might reflectively modify these values.
>
> If that's the only use case, I recommend changing the return type to a deque, 
> and replace the linked list with an array deque instead (as done elsewhere in 
> this pr)

Looks like it's never specified in JavaDoc which particular implementation of 
List is used in fields of affected classes, so it's quite odd to me that 
someone would rely on that when using reflection. But your point about backward 
compatibility is reasonable, so I'll revert mentioned changes.

-

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

Re: RFR: 8263561: Re-examine uses of LinkedList

[Сергей Цыпанов]

On Fri, 26 Feb 2021 15:32:57 GMT, liach  
wrote:

> Are linked lists worse for addition even in cases where addition to array 
> list or deque requires resize and copying? i thought that's the advantage of 
> linked list.

As far as I know `LinkedList` is always worse than `ArrayList` and discouraged 
from being used.

-

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

RFR: 8263560: Remove needless wrapping with BufferedInputStream

[Сергей Цыпанов]

In some cases wrapping of `InputStream` with `BufferedInputStream` is 
redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which does 
not require any buffer having one within.

Other cases are related to reading either a byte or short `byte[]`: in both 
cases `BufferedInputStream.fill()` will be called resulting in load of much 
bigger amount of data (8192 by default) than required.

-

Commit messages:
 - Use InputStream.readNBytes()
 - Drop unnecessary BufferedInputStream-wrapping where possible

Changes: https://git.openjdk.java.net/jdk/pull/2992/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2992=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263560
  Stats: 14 lines in 3 files changed: 2 ins; 7 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2992.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2992/head:pull/2992

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

Re: RFR: 8263560: Remove needless wrapping with BufferedInputStream

[Сергей Цыпанов]

On Sun, 14 Mar 2021 07:51:24 GMT, Alan Bateman  wrote:

>> In some cases wrapping of `InputStream` with `BufferedInputStream` is 
>> redundant, e.g. in case the wrapped one is `ByteArrayOutputStream` which 
>> does not require any buffer having one within.
>> 
>> Other cases are related to reading either a byte or short `byte[]`: in both 
>> cases `BufferedInputStream.fill()` will be called resulting in load of much 
>> bigger amount of data (8192 by default) than required.
>
> src/java.base/share/classes/jdk/internal/jmod/JmodFile.java line 57:
> 
>> 55: // validate the header
>> 56: byte[] magic = new byte[4];
>> 57: in.read(magic);
> 
> While you are there, this can be changed to magic = in.readNBytes(4) and 
> throw IOException if magic.length != 4.

Done.

-

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

Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]

[Сергей Цыпанов]

On Wed, 24 Feb 2021 08:50:36 GMT, Alan Bateman  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8261880: Remove static from declarations of Holder nested classes
>
> src/java.base/windows/classes/sun/nio/ch/PipeImpl.java line 67:
> 
>> 65: private final SinkChannel sink;
>> 66: 
>> 67: private static class Initializer
> 
> This one is okay to do.

Thanks for review! Could you review the rest of the code and approve this PR, 
if it's fine?

-

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

Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]

[Сергей Цыпанов]

> Non-static classes hold a link to their parent classes, which in many cases 
> can be avoided.

Сергей Цыпанов has updated the pull request incrementally with one additional 
commit since the last revision:

  8261880: Remove static from declarations of Holder nested classes

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2589/files
  - new: https://git.openjdk.java.net/jdk/pull/2589/files/5650cce4..c6f9cf6b

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

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

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

Re: RFR: 8261880: Change nested classes in java.base to static nested classes where possible [v2]

[Сергей Цыпанов]

On Wed, 17 Feb 2021 16:24:46 GMT, Claes Redestad  wrote:

>> Сергей Цыпанов has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8261880: Remove static from declarations of Holder nested classes
>
> src/java.base/share/classes/java/lang/invoke/DelegatingMethodHandle.java line 
> 192:
> 
>> 190: 
>> 191: /* Placeholder class for DelegatingMethodHandles generated ahead of 
>> time */
>> 192: static final class Holder {}
> 
> For the four `Holder` classes in `java.lang.invoke`, the class is generated 
> when running jlink via `java.lang.invoke.GenerateJLIClassesHelper`. To stay 
> in sync with the definition you'd have to update that code. Also, since these 
> `Holder` classes will only contain static methods and are never instantiated 
> then I'm not sure it matters whether the classes are static or not.

I'll just revert them

-

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

RFR: 8261880: Change nested classes in java.base to static nested classes where possible

[Сергей Цыпанов]

Non-static classes hold a link to their parent classes, which in many cases can 
be avoided.

-

Commit messages:
 - 8261880: Change nested classes in java.base to static nested classes where 
possible

Changes: https://git.openjdk.java.net/jdk/pull/2589/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2589=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261880
  Stats: 20 lines in 16 files changed: 0 ins; 0 del; 20 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2589.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2589/head:pull/2589

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

Integrated: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)

[Сергей Цыпанов]

On Thu, 3 Dec 2020 14:29:58 GMT, Сергей Цыпанов 
 wrote:

> Instead of allocating a copy of underlying array via 
> `CharArrayWriter.toCharArray()` and passing it to constructor of String
> String str = new String(charArrayWriter.toCharArray());
> we could call `toString()` method
> String str = charArrayWriter.toString();
> decoding existing char[] without making a copy. This slightly speeds up the 
> method reducing at the same time memory consumption for decoding URLs with 
> non-latin symbols:
> @State(Scope.Thread)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class UrlEncoderBenchmark {
>   private static final Charset charset = Charset.defaultCharset();
>   private static final String utf8Url = 
> "https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций;; // UN
> 
>   @Benchmark
>   public String encodeUtf8() {
> return URLEncoder.encode(utf8Url, charset);
>   }
> }
> The benchmark on my maching give the following output:
> before
> BenchmarkMode  Cnt
>  ScoreError   Units
> UrlEncoderBenchmark.encodeUtf8   avgt  100  
> 1166.378 ±  8.411   ns/op
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
> 932.944 ±  6.393  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
> 1712.193 ±  0.005B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
> 929.221 ± 24.268  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
> 1705.444 ± 43.235B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100
>  0.006 ±  0.001  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100
>  0.011 ±  0.002B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
> 652.000   counts
> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
> 334.000   ms
> 
> after
> BenchmarkMode  Cnt
>  ScoreError   Units
> UrlEncoderBenchmark.encodeUtf8   avgt  100  
> 1058.851 ±  6.006   ns/op
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
> 931.489 ±  5.182  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
> 1552.176 ±  0.005B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
> 933.491 ± 24.164  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
> 1555.488 ± 39.204B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100
>  0.006 ±  0.001  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100
>  0.010 ±  0.002B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
> 655.000   counts
> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
> 333.000   ms

This pull request has now been integrated.

Changeset: c822eda1
Author:Sergey Tsypanov 
Committer: Claes Redestad 
URL:   https://git.openjdk.java.net/jdk/commit/c822eda1
Stats: 9 lines in 1 file changed: 0 ins; 0 del; 9 mod

8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)

Reviewed-by: attila, redestad, chegar

-

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

Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset) [v3]

[Сергей Цыпанов]

On Wed, 13 Jan 2021 13:48:40 GMT, Claes Redestad  wrote:

>> Сергей Цыпанов has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains five additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'master' into enc
>>  - 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset) - 
>> small JavaDoc fix
>>  - Merge branch 'master' into enc
>>  - Merge branch 'master' into enc
>>  - Improve URLEncoder.encode(String, Charset)
>
> Looks good.
> 
> I wonder... `CharArrayWriter` is an old and synchronized data structure, and 
> since the instance used here isn't shared that synchronization seem useless. 
> And since you're now bypassing the `char[]` and going straight for a `String` 
> you might get better performance with a `StringBuilder` here? (`setLength(0)` 
> instead of `reset()`...)

@cl4es done

-

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

Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset) [v3]

[Сергей Цыпанов]

> Instead of allocating a copy of underlying array via 
> `CharArrayWriter.toCharArray()` and passing it to constructor of String
> String str = new String(charArrayWriter.toCharArray());
> we could call `toString()` method
> String str = charArrayWriter.toString();
> decoding existing char[] without making a copy. This slightly speeds up the 
> method reducing at the same time memory consumption for decoding URLs with 
> non-latin symbols:
> @State(Scope.Thread)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class UrlEncoderBenchmark {
>   private static final Charset charset = Charset.defaultCharset();
>   private static final String utf8Url = 
> "https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций;; // UN
> 
>   @Benchmark
>   public String encodeUtf8() {
> return URLEncoder.encode(utf8Url, charset);
>   }
> }
> The benchmark on my maching give the following output:
> before
> BenchmarkMode  Cnt
>  ScoreError   Units
> UrlEncoderBenchmark.encodeUtf8   avgt  100  
> 1166.378 ±  8.411   ns/op
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
> 932.944 ±  6.393  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
> 1712.193 ±  0.005B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
> 929.221 ± 24.268  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
> 1705.444 ± 43.235B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100
>  0.006 ±  0.001  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100
>  0.011 ±  0.002B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
> 652.000   counts
> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
> 334.000   ms
> 
> after
> BenchmarkMode  Cnt
>  ScoreError   Units
> UrlEncoderBenchmark.encodeUtf8   avgt  100  
> 1058.851 ±  6.006   ns/op
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
> 931.489 ±  5.182  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
> 1552.176 ±  0.005B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
> 933.491 ± 24.164  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
> 1555.488 ± 39.204B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100
>  0.006 ±  0.001  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100
>  0.010 ±  0.002B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
> 655.000   counts
> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
> 333.000   ms

Сергей Цыпанов has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains five additional 
commits since the last revision:

 - Merge branch 'master' into enc
 - 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset) - small 
JavaDoc fix
 - Merge branch 'master' into enc
 - Merge branch 'master' into enc
 - Improve URLEncoder.encode(String, Charset)

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1598/files
  - new: https://git.openjdk.java.net/jdk/pull/1598/files/ae293c8b..2183af4c

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

  Stats: 3325 lines in 139 files changed: 1985 ins; 870 del; 470 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1598.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1598/head:pull/1598

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

Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset) [v2]

[Сергей Цыпанов]

> Instead of allocating a copy of underlying array via 
> `CharArrayWriter.toCharArray()` and passing it to constructor of String
> String str = new String(charArrayWriter.toCharArray());
> we could call `toString()` method
> String str = charArrayWriter.toString();
> decoding existing char[] without making a copy. This slightly speeds up the 
> method reducing at the same time memory consumption for decoding URLs with 
> non-latin symbols:
> @State(Scope.Thread)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
> public class UrlEncoderBenchmark {
>   private static final Charset charset = Charset.defaultCharset();
>   private static final String utf8Url = 
> "https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций;; // UN
> 
>   @Benchmark
>   public String encodeUtf8() {
> return URLEncoder.encode(utf8Url, charset);
>   }
> }
> The benchmark on my maching give the following output:
> before
> BenchmarkMode  Cnt
>  ScoreError   Units
> UrlEncoderBenchmark.encodeUtf8   avgt  100  
> 1166.378 ±  8.411   ns/op
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
> 932.944 ±  6.393  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
> 1712.193 ±  0.005B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
> 929.221 ± 24.268  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
> 1705.444 ± 43.235B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100
>  0.006 ±  0.001  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100
>  0.011 ±  0.002B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
> 652.000   counts
> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
> 334.000   ms
> 
> after
> BenchmarkMode  Cnt
>  ScoreError   Units
> UrlEncoderBenchmark.encodeUtf8   avgt  100  
> 1058.851 ±  6.006   ns/op
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
> 931.489 ±  5.182  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
> 1552.176 ±  0.005B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
> 933.491 ± 24.164  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
> 1555.488 ± 39.204B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100
>  0.006 ±  0.001  MB/sec
> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100
>  0.010 ±  0.002B/op
> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
> 655.000   counts
> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
> 333.000   ms

Сергей Цыпанов has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains four additional 
commits since the last revision:

 - 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset) - small 
JavaDoc fix
 - Merge branch 'master' into enc
 - Merge branch 'master' into enc
 - Improve URLEncoder.encode(String, Charset)

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/1598/files
  - new: https://git.openjdk.java.net/jdk/pull/1598/files/2856c923..ae293c8b

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

  Stats: 31439 lines in 1018 files changed: 11701 ins; 8302 del; 11436 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1598.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1598/head:pull/1598

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

Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)

[Сергей Цыпанов]

On Wed, 13 Jan 2021 13:48:40 GMT, Claes Redestad  wrote:

>> Instead of allocating a copy of underlying array via 
>> `CharArrayWriter.toCharArray()` and passing it to constructor of String
>> String str = new String(charArrayWriter.toCharArray());
>> we could call `toString()` method
>> String str = charArrayWriter.toString();
>> decoding existing char[] without making a copy. This slightly speeds up the 
>> method reducing at the same time memory consumption for decoding URLs with 
>> non-latin symbols:
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class UrlEncoderBenchmark {
>>   private static final Charset charset = Charset.defaultCharset();
>>   private static final String utf8Url = 
>> "https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций;; // UN
>> 
>>   @Benchmark
>>   public String encodeUtf8() {
>> return URLEncoder.encode(utf8Url, charset);
>>   }
>> }
>> The benchmark on my maching give the following output:
>> before
>> BenchmarkMode  Cnt   
>>   ScoreError   Units
>> UrlEncoderBenchmark.encodeUtf8   avgt  100  
>> 1166.378 ±  8.411   ns/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
>> 932.944 ±  6.393  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
>> 1712.193 ±  0.005B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
>> 929.221 ± 24.268  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
>> 1705.444 ± 43.235B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100   
>>   0.006 ±  0.001  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100   
>>   0.011 ±  0.002B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
>> 652.000   counts
>> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
>> 334.000   ms
>> 
>> after
>> BenchmarkMode  Cnt   
>>   ScoreError   Units
>> UrlEncoderBenchmark.encodeUtf8   avgt  100  
>> 1058.851 ±  6.006   ns/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
>> 931.489 ±  5.182  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
>> 1552.176 ±  0.005B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
>> 933.491 ± 24.164  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
>> 1555.488 ± 39.204B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100   
>>   0.006 ±  0.001  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100   
>>   0.010 ±  0.002B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
>> 655.000   counts
>> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
>> 333.000   ms
>
> Looks good.
> 
> I wonder... `CharArrayWriter` is an old and synchronized data structure, and 
> since the instance used here isn't shared that synchronization seem useless. 
> And since you're now bypassing the `char[]` and going straight for a `String` 
> you might get better performance with a `StringBuilder` here? (`setLength(0)` 
> instead of `reset()`...)

@cl4es SB brings pessimization both for time and memory, try 
`org.openjdk.bench.java.net.URLEncodeDecode`:
master
 (count)  (maxLength)  (mySeed) 
 Mode  Cnt Score   Error   Units
testEncodeUTF8  1024 1024 3 
 avgt   25 8.573 ? 0.023   ms/op
testEncodeUTF8:?gc.alloc.rate   1024 1024 3 
 avgt   25  1202.896 ? 3.225  MB/sec
testEncodeUTF8:?gc.alloc.rate.norm  1024 1024 3 
 avgt   25  11355727.904 ?   196.249B/op
testEncodeUTF8:?gc.churn.G1_Eden_Space  1024 1024 3 
 avgt   25  1203.785 ? 6.240  MB/sec
testEncodeUTF8:?gc.churn.G1_Eden_Space.norm 1024 1024 3 
 avgt   25  11364143.637 ? 52830.222B/op
testEncodeUTF8:?gc.churn.G1_Survivor_Space  1024 1024 3 
 avgt   25 0.008 ? 0.001  MB/sec
testEncodeUTF8:?gc.churn.G1_Survivor_Space.norm 1024 1024 3 
 avgt   2577.088 ? 9.303B/op
testEncodeUTF8:?gc.count1024 1024 3 
 avgt   25  1973.000  counts
testEncodeUTF8:?gc.time 1024 1024 3 
 avgt   25   996.000  

Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)

[Сергей Цыпанов]

On Wed, 13 Jan 2021 13:48:40 GMT, Claes Redestad  wrote:

>> Instead of allocating a copy of underlying array via 
>> `CharArrayWriter.toCharArray()` and passing it to constructor of String
>> String str = new String(charArrayWriter.toCharArray());
>> we could call `toString()` method
>> String str = charArrayWriter.toString();
>> decoding existing char[] without making a copy. This slightly speeds up the 
>> method reducing at the same time memory consumption for decoding URLs with 
>> non-latin symbols:
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class UrlEncoderBenchmark {
>>   private static final Charset charset = Charset.defaultCharset();
>>   private static final String utf8Url = 
>> "https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций;; // UN
>> 
>>   @Benchmark
>>   public String encodeUtf8() {
>> return URLEncoder.encode(utf8Url, charset);
>>   }
>> }
>> The benchmark on my maching give the following output:
>> before
>> BenchmarkMode  Cnt   
>>   ScoreError   Units
>> UrlEncoderBenchmark.encodeUtf8   avgt  100  
>> 1166.378 ±  8.411   ns/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
>> 932.944 ±  6.393  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
>> 1712.193 ±  0.005B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
>> 929.221 ± 24.268  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
>> 1705.444 ± 43.235B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100   
>>   0.006 ±  0.001  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100   
>>   0.011 ±  0.002B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
>> 652.000   counts
>> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
>> 334.000   ms
>> 
>> after
>> BenchmarkMode  Cnt   
>>   ScoreError   Units
>> UrlEncoderBenchmark.encodeUtf8   avgt  100  
>> 1058.851 ±  6.006   ns/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
>> 931.489 ±  5.182  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
>> 1552.176 ±  0.005B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
>> 933.491 ± 24.164  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
>> 1555.488 ± 39.204B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100   
>>   0.006 ±  0.001  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100   
>>   0.010 ±  0.002B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
>> 655.000   counts
>> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
>> 333.000   ms
>
> Looks good.
> 
> I wonder... `CharArrayWriter` is an old and synchronized data structure, and 
> since the instance used here isn't shared that synchronization seem useless. 
> And since you're now bypassing the `char[]` and going straight for a `String` 
> you might get better performance with a `StringBuilder` here? (`setLength(0)` 
> instead of `reset()`...)

@cl4es hi, let me try `StringBuilder`

-

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

Re: RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)

[Сергей Цыпанов]

On Sun, 10 Jan 2021 20:13:51 GMT, Attila Szegedi  wrote:

>> Instead of allocating a copy of underlying array via 
>> `CharArrayWriter.toCharArray()` and passing it to constructor of String
>> String str = new String(charArrayWriter.toCharArray());
>> we could call `toString()` method
>> String str = charArrayWriter.toString();
>> decoding existing char[] without making a copy. This slightly speeds up the 
>> method reducing at the same time memory consumption for decoding URLs with 
>> non-latin symbols:
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class UrlEncoderBenchmark {
>>   private static final Charset charset = Charset.defaultCharset();
>>   private static final String utf8Url = 
>> "https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций;; // UN
>> 
>>   @Benchmark
>>   public String encodeUtf8() {
>> return URLEncoder.encode(utf8Url, charset);
>>   }
>> }
>> The benchmark on my maching give the following output:
>> before
>> BenchmarkMode  Cnt   
>>   ScoreError   Units
>> UrlEncoderBenchmark.encodeUtf8   avgt  100  
>> 1166.378 ±  8.411   ns/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
>> 932.944 ±  6.393  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
>> 1712.193 ±  0.005B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
>> 929.221 ± 24.268  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
>> 1705.444 ± 43.235B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100   
>>   0.006 ±  0.001  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100   
>>   0.011 ±  0.002B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
>> 652.000   counts
>> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
>> 334.000   ms
>> 
>> after
>> BenchmarkMode  Cnt   
>>   ScoreError   Units
>> UrlEncoderBenchmark.encodeUtf8   avgt  100  
>> 1058.851 ±  6.006   ns/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
>> 931.489 ±  5.182  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
>> 1552.176 ±  0.005B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
>> 933.491 ± 24.164  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
>> 1555.488 ± 39.204B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100   
>>   0.006 ±  0.001  MB/sec
>> UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100   
>>   0.010 ±  0.002B/op
>> UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
>> 655.000   counts
>> UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
>> 333.000   ms
>
> Looks good!

@szegedi could you please create a ticket for this change? Otherwise I cannot 
merge it

-

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

RFR: 8259699: Reduce char[] copying in URLEncoder.encode(String, Charset)

[Сергей Цыпанов]

Instead of allocating a copy of underlying array via 
`CharArrayWriter.toCharArray()` and passing it to constructor of String
String str = new String(charArrayWriter.toCharArray());
we could call `toString()` method
String str = charArrayWriter.toString();
decoding existing char[] without making a copy. This slightly speeds up the 
method reducing at the same time memory consumption for decoding URLs with 
non-latin symbols:
@State(Scope.Thread)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
public class UrlEncoderBenchmark {
  private static final Charset charset = Charset.defaultCharset();
  private static final String utf8Url = 
"https://ru.wikipedia.org/wiki/Организация_Объединённых_Наций;; // UN

  @Benchmark
  public String encodeUtf8() {
return URLEncoder.encode(utf8Url, charset);
  }
}
The benchmark on my maching give the following output:
before
BenchmarkMode  Cnt 
ScoreError   Units
UrlEncoderBenchmark.encodeUtf8   avgt  100  
1166.378 ±  8.411   ns/op
UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
932.944 ±  6.393  MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
1712.193 ±  0.005B/op
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
929.221 ± 24.268  MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
1705.444 ± 43.235B/op
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100 
0.006 ±  0.001  MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100 
0.011 ±  0.002B/op
UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
652.000   counts
UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
334.000   ms

after
BenchmarkMode  Cnt 
ScoreError   Units
UrlEncoderBenchmark.encodeUtf8   avgt  100  
1058.851 ±  6.006   ns/op
UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rateavgt  100   
931.489 ±  5.182  MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.alloc.rate.norm   avgt  100  
1552.176 ±  0.005B/op
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space   avgt  100   
933.491 ± 24.164  MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Eden_Space.norm  avgt  100  
1555.488 ± 39.204B/op
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space   avgt  100 
0.006 ±  0.001  MB/sec
UrlEncoderBenchmark.encodeUtf8:·gc.churn.G1_Survivor_Space.norm  avgt  100 
0.010 ±  0.002B/op
UrlEncoderBenchmark.encodeUtf8:·gc.count avgt  100   
655.000   counts
UrlEncoderBenchmark.encodeUtf8:·gc.time  avgt  100   
333.000   ms

-

Commit messages:
 - Merge branch 'master' into enc
 - Improve URLEncoder.encode(String, Charset)

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

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

Re: RFR: 8080272 Refactor I/O stream copying to use java.io.InputStream.transferTo [v8]

[Сергей Цыпанов]

On Mon, 21 Dec 2020 09:51:25 GMT, Andrey Turbanov 
 wrote:

>> src/java.base/share/classes/sun/security/tools/keytool/Main.java line 2459:
>> 
>>> 2457: byte[] bytes = in.readAllBytes();
>>> 2458: return 
>>> CertificateFactory.getInstance("X509").generateCRLs(
>>> 2459: new ByteArrayInputStream(bytes));
>> 
>> Let's just pass `in` into `generateCRLs` instead of reading all bytes and 
>> rewrapping them into `InputStream` again?
>
> Looks like it was done intentionally by original author of the code.
> Check comment above:
> 
> // Read the full stream before feeding to X509Factory,
> // otherwise, keytool -gencrl | keytool -printcrl
> // might not work properly, since -gencrl is slow
> // and there's no data in the pipe at the beginning.

Let's keep it then

-

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

Re: RFR: 8080272 Refactor I/O stream copying to use java.io.InputStream.transferTo [v8]

[Сергей Цыпанов]

On Mon, 21 Dec 2020 09:16:11 GMT, Andrey Turbanov 
 wrote:

>> 8080272  Refactor I/O stream copying to use java.io.InputStream.transferTo
>
> Andrey Turbanov has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   8080272: Refactor I/O stream copying to use java.io.InputStream.transferTo
>   revert changes in Apache Santuario

I'm not a reviewer, but still think we could simplify 
`sun.security.tools.keytool.Main`

src/java.base/share/classes/sun/security/tools/keytool/Main.java line 2459:

> 2457: byte[] bytes = in.readAllBytes();
> 2458: return 
> CertificateFactory.getInstance("X509").generateCRLs(
> 2459: new ByteArrayInputStream(bytes));

Let's just pass `in` into `generateCRLs` instead of reading all bytes and 
rewrapping them into `InputStream` again?

-

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

Integrated: 8255477: Remove unused method URL.set(String protocol, String host, int port, String file, String ref)

[Сергей Цыпанов]

On Wed, 21 Oct 2020 06:52:42 GMT, Сергей Цыпанов 
 wrote:

> Hello, while working with `java.net.URL` I've found unused package-private 
> method `URL.set(String protocol, String host, int port, String file, String 
> ref)` which can be safely removed from JDK. Testing: tier1, tier2
> 
> Could someone crate an issue for tracking of this simple change?

This pull request has now been integrated.

Changeset: 15481041
Author:Sergey Tsypanov 
Committer: Daniel Fuchs 
URL:   https://git.openjdk.java.net/jdk/commit/15481041
Stats: 33 lines in 1 file changed: 0 ins; 33 del; 0 mod

8255477: Remove unused method URL.set(String protocol, String host, int port, 
String file, String ref)

Reviewed-by: dfuchs

-

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

Re: RFR: 8255477: Remove unused method URL.set(String protocol, String host, int port, String file, String ref)

[Сергей Цыпанов]

On Wed, 25 Nov 2020 13:31:06 GMT, Daniel Fuchs  wrote:

>> Done!
>
> I'll have a look at this when I can spare some cycles. @stsypanov did you run 
> tier2 tests?

@dfuch 
Recent run has some errors:
java/net/MulticastSocket/SetOutgoingIf.java: Re-test IPv6 (and specifically 
MulticastSocket) with latest Linux & USAGI code
java/net/MulticastSocket/Test.java: IPv4 and IPv6 multicasting broken on Linux
java/time/test/java/time/format/TestDateTimeFormatterBuilder.java:
sun/security/pkcs11/Secmod/AddTrustedCert.java: make sure we can add a trusted 
cert to the NSS KeyStore module
sun/security/util/HostnameMatcher/NullHostnameCheck.java: Verify hostname 
returns an exception instead of null pointer when creating a new engine
First two are related to unavailable network, for them I have `Unexpected 
exception for MulticastSender(wlp1s0): java.net.SocketException: Network is 
unreachable` and `java.net.SocketException: Network is unreachable`

For the third I have
test 
test.java.time.format.TestDateTimeFormatterBuilder.test_dayPeriodParse(NARROW, 
en_US, 1, 30, "at night"): failure
java.time.format.DateTimeParseException: Text 'at night' could not be parsed, 
unparsed text found at index 1
at 
java.base/java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:2055)
at 
java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:1880)
at 
test.java.time.format.TestDateTimeFormatterBuilder.test_dayPeriodParse(TestDateTimeFormatterBuilder.java:645)
Which also seems to be not related to `URL`.
For the forth I have
--System.err:(21/1603)--
java.security.KeyStoreException: sun.security.pkcs11.wrapper.PKCS11Exception: 
CKR_ATTRIBUTE_READ_ONLY
at 
jdk.crypto.cryptoki/sun.security.pkcs11.P11KeyStore.engineSetEntry(P11KeyStore.java:1050)
at 
jdk.crypto.cryptoki/sun.security.pkcs11.P11KeyStore.engineSetCertificateEntry(P11KeyStore.java:516)
at 
java.base/java.security.KeyStore.setCertificateEntry(KeyStore.java:1228)
at AddTrustedCert.main(AddTrustedCert.java:106)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:564)
at 
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
at java.base/java.lang.Thread.run(Thread.java:831)
Caused by: sun.security.pkcs11.wrapper.PKCS11Exception: CKR_ATTRIBUTE_READ_ONLY
at 
jdk.crypto.cryptoki/sun.security.pkcs11.wrapper.PKCS11.C_CreateObject(Native 
Method)
at 
jdk.crypto.cryptoki/sun.security.pkcs11.P11KeyStore.storeCert(P11KeyStore.java:1568)
at 
jdk.crypto.cryptoki/sun.security.pkcs11.P11KeyStore.engineSetEntry(P11KeyStore.java:1046)
... 9 more
And the last one is
javax.net.ssl.SSLHandshakeException: No appropriate protocol (protocol is 
disabled or cipher suites are inappropriate)
at 
java.base/sun.security.ssl.HandshakeContext.(HandshakeContext.java:172)
at 
java.base/sun.security.ssl.ClientHandshakeContext.(ClientHandshakeContext.java:98)
at 
java.base/sun.security.ssl.TransportContext.kickstart(TransportContext.java:238)
at 
java.base/sun.security.ssl.SSLEngineImpl.beginHandshake(SSLEngineImpl.java:107)
at NullHostnameCheck.handshake(NullHostnameCheck.java:124)
at NullHostnameCheck.main(NullHostnameCheck.java:100)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:564)
at 
com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:298)
at java.base/java.lang.Thread.run(Thread.java:831)

-

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

Re: RFR: 8255477: Remove unused method URL.set(String protocol, String host, int port, String file, String ref) [v2]

[Сергей Цыпанов]

> Hello, while working with `java.net.URL` I've found unused package-private 
> method `URL.set(String protocol, String host, int port, String file, String 
> ref)` which can be safely removed from JDK. Testing: tier1, tier2
> 
> Could someone crate an issue for tracking of this simple change?

Сергей Цыпанов has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains two additional 
commits since the last revision:

 - Merge branch 'master' into url
 - 8255477: Remove unused method URL.set(String protocol, String host, int 
port, String file, String ref)

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/779/files
  - new: https://git.openjdk.java.net/jdk/pull/779/files/e2487aae..24d14e26

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

  Stats: 143793 lines in 2196 files changed: 79159 ins; 49141 del; 15493 mod
  Patch: https://git.openjdk.java.net/jdk/pull/779.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/779/head:pull/779

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

Re: RFR: 8255477: Remove unused method URL.set(String protocol, String host, int port, String file, String ref)

[Сергей Цыпанов]

On Wed, 25 Nov 2020 08:02:07 GMT, Aleksey Shipilev  wrote:

>> @shipilev thanks for filing the issue! Done.
>
> Please merge from master to get a fresh round of testing. @AlanBateman might 
> want to look at it.

Done!

-

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

Integrated: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Сергей Цыпанов]

On Thu, 22 Oct 2020 20:46:15 GMT, Сергей Цыпанов 
 wrote:

> As discussed in https://github.com/openjdk/jdk/pull/510 there is never a 
> reason to explicitly instantiate any instance of `Atomic*` class with its 
> default value, i.e. `new AtomicInteger(0)` could be replaced with `new 
> AtomicInteger()` which is faster:
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @BenchmarkMode(value = Mode.AverageTime)
> public class AtomicBenchmark {
>   @Benchmark
>   public Object defaultValue() {
> return new AtomicInteger();
>   }
>   @Benchmark
>   public Object explicitValue() {
> return new AtomicInteger(0);
>   }
> }
> THis benchmark demonstrates that `explicitValue()` is much slower:
> Benchmark  Mode  Cnt   Score   Error  Units
> AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
> AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
> So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
> progress we could trivially replace explicit zeroing with default 
> constructors gaining some performance benefit with no risk.
> 
> I've tested the changes locally, both tier1 and tier 2 are ok. 
> 
> Could one create an issue for tracking this?

This pull request has now been integrated.

Changeset: 3c4fc793
Author:Sergey Tsypanov 
Committer: Daniel Fuchs 
URL:   https://git.openjdk.java.net/jdk/commit/3c4fc793
Stats: 19 lines in 17 files changed: 0 ins; 3 del; 16 mod

8255299: Drop explicit zeroing at instantiation of Atomic* objects

Reviewed-by: redestad, serb, prr

-

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

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects [v2]

[Сергей Цыпанов]

On Wed, 28 Oct 2020 08:49:38 GMT, Sergey Bylokhov  wrote:

>> Rebased onto master to have the fix introduced in 
>> https://github.com/openjdk/jdk/pull/778
>
> FYI it is better to use merge, instead of rebase+force push. Rebase breaks 
> history and all existed code comments.

@mrserb thanks for pointing this out!

-

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

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects [v2]

[Сергей Цыпанов]

> As discussed in https://github.com/openjdk/jdk/pull/510 there is never a 
> reason to explicitly instantiate any instance of `Atomic*` class with its 
> default value, i.e. `new AtomicInteger(0)` could be replaced with `new 
> AtomicInteger()` which is faster:
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @BenchmarkMode(value = Mode.AverageTime)
> public class AtomicBenchmark {
>   @Benchmark
>   public Object defaultValue() {
> return new AtomicInteger();
>   }
>   @Benchmark
>   public Object explicitValue() {
> return new AtomicInteger(0);
>   }
> }
> THis benchmark demonstrates that `explicitValue()` is much slower:
> Benchmark  Mode  Cnt   Score   Error  Units
> AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
> AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
> So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
> progress we could trivially replace explicit zeroing with default 
> constructors gaining some performance benefit with no risk.
> 
> I've tested the changes locally, both tier1 and tier 2 are ok. 
> 
> Could one create an issue for tracking this?

Сергей Цыпанов has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains one additional commit 
since the last revision:

  8255299: Drop explicit zeroing at instantiation of Atomic* objects

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/818/files
  - new: https://git.openjdk.java.net/jdk/pull/818/files/c1fb362f..7dc646d0

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

  Stats: 4576 lines in 201 files changed: 2659 ins; 1135 del; 782 mod
  Patch: https://git.openjdk.java.net/jdk/pull/818.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/818/head:pull/818

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

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects [v2]

[Сергей Цыпанов]

On Sat, 24 Oct 2020 23:12:09 GMT, Phil Race  wrote:

>> Сергей Цыпанов has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains one additional 
>> commit since the last revision:
>> 
>>   8255299: Drop explicit zeroing at instantiation of Atomic* objects
>
> client changes are fine

Rebased onto master to have the fix introduced in 
https://github.com/openjdk/jdk/pull/778

-

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

Re: RFR: 8255477: Remove unused method URL.set(String protocol, String host, int port, String file, String ref)

[Сергей Цыпанов]

On Tue, 27 Oct 2020 19:26:04 GMT, Aleksey Shipilev  wrote:

>> Hello, while working with `java.net.URL` I've found unused package-private 
>> method `URL.set(String protocol, String host, int port, String file, String 
>> ref)` which can be safely removed from JDK. Testing: tier1, tier2
>> 
>> Could someone crate an issue for tracking of this simple change?
>
> Submitted [here](https://bugs.openjdk.java.net/browse/JDK-8255477), please 
> try to update the PR title to "8255477: Remove unused method URL.set(String 
> protocol, String host, int port, String file, String ref)". Also, merge from 
> master to get the test fixes, which should make the "Testing" all green.

@shipilev thanks for filing the issue! Done.

-

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

RFR: 8255477: Remove unused method URL.set(String protocol, String host, int port, String file, String ref)

[Сергей Цыпанов]

Hello, while working with `java.net.URL` I've found unused package-private 
method `URL.set(String protocol, String host, int port, String file, String 
ref)` which can be safely removed from JDK. Testing: tier1, tier2

Could someone crate an issue for tracking of this simple change?

-

Commit messages:
 - 8255477: Remove unused method URL.set(String protocol, String host, int 
port, String file, String ref)

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

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

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Сергей Цыпанов]

On Fri, 23 Oct 2020 09:12:15 GMT, Daniel Fuchs  wrote:

>> As discussed in https://github.com/openjdk/jdk/pull/510 there is never a 
>> reason to explicitly instantiate any instance of `Atomic*` class with its 
>> default value, i.e. `new AtomicInteger(0)` could be replaced with `new 
>> AtomicInteger()` which is faster:
>> @State(Scope.Thread)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @BenchmarkMode(value = Mode.AverageTime)
>> public class AtomicBenchmark {
>>   @Benchmark
>>   public Object defaultValue() {
>> return new AtomicInteger();
>>   }
>>   @Benchmark
>>   public Object explicitValue() {
>> return new AtomicInteger(0);
>>   }
>> }
>> THis benchmark demonstrates that `explicitValue()` is much slower:
>> Benchmark  Mode  Cnt   Score   Error  Units
>> AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
>> AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
>> So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
>> progress we could trivially replace explicit zeroing with default 
>> constructors gaining some performance benefit with no risk.
>> 
>> I've tested the changes locally, both tier1 and tier 2 are ok. 
>> 
>> Could one create an issue for tracking this?
>
> src/java.base/share/classes/sun/net/ResourceManager.java line 65:
> 
>> 63: } catch (NumberFormatException e) {}
>> 64: maxSockets = defmax;
>> 65: numSockets = new AtomicInteger();
> 
> Changes in sun/net look good to me.

@dfuch Could you then sponsor this PR?

-

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

RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Сергей Цыпанов]

As discussed in https://github.com/openjdk/jdk/pull/510 there is never a reason 
to explicitly instantiate any instance of `Atomic*` class with its default 
value, i.e. `new AtomicInteger(0)` could be replaced with `new AtomicInteger()` 
which is faster:
@State(Scope.Thread)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@BenchmarkMode(value = Mode.AverageTime)
public class AtomicBenchmark {
  @Benchmark
  public Object defaultValue() {
return new AtomicInteger();
  }
  @Benchmark
  public Object explicitValue() {
return new AtomicInteger(0);
  }
}
THis benchmark demonstrates that `explicitValue()` is much slower:
Benchmark  Mode  Cnt   Score   Error  Units
AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
progress we could trivially replace explicit zeroing with default constructors 
gaining some performance benefit with no risk.

I've tested the changes locally, both tier1 and tier 2 are ok. 

Could one create an issue for tracking this?

-

Commit messages:
 - 8255299: Drop explicit zeroing at instantiation of Atomic* objects

Changes: https://git.openjdk.java.net/jdk/pull/818/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=818=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8255299
  Stats: 19 lines in 17 files changed: 0 ins; 3 del; 16 mod
  Patch: https://git.openjdk.java.net/jdk/pull/818.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/818/head:pull/818

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