date:20220217

> This is an early review of changes to better model JVM access flags, that is 
> "modifiers" like public, protected, etc. but explicitly at a VM level.
> 
> Language level modifiers and JVM level access flags are closely related, but 
> distinct. There are concepts that overlap in the two domains (public, 
> private, etc.), others that only have a language-level modifier (sealed), and 
> still others that only have an access flag (synthetic).
> 
> The existing java.lang.reflect.Modifier class is inadequate to model these 
> subtleties. For example, the bit positions used by access flags on different 
> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
> methods. Just having a raw integer does not provide sufficient context to 
> decode the corresponding language-level string. Methods like 
> Modifier.methodModifiers() were introduced to cope with this situation.
> 
> With additional modifiers and flags on the horizon with projects like 
> Valhalla, addressing the existent modeling deficiency now ahead of time is 
> reasonable before further strain is introduced.
> 
> This PR in its current form is meant to give the overall shape of the API. It 
> is missing implementations to map from, say, method modifiers to access 
> flags, taking into account overlaps in bit positions.
> 
> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
> once the API is further along.

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

  Switch to location enum.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7445/files
  - new: https://git.openjdk.java.net/jdk/pull/7445/files/131010f3..c3f6b0a4

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

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

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

Re: RFR: 8275731: CDS archived enums objects are recreated at runtime [v4]

2022-02-17 Thread Calvin Cheung

On Wed, 19 Jan 2022 05:47:57 GMT, Ioi Lam  wrote:

>> **Background:**
>> 
>> In the Java Language, Enums can be tested for equality, so the constants in 
>> an Enum type must be unique. Javac compiles an enum declaration like this:
>> 
>> 
>> public enum Day {  SUNDAY, MONDAY ... } 
>> 
>> 
>> to
>> 
>> 
>> public class Day extends java.lang.Enum {
>> public static final SUNDAY = new Day("SUNDAY");
>> public static final MONDAY = new Day("MONDAY"); ...
>> }
>> 
>> 
>> With CDS archived heap objects, `Day::` is executed twice: once 
>> during `java -Xshare:dump`, and once during normal JVM execution. If the 
>> archived heap objects references one of the Enum constants created at dump 
>> time, we will violate the uniqueness requirements of the Enum constants at 
>> runtime. See the test case in the description of 
>> [JDK-8275731](https://bugs.openjdk.java.net/browse/JDK-8275731)
>> 
>> **Fix:**
>> 
>> During -Xshare:dump, if we discovered that an Enum constant of type X is 
>> archived, we archive all constants of type X. At Runtime, type X will skip 
>> the normal execution of `X::`. Instead, we run 
>> `HeapShared::initialize_enum_klass()` to retrieve all the constants of X 
>> that were saved at dump time.
>> 
>> This is safe as we know that `X::` has no observable side effect -- 
>> it only creates the constants of type X, as well as the synthetic value 
>> `X::$VALUES`, which cannot be observed until X is fully initialized.
>> 
>> **Verification:**
>> 
>> To avoid future problems, I added a new tool, CDSHeapVerifier, to look for 
>> similar problems where the archived heap objects reference a static field 
>> that may be recreated at runtime. There are some manual steps involved, but 
>> I analyzed the potential problems found by the tool are they are all safe 
>> (after the current bug is fixed). See cdsHeapVerifier.cpp for gory details. 
>> An example trace of this tool can be found at 
>> https://bugs.openjdk.java.net/secure/attachment/97242/enum_warning.txt
>> 
>> **Testing:**
>> 
>> Passed Oracle CI tiers 1-4. WIll run tier 5 as well.
>
> Ioi Lam has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   Use InstanceKlass::do_local_static_fields for some field iterations

Looks good. Minor comment below.
Also, several files with copyright year 2021 need updating.

src/hotspot/share/cds/cdsHeapVerifier.cpp line 63:

> 61: // class Bar {
> 62: // // this field is initialized in both CDS dump time and runtime.
> 63: // static final Bar bar = new Bar;

`new Bar` should be `new Bar()`?

-

Marked as reviewed by ccheung (Reviewer).

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

RFR: 8282042: [testbug] FileEncodingTest.java depends on default encoding

2022-02-17 Thread Tyler Steele

FileEncodingTest expects all non-Windows platforms will have 
`Charset.defaultCharset().name()` set to US-ASCII when file.encoding is set to 
COMPAT. This assumption does not hold for AIX where it is ISO-8859-1.

According to [JEP-400](https://openjdk.java.net/jeps/400), we should expect  
`Charset.defaultCharset().name()` to equal 
`System.getProperty("native.encoding")` whenever the COMPAT flag is set. From 
JEP-400: "... if file.encoding is set to COMPAT on the command line, then the 
run-time value of file.encoding will be the same as the run-time value of 
native.encoding...". So one way to resolve this is to choose the value for each 
system from the native.encoding property.

With these changes however, my test systems continue to fail. 

- AIX reports: Default Charset: ISO-8859-1, expected: ISO8859-1
- Linux/Z reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968
- Linux/PowerLE reports: Default Charset: US-ASCII, expected: ANSI_X3.4-1968

Note that the expected value is populated from native.encoding.

This implies more work to be done. It looks to me that some modification to 
java_props_md.c may be needed to ensure that the System properties for 
native.encoding return [canonical 
names](http://www.iana.org/assignments/character-sets). 

---

A tempting alternative is to set the expected value for AIX to "ISO-8859-1" in 
the test explicitly, as was done for the Windows expected encoding prior to 
this proposed change. The main advantage to this alternative is that it is 
quick and easy, but the disadvantages are that it fails to test that COMPAT 
behaves as specified in JEP-400, and the approach does not scale well if it 
happens that other systems require other cases. I wonder if this is the reason 
non-English locals are excluded by the test.

Proceeding with this change and the work implied by the new failures it 
highlights goes beyond the scope of what I thought was a simple testbug. So I'm 
opening this up for some comments before proceeding down the rabbit hole of 
further changes. If there is generally positive support for this direction I'm 
happy to make the modifications necessary to populate native.encoding with 
canonical names. As I am new to OpenJDK, I am especially looking to ensure that 
changing the value returned by native.encoding will not have unintended 
consequences elsewhere in the project.

-

Commit messages:
 - Changes FileEncodingTest test to delegate behaviour of 
-Dfile.encoding=COMPAT to

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

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

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v7]

On Thu, 17 Feb 2022 20:32:45 GMT, ExE Boss  wrote:

> Note that the `accessFlags()` method is still missing in the 
> `ModuleDescriptor.Opens` and the top‑level `ModuleDescriptor` classes:
> 
> https://github.com/openjdk/jdk/blob/5cc6ba50b1155340d13bc29d581c91ac543d2d95/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java#L637-L651
> 
> https://github.com/openjdk/jdk/blob/5cc6ba50b1155340d13bc29d581c91ac543d2d95/src/java.base/share/classes/java/lang/module/ModuleDescriptor.java#L1307-L1324

You're correct; addressed in subsequent push. Thanks.

-

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

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v8]

> This is an early review of changes to better model JVM access flags, that is 
> "modifiers" like public, protected, etc. but explicitly at a VM level.
> 
> Language level modifiers and JVM level access flags are closely related, but 
> distinct. There are concepts that overlap in the two domains (public, 
> private, etc.), others that only have a language-level modifier (sealed), and 
> still others that only have an access flag (synthetic).
> 
> The existing java.lang.reflect.Modifier class is inadequate to model these 
> subtleties. For example, the bit positions used by access flags on different 
> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
> methods. Just having a raw integer does not provide sufficient context to 
> decode the corresponding language-level string. Methods like 
> Modifier.methodModifiers() were introduced to cope with this situation.
> 
> With additional modifiers and flags on the horizon with projects like 
> Valhalla, addressing the existent modeling deficiency now ahead of time is 
> reasonable before further strain is introduced.
> 
> This PR in its current form is meant to give the overall shape of the API. It 
> is missing implementations to map from, say, method modifiers to access 
> flags, taking into account overlaps in bit positions.
> 
> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
> once the API is further along.

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

  Respond to review feedback.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7445/files
  - new: https://git.openjdk.java.net/jdk/pull/7445/files/5cc6ba50..131010f3

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

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

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

Re: RFR: 8279995: jpackage --add-launcher option should allow overriding description [v3]

2022-02-17 Thread Alexander Matveev

On Tue, 15 Feb 2022 15:48:02 GMT, Alexey Semenyuk  wrote:

>> Alexander Matveev has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   8279995: jpackage --add-launcher option should allow overriding 
>> description [v3]
>
> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/WindowsHelper.java line 159:
> 
>> 157: }
>> 158: 
>> 159: TKit.assertNotNull(description, "Failed to get file 
>> description");
> 
> Failure to get the executable's description through powersehll script is not 
> an issue of a test case being executed. This is the testing framework issue. 
> Throwing RuntimeException with the message containing file path will be the 
> appropriate error handling. Having a file name in the exception message will 
> help to localize the issue.

Fixed. Error message also fixed for description check.

-

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

Re: RFR: 8279995: jpackage --add-launcher option should allow overriding description [v5]

2022-02-17 Thread Alexander Matveev

> Added ability to override description for additional launchers via 
> "description" property.

Alexander Matveev has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains five commits:

 - Merge master
 - 8279995: jpackage --add-launcher option should allow overriding description 
[v4]
 - 8279995: jpackage --add-launcher option should allow overriding description 
[v3]
 - 8279995: jpackage --add-launcher option should allow overriding description 
[v2]
 - 8279995: jpackage --add-launcher option should allow overriding description

-

Changes: https://git.openjdk.java.net/jdk/pull/7399/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7399=04
  Stats: 77 lines in 7 files changed: 67 ins; 0 del; 10 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7399.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7399/head:pull/7399

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

Re: RFR: 8281962: Avoid unnecessary native calls in InflaterInputStream [v2]

On Thu, 17 Feb 2022 16:02:42 GMT, Volker Simonis  wrote:

>> Currently, `InflaterInputStream::read()` first does a native call to the 
>> underlying zlib `inflate()` function and only afterwards checks if the 
>> inflater requires input (i.e. `Inflater::needsInput()`) or has finished 
>> inflating (`Inflater::finished()`). This leads to an unnecessary native call 
>> to `inflate()` when `InflaterInputStream::read()` is invoked for the very 
>> first time because `Inflater::fill()` hasn't been called and another 
>> unnecessary native call to detect the end of the stream. For small 
>> streams/files which completely fit into the output buffer passed to 
>> `InflaterInputStream::read()` we can therefore save two out of three native 
>> calls. The attached micro benchmark shows that this results in a 5%-10% 
>> performance improvement for zip files of sizes between 4096 to 256 bytes 
>> (notice that in common jars like Tomcat, Spring-Boot, Maven, Jackson, etc. 
>> about 60-80% of the classes are smaller than 4096 bytes).
>> 
>> 
>> before JDK-8281962
>> --
>> Benchmark (size)  Mode  Cnt  Score   
>> Error  Units
>> InflaterInputStreams.inflaterInputStreamRead 256  avgt5  2.571 ± 
>> 0.120  us/op
>> InflaterInputStreams.inflaterInputStreamRead 512  avgt5  2.861 ± 
>> 0.064  us/op
>> InflaterInputStreams.inflaterInputStreamRead4096  avgt5  5.110 ± 
>> 0.278  us/op
>> 
>> after JDK-8281962
>> -
>> Benchmark (size)  Mode  Cnt  Score   
>> Error  Units
>> InflaterInputStreams.inflaterInputStreamRead 256  avgt5  2.332 ± 
>> 0.081  us/op
>> InflaterInputStreams.inflaterInputStreamRead 512  avgt5  2.691 ± 
>> 0.293  us/op
>> InflaterInputStreams.inflaterInputStreamRead4096  avgt5  4.812 ± 
>> 1.038  us/op
>> 
>> 
>> Tested with the JTreg zip/jar/zipfs and the JCK zip/jar tests.
>> 
>> As a side effect, this change also fixes an issue with alternative zlib 
>> implementations like zlib-Chromium or zlib-Cloudflare which pad the inflated 
>> bytes with a specif byte pattern at the end of the output for debugging 
>> purpose. This breaks code patterns like the following:
>> 
>> 
>> int readcount = 0;
>> while ((bytesRead = inflaterInputStream.read(data, 0, bufferSize)) != -1) {
>> outputStream.write(data, 0, bytesRead);
>> readCount++;
>> }
>> if (readCount == 1) {
>> return data; //  < first bytes might be overwritten
>> }
>> return outputStream.toByteArray();
>> 
>> 
>> Even if the whole data fits into the `data` array during the first call to 
>> `inflaterInputStream.read()`, we still need a second call to 
>> `inflaterInputStream.read()` in order to detect the end of the inflater 
>> stream. If this second call calls into the native `inflate()` function of 
>> Cloudflare/Chromium's zlib version, this will still write some padding bytes 
>> at the beginning of the `data` array and overwrite the previously read data. 
>> This issue has been reported in Spring [1] and ASM [2] when using these 
>> libraries with Cloudflares zlib version (by setting `LD_LIBRARY_PATH` 
>> accordingly).
>> 
>> [1] https://github.com/spring-projects/spring-framework/issues/27429
>> [2] https://gitlab.ow2.org/asm/asm/-/issues/317955
>
> Volker Simonis has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Changed hardcoded constant to JMH parmater and removed non-ASCII chars from 
> comments

The change looks innocuous so it is probably OK.  I would like to kick of our 
Mach5 runs to see if it shakes out any potential issues.

>From reading the 3rd party problem reports, it appears that the problem is 
>with the 3rd party zlib implementations.  Hopefully this change will not mask 
>other issues

-

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

Re: RFR: 8281317: CompactNumberFormat displays 4-digit values when rounding to a new range [v2]

2022-02-17 Thread Naoto Sato

On Thu, 17 Feb 2022 07:32:36 GMT, Selikoff  wrote:

> Can I please be added as a Reviewer? I was the one who originally reported 
> this bug on 2/7/2022 via Oracle's web form.

To be listed as a Reviewer, you will need to be a Reviewer of the JDK Project 
(https://openjdk.java.net/bylaws#reviewer). Instead, I tried to add you as a 
contributor in this PR, but failed as I wasn't aware `/contributor` command 
does not work after the PR is closed. Sorry.

-

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

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v6]

Okay; I'll work on a location/context enum to model where flags can 
appear for the next iteration as the place-holder ElementType enum fro 
annotations doesn't quite work for this VM-centric context.


Thanks,

-Joe

On 2/17/2022 10:16 AM, Brian Goetz wrote:

Yes, and ...

There are words of flags elsewhere scattered through the JDK, such the 
InnerClasses attribute, module flags in the Module attribute, and 
flags on the "requires" entries in the Module attribute. Having one 
abstraction to cover all these locations, even though reflection 
doesn't currently serve up them all, would be a sensible thing.




On 2/17/2022 11:34 AM, Roger Riggs wrote:

On Thu, 17 Feb 2022 01:38:42 GMT, Joe Darcy  wrote:

This is an early review of changes to better model JVM access 
flags, that is "modifiers" like public, protected, etc. but 
explicitly at a VM level.


Language level modifiers and JVM level access flags are closely 
related, but distinct. There are concepts that overlap in the two 
domains (public, private, etc.), others that only have a 
language-level modifier (sealed), and still others that only have 
an access flag (synthetic).


The existing java.lang.reflect.Modifier class is inadequate to 
model these subtleties. For example, the bit positions used by 
access flags on different kinds of elements overlap (such as 
"volatile" for fields and "bridge" for methods. Just having a raw 
integer does not provide sufficient context to decode the 
corresponding language-level string. Methods like 
Modifier.methodModifiers() were introduced to cope with this 
situation.


With additional modifiers and flags on the horizon with projects 
like Valhalla, addressing the existent modeling deficiency now 
ahead of time is reasonable before further strain is introduced.


This PR in its current form is meant to give the overall shape of 
the API. It is missing implementations to map from, say, method 
modifiers to access flags, taking into account overlaps in bit 
positions.


The CSRhttps://bugs.openjdk.java.net/browse/JDK-8281660 will be 
filled in once the API is further along.
Joe Darcy 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 
ten additional commits since the last revision:


  - Update JVMS references.
  - Merge branch 'master' into JDK-8266670
  - Reorder constants by mask value per review feedback.
  - Merge branch 'master' into JDK-8266670
  - Respond to more review feedback.
  - Respond to review feedback explicitly stating returned sets are 
immutable.

  - Fix typo from review feedback.
  - Merge branch 'master' into JDK-8266670
  - JDK-8266670: Better modeling of modifiers in core reflection

The jvms also has `access_flags` for parameters.
[4.7.24. The MethodParameters 
Attribute](https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.7.24)


Even if java.lang.reflect.Parameter is not a "Member", it would be 
useful for Parameter to have an `accessFlags()` method and loosen up 
the spec for AccessFlag to allow its use in Parameter.

Its access flags have the same underlying bit patterns and definitions.

-

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

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v7]

2022-02-17 Thread ExE Boss

On Thu, 17 Feb 2022 19:26:13 GMT, Joe Darcy  wrote:

>> This is an early review of changes to better model JVM access flags, that is 
>> "modifiers" like public, protected, etc. but explicitly at a VM level.
>> 
>> Language level modifiers and JVM level access flags are closely related, but 
>> distinct. There are concepts that overlap in the two domains (public, 
>> private, etc.), others that only have a language-level modifier (sealed), 
>> and still others that only have an access flag (synthetic).
>> 
>> The existing java.lang.reflect.Modifier class is inadequate to model these 
>> subtleties. For example, the bit positions used by access flags on different 
>> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
>> methods. Just having a raw integer does not provide sufficient context to 
>> decode the corresponding language-level string. Methods like 
>> Modifier.methodModifiers() were introduced to cope with this situation.
>> 
>> With additional modifiers and flags on the horizon with projects like 
>> Valhalla, addressing the existent modeling deficiency now ahead of time is 
>> reasonable before further strain is introduced.
>> 
>> This PR in its current form is meant to give the overall shape of the API. 
>> It is missing implementations to map from, say, method modifiers to access 
>> flags, taking into account overlaps in bit positions.
>> 
>> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
>> once the API is further along.
>
> Joe Darcy 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 11 additional commits since 
> the last revision:
> 
>  - Add support for module flags; fix typo.
>  - Merge branch 'master' into JDK-8266670
>  - Update JVMS references.
>  - Merge branch 'master' into JDK-8266670
>  - Reorder constants by mask value per review feedback.
>  - Merge branch 'master' into JDK-8266670
>  - Respond to more review feedback.
>  - Respond to review feedback explicitly stating returned sets are immutable.
>  - Fix typo from review feedback.
>  - Merge branch 'master' into JDK-8266670
>  - ... and 1 more: 
> https://git.openjdk.java.net/jdk/compare/b1511105...5cc6ba50

Note that the `accessFlags()` method is still missing in the 
`ModuleDescriptor.Opens` and the top‑level `ModuleDescriptor` classes: 

 


-

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

Re: RFR: 8281317: CompactNumberFormat displays 4-digit values when rounding to a new range [v2]

2022-02-17 Thread Selikoff

On Tue, 15 Feb 2022 22:31:47 GMT, Naoto Sato  wrote:

>> Fixing an issue in `CompactNumberFormat` which was caused by 
>> BigDecimal.divide() that incremented the number in the resulting format 
>> string. Also fixing some typos by taking this opportunity.
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Addressing review comments

Can I please be added as a Reviewer?  I was the one who originally reported 
this bug on 2/7/2022 via Oracle's web form.

-

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

Re: RFR: 8279488: ProcessBuilder inherits contextClassLoader when spawning a process reaper thread

2022-02-17 Thread Roger Riggs

On Tue, 18 Jan 2022 15:57:58 GMT, Roger Riggs  wrote:

> The thread factory used to create the process reaper threads unnecessarily 
> inherits the callers thread context classloader.
> The result is retention of the class loader.
> 
> The thread factory used for the pool of process reaper threads is modified to 
> use an InnocuousThread with a given stacksize.
> The test verifies that the process reaper threads have a null context 
> classloader.

Please review this use of InnocuousThread.  Tnx

-

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

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v18]

> 8281631: HashMap copy constructor and putAll can over-allocate table

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

  migrate to junit

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7431/files
  - new: https://git.openjdk.java.net/jdk/pull/7431/files/29af7c24..cdfb03bb

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=17
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=16-17

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

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

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v17]

> 8281631: HashMap copy constructor and putAll can over-allocate table

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

  migrate to junit

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7431/files
  - new: https://git.openjdk.java.net/jdk/pull/7431/files/87b73334..29af7c24

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=16
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=15-16

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

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

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v7]

> This is an early review of changes to better model JVM access flags, that is 
> "modifiers" like public, protected, etc. but explicitly at a VM level.
> 
> Language level modifiers and JVM level access flags are closely related, but 
> distinct. There are concepts that overlap in the two domains (public, 
> private, etc.), others that only have a language-level modifier (sealed), and 
> still others that only have an access flag (synthetic).
> 
> The existing java.lang.reflect.Modifier class is inadequate to model these 
> subtleties. For example, the bit positions used by access flags on different 
> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
> methods. Just having a raw integer does not provide sufficient context to 
> decode the corresponding language-level string. Methods like 
> Modifier.methodModifiers() were introduced to cope with this situation.
> 
> With additional modifiers and flags on the horizon with projects like 
> Valhalla, addressing the existent modeling deficiency now ahead of time is 
> reasonable before further strain is introduced.
> 
> This PR in its current form is meant to give the overall shape of the API. It 
> is missing implementations to map from, say, method modifiers to access 
> flags, taking into account overlaps in bit positions.
> 
> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
> once the API is further along.

Joe Darcy 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 11 additional commits since the 
last revision:

 - Add support for module flags; fix typo.
 - Merge branch 'master' into JDK-8266670
 - Update JVMS references.
 - Merge branch 'master' into JDK-8266670
 - Reorder constants by mask value per review feedback.
 - Merge branch 'master' into JDK-8266670
 - Respond to more review feedback.
 - Respond to review feedback explicitly stating returned sets are immutable.
 - Fix typo from review feedback.
 - Merge branch 'master' into JDK-8266670
 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/d7b3d5dd...5cc6ba50

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7445/files
  - new: https://git.openjdk.java.net/jdk/pull/7445/files/02228108..5cc6ba50

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

  Stats: 494 lines in 13 files changed: 48 ins; 412 del; 34 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7445.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7445/head:pull/7445

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

Integrated: 8281317: CompactNumberFormat displays 4-digit values when rounding to a new range

2022-02-17 Thread Naoto Sato

On Wed, 9 Feb 2022 22:37:45 GMT, Naoto Sato  wrote:

> Fixing an issue in `CompactNumberFormat` which was caused by 
> BigDecimal.divide() that incremented the number in the resulting format 
> string. Also fixing some typos by taking this opportunity.

This pull request has now been integrated.

Changeset: 12927765
Author:Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/129277653e51e9b1387ecee279a6ccee9199c8ff
Stats: 42 lines in 2 files changed: 31 ins; 0 del; 11 mod

8281317: CompactNumberFormat displays 4-digit values when rounding to a new 
range

Reviewed-by: joehw

-

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

Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName() [v3]

On Fri, 11 Feb 2022 17:43:47 GMT, Lance Andersen  wrote:

>> src/java.base/share/classes/java/util/jar/JarFile.java line 881:
>> 
>>> 879: ze = getJarEntry(entryName);
>>> 880: } else {
>>> 881: throw new ZipException("ZipEntry::getName returned null");
>> 
>> Throwing ZipException when ZipEntry::getName returns null is still 
>> surprising but not terrible.  The spec for getInputStream specifies 
>> ZipException for when a zip file format occurs so we might have to extend 
>> that to add "or the zip entry name is null".
>
> If you  would like me to update the ZipException to clarify this I can.   The 
> original issue was due to a format error in the CEN so the current wording 
> covers that.  It does not cover the case where ZipEntry is subclassed and 
> ZipEntry::getName() returns null which is what your suggested wording would 
> address.
> 
> Please note the above change addresses the signed jar scenario.  I can add an 
> additional check in JarFile::getInputStream to check for null from 
> ZipEntry::getName so that it covers unsigned jars and signed jars not being 
> verified.
> 
> The issue will not occur with ZipEFile::getInputStream given its use of 
> ZipEntry.name which can never be null.
> 
> Another thought would be to return null for the InputStream similar to when 
> the ZipEntry does not represent a file within the Jar
> 
> Please let me know your preference

Per a discussion with Sean and Alan,  we are now returning null for the 
InputStream to be consistent with ZipFile::getInputStream and how unsigned jars 
are handled

-

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

Re: RFR: 8280409: JarFile::verifiableEntry can fail with NPE accessing ze.getName() [v4]

> Hi all,
> 
> Please review the attached patch to address
> 
> - That JarFile::getInputStream did not check for a null ZipEntry passed as a 
> parameter
> - Have Zip/JarFile::getInputStream throw a ZipException in the event that an 
> unexpected exception occurs
> 
> Mach5 tiers1-3 runs are clean as are the TCK java.util.zip and java.util.jar 
> test runs
> 
> Best
> Lance

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

  Return null when ZipEntry::getName is null

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7348/files
  - new: https://git.openjdk.java.net/jdk/pull/7348/files/32f6c284..d5cf8db8

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

  Stats: 201 lines in 2 files changed: 34 ins; 96 del; 71 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7348.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7348/head:pull/7348

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

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v6]

2022-02-17 Thread Joseph D. Darcy

On 2/17/2022 8:34 AM, Roger Riggs wrote:

On Thu, 17 Feb 2022 01:38:42 GMT, Joe Darcy wrote:

This is an early review of changes to better model JVM access flags, that is
"modifiers" like public, protected, etc. but explicitly at a VM level.

Language level modifiers and JVM level access flags are closely related, but
distinct. There are concepts that overlap in the two domains (public, private,
etc.), others that only have a language-level modifier (sealed), and still
others that only have an access flag (synthetic).

The existing java.lang.reflect.Modifier class is inadequate to model these subtleties. For example,
the bit positions used by access flags on different kinds of elements overlap (such as
"volatile" for fields and "bridge" for methods. Just having a raw integer does
not provide sufficient context to decode the corresponding language-level string. Methods like
Modifier.methodModifiers() were introduced to cope with this situation.

With additional modifiers and flags on the horizon with projects like Valhalla,
addressing the existent modeling deficiency now ahead of time is reasonable
before further strain is introduced.

This PR in its current form is meant to give the overall shape of the API. It
is missing implementations to map from, say, method modifiers to access flags,
taking into account overlaps in bit positions.

The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in once
the API is further along.

Joe Darcy 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 ten additional commits since the
last revision:

- Update JVMS references.
- Merge branch 'master' into JDK-8266670
- Reorder constants by mask value per review feedback.
- Merge branch 'master' into JDK-8266670
- Respond to more review feedback.
- Respond to review feedback explicitly stating returned sets are immutable.
- Fix typo from review feedback.
- Merge branch 'master' into JDK-8266670
- JDK-8266670: Better modeling of modifiers in core reflection

The jvms also has `access_flags` for parameters.
[4.7.24. The MethodParameters
Attribute](https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.7.24)

I'll augment/edit the spec describing where the flags come from to
mention method parameters.

Even if java.lang.reflect.Parameter is not a "Member", it would be useful for
Parameter to have an `accessFlags()` method and loosen up the spec for AccessFlag to
allow its use in Parameter.
Its access flags have the same underlying bit patterns and definitions.

An accessFlags method on Parameter has been part of this proposal since
the first push:

https://openjdk.github.io/cr/?repo=jdk=7445=00#udiff-5-src/java.base/share/classes/java/lang/reflect/Parameter.java

-Joe

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v5]

On Wed, 16 Feb 2022 13:18:47 GMT, ExE Boss  wrote:

>> Joe Darcy 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 seven additional commits 
>> since the last revision:
>> 
>>  - Reorder constants by mask value per review feedback.
>>  - Merge branch 'master' into JDK-8266670
>>  - Respond to more review feedback.
>>  - Respond to review feedback explicitly stating returned sets are immutable.
>>  - Fix typo from review feedback.
>>  - Merge branch 'master' into JDK-8266670
>>  - JDK-8266670: Better modeling of modifiers in core reflection
>
> test/jdk/java/lang/reflect/AccessFlag/BasicAccessFlagTest.java line 65:
> 
>> 63: if (left.mask() > right.mask()) {
>> 64: throw new RuntimeException(left
>> 65:+ "has a greater mas than "
> 
> Typo:
> Suggestion:
> 
>+ "has a greater mask than "

Will correct in next push; thanks.

-

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

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v6]

2022-02-17 Thread Brian Goetz

Yes, and ...

There are words of flags elsewhere scattered through the JDK, such the
InnerClasses attribute, module flags in the Module attribute, and flags
on the "requires" entries in the Module attribute. Having one
abstraction to cover all these locations, even though reflection doesn't
currently serve up them all, would be a sensible thing.

On 2/17/2022 11:34 AM, Roger Riggs wrote:

On Thu, 17 Feb 2022 01:38:42 GMT, Joe Darcy wrote:

This is an early review of changes to better model JVM access flags, that is
"modifiers" like public, protected, etc. but explicitly at a VM level.

With additional modifiers and flags on the horizon with projects like Valhalla,
addressing the existent modeling deficiency now ahead of time is reasonable
before further strain is introduced.

The CSRhttps://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in once
the API is further along.

The jvms also has `access_flags` for parameters.
[4.7.24. The MethodParameters
Attribute](https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.7.24)

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

Re: RFR: 8276686: Malformed Javadoc inline tags in JDK source in /java/util/regex/Pattern.java

On Thu, 17 Feb 2022 18:02:20 GMT, Ian Graves  wrote:

> Adding a missing period per this doc bug.

Marked as reviewed by lancea (Reviewer).

-

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

Re: RFR: 8276686: Malformed Javadoc inline tags in JDK source in /java/util/regex/Pattern.java

2022-02-17 Thread Iris Clark

On Thu, 17 Feb 2022 18:02:20 GMT, Ian Graves  wrote:

> Adding a missing period per this doc bug.

Marked as reviewed by iris (Reviewer).

-

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

Re: RFR: 8276686: Malformed Javadoc inline tags in JDK source in /java/util/regex/Pattern.java

2022-02-17 Thread Brian Burkhalter

On Thu, 17 Feb 2022 18:02:20 GMT, Ian Graves  wrote:

> Adding a missing period per this doc bug.

Marked as reviewed by bpb (Reviewer).

-

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

RFR: 8276686: Malformed Javadoc inline tags in JDK source in /java/util/regex/Pattern.java

2022-02-17 Thread Ian Graves

Adding a missing period per this doc bug.

-

Commit messages:
 - Adding missing period

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

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

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

2022-02-17 Thread Jatin Bhateja

> Summary of changes:
> - Intrinsify Math.round(float) and Math.round(double) APIs.
> - Extend auto-vectorizer to infer vector operations on encountering scalar IR 
> nodes for above intrinsics.
> - Test creation using new IR testing framework.
> 
> Following are the performance number of a JMH micro included with the patch 
> 
> Test System: Intel(R) Xeon(R) Platinum 8380 CPU @ 2.30GHz (Icelake Server)
> 
> 
> TESTSIZE | Baseline AVX3 (ops/ms) | Withopt AVX3 (ops/ms) | Gain ratio | 
> Baseline AVX2 (ops/ms) | Withopt AVX2 (ops/ms) | Gain ratio
> -- | -- | -- | -- | -- | -- | --
> 1024.00 | 510.41 | 1811.66 | 3.55 | 510.40 | 502.65 | 0.98
> 2048.00 | 293.52 | 984.37 | 3.35 | 304.96 | 177.88 | 0.58
> 1024.00 | 825.94 | 3387.64 | 4.10 | 750.77 | 1925.15 | 2.56
> 2048.00 | 411.91 | 1942.87 | 4.72 | 412.22 | 1034.13 | 2.51
> 
> 
> Kindly review and share your feedback.
> 
> Best Regards,
> Jatin

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

  8279508: Fixing for windows failure.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7094/files
  - new: https://git.openjdk.java.net/jdk/pull/7094/files/73674fe4..f35ed9cf

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

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

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

Integrated: JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is null

2022-02-17 Thread Tim Prinzing

On Fri, 11 Feb 2022 20:36:26 GMT, Tim Prinzing  wrote:

> JDK-8281000 ClassLoader::registerAsParallelCapable throws NPE if caller is 
> null

This pull request has now been integrated.

Changeset: a6f8a386
Author:Tim Prinzing 
Committer: Mandy Chung 
URL:   
https://git.openjdk.java.net/jdk/commit/a6f8a386efa7af162f4b815951287f0a9bc1f396
Stats: 216 lines in 5 files changed: 216 ins; 0 del; 0 mod

8281000: ClassLoader::registerAsParallelCapable throws NPE if caller is null

Reviewed-by: erikj, ihse, mchung, bchristi

-

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

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v16]

> 8281631: HashMap copy constructor and putAll can over-allocate table

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

  migrate to junit

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7431/files
  - new: https://git.openjdk.java.net/jdk/pull/7431/files/2998571a..87b73334

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=15
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=14-15

  Stats: 100 lines in 1 file changed: 74 ins; 7 del; 19 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7431.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7431/head:pull/7431

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

Integrated: 8268250: Class.arrayType() for a 255-d array throws undocumented IllegalArgumentException

On Mon, 7 Jun 2021 00:09:33 GMT, Joe Darcy  wrote:

> Make explicit illegal argument cases of Class.arrayType.
> 
> Please also review the corresponding CSR: 
> https://bugs.openjdk.java.net/browse/JDK-8268300

This pull request has now been integrated.

Changeset: 4c7f8b49
Author:Joe Darcy 
URL:   
https://git.openjdk.java.net/jdk/commit/4c7f8b49a4845acf58272c42327328d6d2837cea
Stats: 59 lines in 2 files changed: 58 ins; 0 del; 1 mod

8268250: Class.arrayType() for a 255-d array throws undocumented 
IllegalArgumentException

Reviewed-by: sundar, alanb

-

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

Re: RFR: 8254574: PrintWriter handling of InterruptedIOException is not documented

2022-02-17 Thread Brian Burkhalter

On Wed, 16 Feb 2022 22:32:21 GMT, Brian Burkhalter  wrote:

> Remove reference to `java.io.InterruptedIOException` from 
> `java.io.PrintStream`, and make the specifications of `checkError()`, 
> `setError()`, and `clearError()` consistent between `java.io.PrintStream` and 
> `java.io.PrintWriter`.

Thanks. I was holding off on that label until the PR approached consensus.

-

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

Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v6]

2022-02-17 Thread Roger Riggs

On Thu, 17 Feb 2022 01:38:42 GMT, Joe Darcy  wrote:

>> This is an early review of changes to better model JVM access flags, that is 
>> "modifiers" like public, protected, etc. but explicitly at a VM level.
>> 
>> Language level modifiers and JVM level access flags are closely related, but 
>> distinct. There are concepts that overlap in the two domains (public, 
>> private, etc.), others that only have a language-level modifier (sealed), 
>> and still others that only have an access flag (synthetic).
>> 
>> The existing java.lang.reflect.Modifier class is inadequate to model these 
>> subtleties. For example, the bit positions used by access flags on different 
>> kinds of elements overlap (such as "volatile" for fields and "bridge" for 
>> methods. Just having a raw integer does not provide sufficient context to 
>> decode the corresponding language-level string. Methods like 
>> Modifier.methodModifiers() were introduced to cope with this situation.
>> 
>> With additional modifiers and flags on the horizon with projects like 
>> Valhalla, addressing the existent modeling deficiency now ahead of time is 
>> reasonable before further strain is introduced.
>> 
>> This PR in its current form is meant to give the overall shape of the API. 
>> It is missing implementations to map from, say, method modifiers to access 
>> flags, taking into account overlaps in bit positions.
>> 
>> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in 
>> once the API is further along.
>
> Joe Darcy 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 ten additional commits since 
> the last revision:
> 
>  - Update JVMS references.
>  - Merge branch 'master' into JDK-8266670
>  - Reorder constants by mask value per review feedback.
>  - Merge branch 'master' into JDK-8266670
>  - Respond to more review feedback.
>  - Respond to review feedback explicitly stating returned sets are immutable.
>  - Fix typo from review feedback.
>  - Merge branch 'master' into JDK-8266670
>  - JDK-8266670: Better modeling of modifiers in core reflection

The jvms also has `access_flags` for parameters. 
[4.7.24. The MethodParameters 
Attribute](https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.7.24)

Even if java.lang.reflect.Parameter is not a "Member", it would be useful for 
Parameter to have an `accessFlags()` method and loosen up the spec for 
AccessFlag to allow its use in Parameter.
Its access flags have the same underlying bit patterns and definitions.

-

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

Re: RFR: 8281962: Avoid unnecessary native calls in InflaterInputStream

On Thu, 17 Feb 2022 09:59:08 GMT, Alan Bateman  wrote:

> This change is probably okay 

Hi Alan,

thanks for looking at this change.

> but will require study to see if there are any side effects (sadly, this area 
> has a history of side effects being reported months and years after a 
> change). Would you mind holding off integrating this change until it has been 
> reviewed by someone that works in the area?

Sure, no problem. But can you please be a little more specific on who you 
consider to qualify as "*works in the area*" :)

-

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

Re: RFR: 8281962: Avoid unnecessary native calls in InflaterInputStream [v2]

On Thu, 17 Feb 2022 10:01:11 GMT, Claes Redestad  wrote:

>> Volker Simonis has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Changed hardcoded constant to JMH parmater and removed non-ASCII chars 
>> from comments
>
> test/micro/org/openjdk/bench/java/util/zip/InflaterInputStreams.java line 78:
> 
>> 76: private byte[] words;
>> 77: private static final int MAX_SIZE = 5000; // Should be bigger than 
>> the biggest size @Param
>> 78: private static byte[] inflated = new byte[MAX_SIZE];
> 
> If it isn't important for the benchmark, I'd remove the hard-coded `MAX_SIZE` 
> and make `inflated` non-static and allocate it in `beforeRun` using `size` 
> (or some multiple thereof). Future enhancements could very well be interested 
> in assessing the performance of larger sized entries, and we shouldn't put up 
> trip-wires if they want to increase `size`.

Sure , you're absolutely right. Updated as suggested.

I've also removed some non-ASCII characters from the comment to avoid problems 
with javac.

Finally, a year isn't required in the Amazon copyright header. We specially 
removed all years a while back.

PS: and thanks a lot for your review :)

-

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

Re: RFR: 8281962: Avoid unnecessary native calls in InflaterInputStream [v2]

> Currently, `InflaterInputStream::read()` first does a native call to the 
> underlying zlib `inflate()` function and only afterwards checks if the 
> inflater requires input (i.e. `Inflater::needsInput()`) or has finished 
> inflating (`Inflater::finished()`). This leads to an unnecessary native call 
> to `inflate()` when `InflaterInputStream::read()` is invoked for the very 
> first time because `Inflater::fill()` hasn't been called and another 
> unnecessary native call to detect the end of the stream. For small 
> streams/files which completely fit into the output buffer passed to 
> `InflaterInputStream::read()` we can therefore save two out of three native 
> calls. The attached micro benchmark shows that this results in a 5%-10% 
> performance improvement for zip files of sizes between 4096 to 256 bytes 
> (notice that in common jars like Tomcat, Spring-Boot, Maven, Jackson, etc. 
> about 60-80% of the classes are smaller than 4096 bytes).
> 
> 
> before JDK-8281962
> --
> Benchmark (size)  Mode  Cnt  Score   
> Error  Units
> InflaterInputStreams.inflaterInputStreamRead 256  avgt5  2.571 ± 
> 0.120  us/op
> InflaterInputStreams.inflaterInputStreamRead 512  avgt5  2.861 ± 
> 0.064  us/op
> InflaterInputStreams.inflaterInputStreamRead4096  avgt5  5.110 ± 
> 0.278  us/op
> 
> after JDK-8281962
> -
> Benchmark (size)  Mode  Cnt  Score   
> Error  Units
> InflaterInputStreams.inflaterInputStreamRead 256  avgt5  2.332 ± 
> 0.081  us/op
> InflaterInputStreams.inflaterInputStreamRead 512  avgt5  2.691 ± 
> 0.293  us/op
> InflaterInputStreams.inflaterInputStreamRead4096  avgt5  4.812 ± 
> 1.038  us/op
> 
> 
> Tested with the JTreg zip/jar/zipfs and the JCK zip/jar tests.
> 
> As a side effect, this change also fixes an issue with alternative zlib 
> implementations like zlib-Chromium or zlib-Cloudflare which pad the inflated 
> bytes with a specif byte pattern at the end of the output for debugging 
> purpose. This breaks code patterns like the following:
> 
> 
> int readcount = 0;
> while ((bytesRead = inflaterInputStream.read(data, 0, bufferSize)) != -1) {
> outputStream.write(data, 0, bytesRead);
> readCount++;
> }
> if (readCount == 1) {
> return data; //  < first bytes might be overwritten
> }
> return outputStream.toByteArray();
> 
> 
> Even if the whole data fits into the `data` array during the first call to 
> `inflaterInputStream.read()`, we still need a second call to 
> `inflaterInputStream.read()` in order to detect the end of the inflater 
> stream. If this second call calls into the native `inflate()` function of 
> Cloudflare/Chromium's zlib version, this will still write some padding bytes 
> at the beginning of the `data` array and overwrite the previously read data. 
> This issue has been reported in Spring [1] and ASM [2] when using these 
> libraries with Cloudflares zlib version (by setting `LD_LIBRARY_PATH` 
> accordingly).
> 
> [1] https://github.com/spring-projects/spring-framework/issues/27429
> [2] https://gitlab.ow2.org/asm/asm/-/issues/317955

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

  Changed hardcoded constant to JMH parmater and removed non-ASCII chars from 
comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7492/files
  - new: https://git.openjdk.java.net/jdk/pull/7492/files/8314380e..c393b584

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

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

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

Re: RFR: 8281962: Avoid unnecessary native calls in InflaterInputStream [v2]

On Thu, 17 Feb 2022 09:35:47 GMT, Christoph Langer  wrote:

> Makes sense to me. Benchmark numbers look good.

Thanks Christoph!

-

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

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v14]

On Thu, 17 Feb 2022 05:46:38 GMT, Stuart Marks  wrote:

>> XenoAmess has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fix test
>
> test/jdk/java/util/Map/HashMapsPutAllOverAllocateTableTest.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
> 
> Fix copyright year.

changed to 2022.

-

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

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v15]

> 8281631: HashMap copy constructor and putAll can over-allocate table

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

  fix license year in test

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7431/files
  - new: https://git.openjdk.java.net/jdk/pull/7431/files/d0a4e6fa..2998571a

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7431=14
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7431=13-14

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

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

Re: RFR: 8281146: Replace StringCoding.hasNegatives with countPositives [v3]

2022-02-17 Thread Claes Redestad

> I'm requesting comments and, hopefully, some help with this patch to replace 
> `StringCoding.hasNegatives` with `countPositives`. The new method does a very 
> similar pass, but alters the intrinsic to return the number of leading bytes 
> in the `byte[]` range which only has positive bytes. This allows for dealing 
> much more efficiently with those `byte[]`s that has a ASCII prefix, with no 
> measurable cost on ASCII-only or latin1/UTF16-mostly input.
> 
> Microbenchmark results: 
> https://jmh.morethan.io/?gists=428b487e92e3e47ccb7f169501600a88,3c585de7435506d3a3bdb32160fe8904
> 
> - Only implemented on x86 for now, but I want to verify that implementations 
> of `countPositives` can be implemented with similar efficiency on all 
> platforms that today implement a `hasNegatives` intrinsic (aarch64, ppc etc) 
> before moving ahead. This pretty much means holding up this until it's 
> implemented on all platforms, which can either contributed to this PR or as 
> dependent follow-ups.
> 
> - An alternative to holding up until all platforms are on board is to allow 
> the implementation of `StringCoding.hasNegatives` and `countPositives` to be 
> implemented so that the non-intrinsified method calls into the intrinsified. 
> This requires structuring the implementations differently based on which 
> intrinsic - if any - is actually implemented. One way to do this could be to 
> mimic how `java.nio` handles unaligned accesses and expose which intrinsic is 
> available via `Unsafe` into a `static final` field.
> 
> - There are a few minor regressions (~5%) in the x86 implementation on 
> `encode-/decodeLatin1Short`. Those regressions disappear when mixing inputs, 
> for example `encode-/decodeShortMixed` even see a minor improvement, which 
> makes me consider those corner case regressions with little real world 
> implications (if you have latin1 Strings, you're likely to also have 
> ASCII-only strings in your mix).

Claes Redestad 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 25 additional commits 
since the last revision:

 - Revert micro changes, split out to #7516
 - Merge branch 'master' of https://github.com/cl4es/jdk into count_positives
 - Merge branch 'master' into count_positives
 - Restore partial vector checks in AVX2 and SSE intrinsic variants
 - Let countPositives use hasNegatives to allow ports not implementing the 
countPositives intrinsic to stay neutral
 - Simplify changes to encodeUTF8
 - Fix little-endian error caught by testing
 - Reduce jumps in the ascii path
 - Remove unused tail_mask
 - Remove has_negatives intrinsic on x86 (and hook up 32-bit x86 to use 
count_positives)
 - ... and 15 more: https://git.openjdk.java.net/jdk/compare/1ca44ef9...531139a1

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7231/files
  - new: https://git.openjdk.java.net/jdk/pull/7231/files/c4bb3612..531139a1

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

  Stats: 10910 lines in 329 files changed: 7340 ins; 2150 del; 1420 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7231.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7231/head:pull/7231

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

Re: RFR: 8281631: HashMap copy constructor and putAll can over-allocate table [v14]