Re: RFR: 8247872: Upgrade HarfBuzz to the latest 2.7.2

2020-11-01 Thread Phil Race
On Mon, 2 Nov 2020 04:19:57 GMT, Phil Race  wrote:

> This upgrades JDK to import the current 2.7.2 version of harfbuzz - an 
> OpenType text shaping library
> 
> https://bugs.openjdk.java.net/browse/JDK-8247872
> 
> This has passed building and headless and headful automated tests on all 
> platforms.

Aside from the code change there is a small build change is to remove some 
obsolete defines

-

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


RFR: 8247872: Upgrade HarfBuzz to the latest 2.7.2

2020-11-01 Thread Phil Race
This upgrades JDK to import the current 2.7.2 version of harfbuzz - an OpenType 
text shaping library

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

This has passed building and headless and headful automated tests on all 
platforms.

-

Commit messages:
 - 8247872: Upgrade HarfBuzz to the latest 2.7.2

Changes: https://git.openjdk.java.net/jdk/pull/993/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=993=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8247872
  Stats: 44663 lines in 207 files changed: 25198 ins; 12729 del; 6736 mod
  Patch: https://git.openjdk.java.net/jdk/pull/993.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/993/head:pull/993

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


Re: RFR: 8255616: Disable AOT and Graal in Oracle OpenJDK

2020-11-01 Thread Vladimir Kozlov
On Sun, 1 Nov 2020 20:15:01 GMT, Igor Veresov  wrote:

>> We shipped Ahead-of-Time compilation (the jaotc tool) in JDK 9, as an 
>> experimental feature. We shipped Graal as an experimental JIT compiler in 
>> JDK 10. We haven't seen much use of these features, and the effort required 
>> to support and enhance them is significant. We therefore intend to disable 
>> these features in Oracle builds as of JDK 16. 
>> 
>> We'll leave the sources for these features in the repository, in case any 
>> one else is interested in building them. But we will not update or test them.
>> 
>> We'll continue to build and ship JVMCI as an experimental feature in Oracle 
>> builds.
>> 
>> Tested changes in all tiers.
>> 
>> I verified that with these changes I still able to build Graal in open repo 
>> and run graalunit testing: 
>> 
>> `open$ bash test/hotspot/jtreg/compiler/graalunit/downloadLibs.sh 
>> /mydir/graalunit_lib/`
>> `open$ bash configure --with-debug-level=fastdebug 
>> --with-graalunit-lib=/mydir/graalunit_lib/ --with-jtreg=/mydir/jtreg`
>> `open$ make jdk-image`
>> `open$ make test-image`
>> `open$ make run-test TEST=compiler/graalunit/HotspotTest.java`
>
> Marked as reviewed by iveresov (Reviewer).

Thank you for reviews.

-

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


Re: RFR: 8255616: Disable AOT and Graal in Oracle OpenJDK

2020-11-01 Thread Igor Veresov
On Fri, 30 Oct 2020 17:40:51 GMT, Vladimir Kozlov  wrote:

> We shipped Ahead-of-Time compilation (the jaotc tool) in JDK 9, as an 
> experimental feature. We shipped Graal as an experimental JIT compiler in JDK 
> 10. We haven't seen much use of these features, and the effort required to 
> support and enhance them is significant. We therefore intend to disable these 
> features in Oracle builds as of JDK 16. 
> 
> We'll leave the sources for these features in the repository, in case any one 
> else is interested in building them. But we will not update or test them.
> 
> We'll continue to build and ship JVMCI as an experimental feature in Oracle 
> builds.
> 
> Tested changes in all tiers.
> 
> I verified that with these changes I still able to build Graal in open repo 
> and run graalunit testing: 
> 
> `open$ bash test/hotspot/jtreg/compiler/graalunit/downloadLibs.sh 
> /mydir/graalunit_lib/`
> `open$ bash configure --with-debug-level=fastdebug 
> --with-graalunit-lib=/mydir/graalunit_lib/ --with-jtreg=/mydir/jtreg`
> `open$ make jdk-image`
> `open$ make test-image`
> `open$ make run-test TEST=compiler/graalunit/HotspotTest.java`

Marked as reviewed by iveresov (Reviewer).

-

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


Re: RFR: 8255616: Disable AOT and Graal in Oracle OpenJDK

2020-11-01 Thread Vladimir Ivanov
On Fri, 30 Oct 2020 17:40:51 GMT, Vladimir Kozlov  wrote:

> We shipped Ahead-of-Time compilation (the jaotc tool) in JDK 9, as an 
> experimental feature. We shipped Graal as an experimental JIT compiler in JDK 
> 10. We haven't seen much use of these features, and the effort required to 
> support and enhance them is significant. We therefore intend to disable these 
> features in Oracle builds as of JDK 16. 
> 
> We'll leave the sources for these features in the repository, in case any one 
> else is interested in building them. But we will not update or test them.
> 
> We'll continue to build and ship JVMCI as an experimental feature in Oracle 
> builds.
> 
> Tested changes in all tiers.
> 
> I verified that with these changes I still able to build Graal in open repo 
> and run graalunit testing: 
> 
> `open$ bash test/hotspot/jtreg/compiler/graalunit/downloadLibs.sh 
> /mydir/graalunit_lib/`
> `open$ bash configure --with-debug-level=fastdebug 
> --with-graalunit-lib=/mydir/graalunit_lib/ --with-jtreg=/mydir/jtreg`
> `open$ make jdk-image`
> `open$ make test-image`
> `open$ make run-test TEST=compiler/graalunit/HotspotTest.java`

Looks good.

-

Marked as reviewed by vlivanov (Reviewer).

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


Re: RFR: 8254162: Implementation of Foreign-Memory Access API (Third Incubator)

2020-11-01 Thread Alan Bateman
On Thu, 29 Oct 2020 14:13:40 GMT, Maurizio Cimadamore  
wrote:

>>> @mcimadamore, if you pull from current master, you would get the Linux 
>>> x86_32 tier1 run "for free".
>> 
>> Just did that - I also removed TestMismatch from the problem list in the 
>> latest iteration, and fixed the alignment for long/double layouts, after 
>> chatting with the team (https://bugs.openjdk.java.net/browse/JDK-8255350)
>
> I've just uploaded another iteration which addresses some comments from 
> @AlanBateman. Basically, there are some operations on Channel and Socket 
> which take ByteBuffer as arguments, and then, if such buffers are *direct*, 
> they get the address and pass it down to some native function. This idiom is 
> problematic because there's no way to guarantee that the buffer won't be 
> closed (if obtained from a memory segment) after the address has been 
> obtained. As a stop gap solution, I've introduced checks in 
> `DirectBuffer::address` method, which is used in around 30 places in the JDK. 
> This method will now throw if (a) the buffer has a shared scope, or (b) if 
> the scope is confined, but already closed. With this extra check, I believe 
> there's no way to misuse the buffer obtained from a segment. We have 
> discussed plans to remove this limitations (which we think will be possible) 
> - but for the time being, it's better to play the conservative card.

I looked through the changes in this update.

The shared memory segment support looks sound and the mechanism to close a 
shared memory segment is clever (albeit a bit surprising at first that it does 
global handshake to look for a frame in a scoped region. Also surprising that 
close can cause failure at both ends - it took me a while to see that this is 
pragmatic approach).

The share method specifies NPE if thread == null but there is no thread 
parameter, is this a cut 'n paste error? Another one in registerCleaner where 
it should be NPE if the cleaner is null.

I think the javadoc for the close method needs to be a bit clearer on the state 
of the memory segment when IllegalStateException is thrown. Will it be marked 
"not alive" when it fails? Does this mean there is a resource leak? I think an 
apiNote to explain the rational for why close is not idempotent is also needed, 
or maybe it should be re-visited so that close is a no-op when the memory 
segment is not alive.

Now that MemorySegment is AutoCloseable then maybe the term "alive" should be 
replaced with "open" or  "closed" and isAlive replaced with isOpen is isClosed.

FileDescriptor can be attraction nuisance and forced reference counting 
everywhere that it is used. Is it needed? Could an isMapped method work instead?

mapFromPath was in the second preview but I think the method name should be 
re-examined as it maps a file, the path just locates the file.  Naming is 
subjectives but in this case using "map" or "mapFile" would fit beside the 
allocateNative methods.

MappedMemorySegments. The force method specifies a write back guarantee but at 
the same time, the implNote in the class description suggests that the methods 
might be a no-op. You might want to adjust the wording to avoid any suggestion 
that force might be a no-op.

The javadoc for copyFrom isn't changed in this update but I notice it specifies 
IndexOutOfBoundException when the source segment is larger than the receiver, 
have other exceptions been examined?

I don't have any any comments on MemoryAccess except that it's not immediately 
clear why there are "Byte" methods that take a ByteOrder. Make sense for the 
multi-byte types of course.

The updates the java/nio sources look okay but it would be helpful if the 
really long lines could be chopped down as it's just too hard to do 
side-by-side reviews when the lines are so long. A minor nit but the changes 
X-Buffer.java.template mess up the alignment of the parameters to 
copyMemory/copySwapMemory methods.

-

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