Re: (ch) InnocuousThread names

2021-10-16 Thread Bernd Eckenfels
Looking at the code some more I wonder:

  *if defaultThreadFactoty should use a Thread group for the pool or at 
least for NIO?
  *   If it can skip the security manager check and use InnocousThread in all 
cases (to avoid ThreadLocals - not sure if some encode cache is hurt by it?)

  *   If it can skip the priveledged call as IThread does that itself.

https://github.com/openjdk/jdk/blob/6765f902505fbdd02f25b599f942437cd805cad1/src/java.base/share/classes/sun/nio/ch/ThreadPool.java#L76

--
http://bernd.eckenfels.net

Von: Bernd Eckenfels 
Gesendet: Sunday, October 17, 2021 4:56:17 AM
An: OpenJDK Dev list ; nio-...@openjdk.java.net 

Betreff: InnocuousThread names (was: [8u] RFR 8190482: InnocuousThread creation 
should not require the caller to possess enableContextClassLoaderOverride)

Apropos InnocousThread backporting - I Wonder if we should remove the auto 
threadname infrastructure and only create properly named threads. The generic 
name seems to be rather confusing and it seems it is only used in an NIO Pool, 
where a thread-name should be set, anyway?

https://github.com/openjdk/jdk/blob/6765f902505fbdd02f25b599f942437cd805cad1/src/java.base/share/classes/sun/nio/ch/ThreadPool.java#L86

Gruss
Bernd
--
http://bernd.eckenfels.net

Von: jdk8u-dev  im Auftrag von Zhengyu Gu 

Gesendet: Sunday, October 17, 2021 1:39:12 AM
An: Hohensee, Paul ; jdk8u-dev 
Betreff: Re: [8u] RFR 8190482: InnocuousThread creation should not require the 
caller to possess enableContextClassLoaderOverride

Thanks, Paul

-Zhengyu


On 10/15/21 17:30, Hohensee, Paul wrote:
> Lgtm.
>
> Thanks,
> Paul
>
> -Original Message-
> From: jdk8u-dev  on behalf of Zhengyu Gu 
> 
> Date: Tuesday, October 5, 2021 at 8:12 AM
> To: jdk8u-dev 
> Subject: [8u] RFR 8190482: InnocuousThread creation should not require the 
> caller to possess enableContextClassLoaderOverride
>
> This backport is for parity with Oracle 8u321.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8190482
> Webrev: http://hg.openjdk.java.net/jdk/jdk/rev/5e7cf99b1303
>
> The original patch does not apply cleanly. There is not newThread(String
> name, Runnable target) method in 8u, so discard this part of change.
>
>
> 8u webrev: http://cr.openjdk.java.net/~zgu/JDK-8190482-8u/webrev.00/
>
>
> Thanks,
>
> -Zhengyu
>
>



InnocuousThread names (was: [8u] RFR 8190482: InnocuousThread creation should not require the caller to possess enableContextClassLoaderOverride)

2021-10-16 Thread Bernd Eckenfels
Apropos InnocousThread backporting - I Wonder if we should remove the auto 
threadname infrastructure and only create properly named threads. The generic 
name seems to be rather confusing and it seems it is only used in an NIO Pool, 
where a thread-name should be set, anyway?

https://github.com/openjdk/jdk/blob/6765f902505fbdd02f25b599f942437cd805cad1/src/java.base/share/classes/sun/nio/ch/ThreadPool.java#L86

Gruss
Bernd
--
http://bernd.eckenfels.net

Von: jdk8u-dev  im Auftrag von Zhengyu Gu 

Gesendet: Sunday, October 17, 2021 1:39:12 AM
An: Hohensee, Paul ; jdk8u-dev 
Betreff: Re: [8u] RFR 8190482: InnocuousThread creation should not require the 
caller to possess enableContextClassLoaderOverride

Thanks, Paul

-Zhengyu


On 10/15/21 17:30, Hohensee, Paul wrote:
> Lgtm.
>
> Thanks,
> Paul
>
> -Original Message-
> From: jdk8u-dev  on behalf of Zhengyu Gu 
> 
> Date: Tuesday, October 5, 2021 at 8:12 AM
> To: jdk8u-dev 
> Subject: [8u] RFR 8190482: InnocuousThread creation should not require the 
> caller to possess enableContextClassLoaderOverride
>
> This backport is for parity with Oracle 8u321.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8190482
> Webrev: http://hg.openjdk.java.net/jdk/jdk/rev/5e7cf99b1303
>
> The original patch does not apply cleanly. There is not newThread(String
> name, Runnable target) method in 8u, so discard this part of change.
>
>
> 8u webrev: http://cr.openjdk.java.net/~zgu/JDK-8190482-8u/webrev.00/
>
>
> Thanks,
>
> -Zhengyu
>
>



Re: RFR: 8271820: Implementation of JEP 416: Reimplement Core Reflection with Method Handle [v13]

2021-10-16 Thread Mandy Chung
On Fri, 15 Oct 2021 15:09:23 GMT, Peter Levart  wrote:

>> It's not driven by performance data.   It's part of Peter's contribution.   
>> I also prefer it without the packing.   I propose to keep the parameter 
>> count as a separate field and examine it when there is footprint issue.
>
> The reason for this packing is (was) ORing the value with a non-zero value so 
> that field never held zero value. When for example an individual value (say 
> paramCount) is used in a separate @Stable field, its value set may include 
> the default value (zero) which is then not optimized by JIT as a constant. 
> This is from @Stable docs:
> 
>  * By extension, any variable (either array or field) which has annotated
>  * as stable is called a stable variable, and its non-null or non-zero
>  * value is called a stable value.
> 
> ...and:
> 
>  * The HotSpot VM relies on this annotation to promote a non-null (resp.,
>  * non-zero) component value to a constant, thereby enabling superior
>  * optimizations of code depending on such a value (such as constant folding).
>  * More specifically, the HotSpot VM will process non-null stable fields 
> (final
>  * or otherwise) in a similar manner to static final fields with respect to
>  * promoting the field's value to a constant.  Thus, placing aside the
>  * differences for null/non-null values and arrays, a final stable field is
>  * treated as if it is really final from both the Java language and the 
> HotSpot
> 
> So now that some of these fields are final and not annotated with @Stable any 
> more but are treated as "trusted final" fields, the question is whether JIT 
> is treating the default (zero, null) values differently or not. If they are 
> treated as static final fields where default value makes no difference, then 
> it's ok to split this multi-valued field into individual fields.

The compiler team confirms that the zero value only matters for stable fields.  
 For static/trusted non-static final fields, zero value is treated as a 
constant.

-

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


Re: SourceVersion::feature

2021-10-16 Thread Michael Bien

thank you! I replied on the PR.
-michael

On 16.10.21 01:19, Joseph D. Darcy wrote:

PS See https://github.com/openjdk/jdk/pull/5973

-Joe

On 10/14/2021 1:53 PM, Joseph D. Darcy wrote:

On 10/14/2021 10:23 AM, Michael Bien wrote:

is this the right mailing list for javax.lang.model.* discussions?



The compiler-dev list would be appropriate as well, but core-libs 
will work.


First, I understand the desire for a method like this. One of the 
potential issues is SourceVersion models language versions and while, 
to date, there has been a simple linear progression, that is not 
guaranteed to be case arbitrarily far into the future, even though it 
is the most likely outcome.


Since java.lang.Runtime.Version is in the platform now, I think this 
request would be better satisfied via a


    public static SourceVersion valueOf(Runtime.Version version)

method on SourceVersion. That way the modeling of SourceVersion can 
absorb any non-linearity in versioning and presumably provide 
sufficient information for a  Runtime.Version -> SourceVersion 
mapping to query.


I've filed the RFE

    JDK-8275308: Add valueOf(Runtime.Version) factory to SourceVersion

Cheers,

-Joe




if yes:

instead of adding
    /**
 * Returns the version as int representing the feature release.
 * @see Runtime.Version#feature()
 */
    public int feature() {
    return this.ordinal();
    }
to SourceVersion.

a note could be added to the doc that the ordinal can be used as 
feature version. Since this statement would apply to the past too, 
the note could be backported to all maintained JDKs. This comes with 
the usual downside of not being able to add enums for in-between 
versions in future.


(doing both would be an option too of course)


To not use the ordinal, client code has to go through some enum init 
rituals to be able to do version comparisons:


   final static SOURCE_VERSION_RELEASE_18;
   static {
SourceVersion tmp18;
    try {
    tmp18 = SourceVersion.valueOf("RELEASE_18");
    } catch (IllegalArgumentException ex) {
tmp18 = null;
    }
    SOURCE_VERSION_RELEASE_18 = tmp18;
    //... more versions
   }
just to be able to

    if (SOURCE_VERSION_RELEASE_18 != null && 
version.compareTo(SOURCE_VERSION_RELEASE_18) >= 0) {}


or

    if 
(Integer.parseInt(version.name().substring(version.name().indexOf('_')+1)) 
>= 18) {}


which is shorter but not a good solution either.

what the client code actually wants is:

    if (SourceVersion.latest().feature() >= 18) {}


it was a wise decision for java.lang.Runtime.Version to not use 
enums :)


best regards,
michael



On 09.10.21 20:58, Michael Bien wrote:

Hello,

could javax.lang.model.SourceVersion receive a feature() method 
returning the version as an int, analog to java.lang.Runtime.Version?


if (SourceVersion.latest().feature() >= 18) {}

is simpler than comparing enums which may or may not exist 
dependent on the deployed java version and cleaner than ordinal() 
tricks.


best regards,

michael





Re: RFR: 8266936: Add a finalization JFR event [v18]

2021-10-16 Thread Markus Grönlund
> Greetings,
> 
> Object.finalize() was deprecated in JDK9. There is an ongoing effort to 
> replace and mitigate Object.finalize() uses in the JDK libraries; please see 
> https://bugs.openjdk.java.net/browse/JDK-8253568 for more information. 
> 
> We also like to assist users in replacing and mitigating uses in non-JDK code.
> 
> Hence, this changeset adds a periodic JFR event to help identify which 
> classes are overriding Object.finalize().
> 
> Thanks
> Markus

Markus Grönlund has updated the pull request incrementally with one additional 
commit since the last revision:

  ThreadBlockInVM instead of ThreadToNativeFromVM for sampling exclusion

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4731/files
  - new: https://git.openjdk.java.net/jdk/pull/4731/files/9b418149..d10eb309

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

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

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


Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]

2021-10-16 Thread Mark Sheppard
On Tue, 12 Oct 2021 15:43:24 GMT, Aleksei Efimov  wrote:

>> This change implements a new service provider interface for host name and 
>> address resolution, so that java.net.InetAddress API can make use of 
>> resolvers other than the platform's built-in resolver.
>> 
>> The following API classes are added to `java.net.spi` package to facilitate 
>> this:
>> - `InetAddressResolverProvider` -  abstract class defining a service, and 
>> is, essentially, a factory for `InetAddressResolver` resolvers.
>> - `InetAddressResolverProvider.Configuration ` - an interface describing the 
>> platform's built-in configuration for resolution operations that could be 
>> used to bootstrap a resolver construction, or to implement partial 
>> delegation of lookup operations.
>> - `InetAddressResolver` - an interface that defines methods for the 
>> fundamental forward and reverse lookup operations.
>> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the 
>> characteristics of one forward lookup operation.  
>> 
>> More details in [JEP-418](https://openjdk.java.net/jeps/418).
>> 
>> Testing: new and existing `tier1:tier3` tests
>
> Aleksei Efimov has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add @since tags to new API classes
>  - Add checks and test for empty stream resolver results

Refactor remove Configuration and simplify interface:

In the InetAddressResolver a Configuration abstraction is defined, and this is 
supplied to
a get method of the InetAddressResolverProvider.

The objective is to “inject” some platform configuration into an 
InetAddressResolver. This
consistents of two pieces of platform configuration detail, a builtin resolver 
reference and the hostname.
Why not provide them as parameters to the provider get method and dispense with 
the Configuration
abstraction? Thus simplifying the interface. The hostname is being supplied by 
the getLocalHostName
of an InetAddressImpl. This is essentially a wrapper to the library call 
gethostname. This
could be provided as a static method in InetAddressImpl, called once during 
loadResolver to 
provide the hostname parameter String. The hostname is a static piece of system 
configuration, which
requires a system restart if it were to be changed - so need to provide a 
lambda.

So Suggestion is refector remove Configuration to simplify the interface and 
provide the
BULITIN_RESOLVER and hostname as parameters to the 
InetAddressResolverProvider::get method

-

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


AbstractProcessor overrides process(..) for no reason

2021-10-16 Thread Japris Pogrammer
Hi there, I've seen that javax.annotation.processing.AbstractProcessor
overrides `Processor#process(..)` method without any changes to it [1]:
- it still is abstract
- signature is untouched
- javadoc is untouched
Is this done intentionally or this redundant override can be eliminated?

[1]:
https://github.com/openjdk/jdk/blob/bfcf6a29a16bc12d77a897fbec304868957c3188/src/java.compiler/share/classes/javax/annotation/processing/AbstractProcessor.java#L167-L171


Thanks,
Peter


Re: RFR: 8275063: Implementation of Foreign Function & Memory API (Second incubator) [v6]

2021-10-16 Thread Maurizio Cimadamore
> This PR contains the API and implementation changes for JEP-419 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/419

Maurizio Cimadamore has updated the pull request incrementally with two 
additional commits since the last revision:

 - * use `invokeWithArguments` to simplify new test
 - Add test for liveness check with high-aririty downcalls
   (make sure that if an exception occurs in a downcall because of liveness,
   ref count of other resources are left intact).

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5907/files
  - new: https://git.openjdk.java.net/jdk/pull/5907/files/5ed94c30..a4db81fe

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

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

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


Re: RFR: 8244202: Implementation of JEP 418: Internet-Address Resolution SPI [v3]

2021-10-16 Thread Mark Sheppard
On Tue, 12 Oct 2021 15:43:24 GMT, Aleksei Efimov  wrote:

>> This change implements a new service provider interface for host name and 
>> address resolution, so that java.net.InetAddress API can make use of 
>> resolvers other than the platform's built-in resolver.
>> 
>> The following API classes are added to `java.net.spi` package to facilitate 
>> this:
>> - `InetAddressResolverProvider` -  abstract class defining a service, and 
>> is, essentially, a factory for `InetAddressResolver` resolvers.
>> - `InetAddressResolverProvider.Configuration ` - an interface describing the 
>> platform's built-in configuration for resolution operations that could be 
>> used to bootstrap a resolver construction, or to implement partial 
>> delegation of lookup operations.
>> - `InetAddressResolver` - an interface that defines methods for the 
>> fundamental forward and reverse lookup operations.
>> - `InetAddressResolver.LookupPolicy` - a class whose instances describe the 
>> characteristics of one forward lookup operation.  
>> 
>> More details in [JEP-418](https://openjdk.java.net/jeps/418).
>> 
>> Testing: new and existing `tier1:tier3` tests
>
> Aleksei Efimov has updated the pull request incrementally with two additional 
> commits since the last revision:
> 
>  - Add @since tags to new API classes
>  - Add checks and test for empty stream resolver results

What’s in a name?
I find the method names of the InetAddressResolver interface a bit ambiguous. 
Typically in this DNS problem space
one speaks of lookup to resolve a hostname to its associated addresses and 
reverse lookup
to resolve an IP address to a hostname (or names) associated with it.

For the method names lookupHostname, and lookupAddresses,
and the semantics conveyed in those names, 
I would expect lookupHostname to resolve a hostname, that is, take a hostname 
as parameter and
the return value the addresses, while lookupAddresses I would expect to resolve 
an address to its hostname,
that is, take an address and return a hostname.
However, the current pair of methods names convey the opposite, lookupHostname 
resolves an address and
lookupAddresses resolves a hostname.

I would proffer a method name change to use lookup and reverseLookup
OR to use resolvesHostname and resolveAddress, to resolve a hostname and address
respectively.

Thus, lookupHostname should be renamed to reverseLookup and
lookupAddresses to lookup OR
lookupHostname renamed to resolveAddress and lookupAddresses renamed to 
resolveHostname.

-

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


Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources

2021-10-16 Thread Glavo
Oh, sorry, I found that I misunderstood the closing time. I tried it
in the wrong way, so I came to the wrong conclusion.

Thank you for your reminder.


Re: RFR: 8274412: Add a method to Stream API to consume and close the stream without using try-with-resources

2021-10-16 Thread Remi Forax



- Original Message -
> From: "Glavo" 
> To: "Tagir F.Valeev" 
> Cc: "core-libs-dev" 
> Sent: Samedi 16 Octobre 2021 06:25:40
> Subject: Re: RFR: 8274412: Add a method to Stream API to consume and close 
> the stream without using try-with-resources

> I don't think it is a perfect solution to combine the close operation
> with the terminator operation, because there are similar issues
> in the intermediate operation.
> 
> Consider this use case (for example only):
> 
> try (var stream = Files.list(path)
>.flatMap(dir -> {
>try {
>return Files.list(dir);
>} catch (IOException e) {
>return Stream.empty();
>}
>})) {
>// ...
> }
> 
> It looks closed, but it doesn't. Closing the Stream generated by flatMap
> will not close all member Stream. 

The stream is closed, from the javadoc
"Each mapped stream is closed after its contents have been placed into this 
stream."


> Further consideration is needed to
> deal with the problem of closing the Stream. Perhaps it is a withCleaner
> method that registers the Stream with the cleaner, or other solutions.
> 
> 

regards,
Rémi

> 
> 
> Tagir F.Valeev  于2021年10月4日周一 下午2:52写道:
> 
>> Currently, when the stream holds a resource, it's necessary to wrap it
>> with try-with-resources. This undermines the compact and fluent style of
>> stream API calls. For example, if we want to get the `List` of files inside
>> the directory and timely close the underlying filehandle, we should use
>> something like this:
>>
>>
>> List paths;
>> try (Stream stream = Files.list(Path.of("/etc"))) {
>> paths = stream.toList();
>> }
>> // use paths
>>
>>
>> I suggest to add a new default method to Stream interface named
>> `consumeAndClose`, which allows performing terminal stream operation and
>> closing the stream at the same time. It may look like this:
>>
>>
>> default  R consumeAndClose(Function, ? extends R>
>> function) {
>> Objects.requireNonNull(function);
>> try(this) {
>> return function.apply(this);
>> }
>> }
>>
>>
>> Now, it will be possible to get the list of the files in the fluent manner:
>>
>>
>> List list =
>> Files.list(Path.of("/etc")).consumeAndClose(Stream::toList);
>>
>> -
>>
>> Commit messages:
>>  - Fix tests
>>  - 8274412: Add a method to Stream API to consume and close the stream
>> without using try-with-resources
>>
>> Changes: https://git.openjdk.java.net/jdk/pull/5796/files
>>  Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5796=00
>>   Issue: https://bugs.openjdk.java.net/browse/JDK-8274412
>>   Stats: 140 lines in 5 files changed: 135 ins; 0 del; 5 mod
>>   Patch: https://git.openjdk.java.net/jdk/pull/5796.diff
>>   Fetch: git fetch https://git.openjdk.java.net/jdk
>> pull/5796/head:pull/5796
>>
>> PR: https://git.openjdk.java.net/jdk/pull/5796