Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

[Andrej Golovnin]

Hi Claes,

> http://cr.openjdk.java.net/~redestad/8193128/open.02/

I think you should provide specialised implementations of the
#indexOf() and #lastIndexOf() methods in the classes List12 and ListN.
Using an iterator in List12 is an overkill. But even in ListN using a
simple for loop would be much better. In any case please take look at
the implementation of the #lastIndexOf() method in the
AbstractImmutableList class. It looks like a copy of
AbstractImmutableList#indexOf() and this is wrong.

Best regards,
Andrej Golovnin

Possible bug in StackFrameInfo#getByteCodeIndex?

[David Lloyd]

I was doing some research related to AccessController, and I noted
this code [1] in StackFrameInfo#getByteCodeIndex():

@Override
public int getByteCodeIndex() {
// bci not available for native methods
if (isNativeMethod())
return -1;

return bci;
}

Now bci is of type short, and given the spec of the method, should the
return not be:

return bci & 0x;

instead?  Else it would return wrong values for methods with more than
32767 bytecodes in them.

[1] 
http://hg.openjdk.java.net/jdk/jdk/file/e3b6cb90d7ce/src/java.base/share/classes/java/lang/StackFrameInfo.java#l96

-- 
- DML

Re: RFR(s): 8060192: Add default method Collection.toArray(generator)

[Martin Buchholz]

On Thu, Dec 7, 2017 at 4:47 PM, Stuart Marks 
wrote:

>
>
> On 12/7/17 3:50 PM, Jonathan Bluett-Duncan wrote:
>
> Looking over http://cr.openjdk.java.net/~smarks/reviews/8060192/webrev.2/
>> src/java.base/share/classes/java/util/ArrayList.java.cdiff.html, I'm
>> wondering if the method `ArrayList.toArray(IntFunction)` should have an
>> `@Override` to make it clear that it's overriding the default method in
>> Collection. What do you think?
>>
>
> I guess it wouldn't hurt. I had thought about adding it, but most of the
> methods in ArrayList that override implementations from supertypes *don't*
> have @Override. Looking more closely, it appears that the ones that
> override interface default methods *do* have @Override.
>
> I don't think there's a rule that says that @Override should be used when
> overriding interface default methods but not when overriding
> implementations from a superclass or when implementing an abstract
> interface method. Frankly, I think the JDK is just sloppy here. Most of the
> existing implementations didn't use @Override -- possibly because they were
> introduced before annotations existed -- and later on, whoever implemented
> the overrides of the Java 8 default methods decided to add @Override.
>

Part of it is that Doug and I are not huge fans of @Override.  Is the gain
in safety really worth the added verbosity?

If we do decide to mandate the use of @Override in all of openjdk, then the
errorprone project has a bug pattern check for you, which will provide more
safety yet.

Re: RFR(s): 8060192: Add default method Collection.toArray(generator)

[David Lloyd]

Yes!  It would be even better for the "toArray(Class)" case if
Class had a "getArrayClass()" method which returned the Class,
which would allow:

  return (T[]) Arrays.copyOf(elementData, size, componentType.getArrayClass());

and similar for other array-backed collections.  I never understood
why that method doesn't exist; in fact, am I wrong in understanding
that "Array.newInstance(clazz, 0).getClass()" is effectively the only
way to do this today?

On Thu, Dec 7, 2017 at 7:03 PM, Martin Buchholz  wrote:
> (I'm still trying to love this new API)
>
> The changes to the jsr166 tck are fine.
>
> I'm not convinced that the new implementation for ArrayList is progress.
> The current implementation of toArray(T[]) does
>
> // Make a new array of a's runtime type, but my contents:
> return (T[]) Arrays.copyOf(elementData, size, a.getClass());
>
> which does not have to pay the cost of zeroing the array, so is potentially
> faster.  (depends on whether the VM provides cheap pre-zeroed memory?! what
> does jmh say?)
>
> If we're not getting type safety and not getting significantly better
> performance, all we have is a more natural API.  But toArray(IntFunction)
> also seems not very natural to me, and we'll have to live with the
> toArray(new String[0]) wart forever anyways.  (But is it really so bad?)
>  (Maybe toArray(Class) is actually more natural?)
>
> On Thu, Dec 7, 2017 at 2:58 PM, Stuart Marks 
> wrote:
>
>> [Martin: please review changes to the JSR 166 TCK.]
>>
>> Another updated webrev for this changeset:
>>
>> http://cr.openjdk.java.net/~smarks/reviews/8060192/webrev.2/
>>
>> This includes an override of toArray(IntFunction) in the implementation of
>> Arrays.asList(), as requested by Tagir Valeev.
>>
>> Overrides are also added for various wrapper classes in j.u.Collections.
>> Turns out there's a test test/jdk/java/util/Collections/Wrappers.java
>> that checks to ensure that the wrappers don't inherit any default methods.
>> (This has been a source of bugs in the past.)
>>
>> Significantly, this webrev also includes changes to several tests in the
>> JSR 166 TCK. The issue is that these tests have cases for null handling,
>> where they call
>>
>> coll.toArray(null)
>>
>> to ensure that NPE is thrown. Given that this method is now overloaded:
>>
>> toArray(T[])
>> toArray(IntFunction)
>>
>> passing null is now ambiguous and thus generates a compiler error. I've
>> modified the tests to call toArray((Object[])null) which is a bit of a
>> stopgap. I can't think of anything obviously better to do, though.
>>
>> s'marks
>>



-- 
- DML

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Brian Burkhalter]

On Dec 7, 2017, at 4:33 PM, Stuart Marks  wrote:

> Please review this changeset that introduces a new no-arg method 
> orElseThrow() to Optional, as a preferred alternative to the get() method.

Looks OK to me.

> Corresponding methods are also added to OptionalDouble, Int, and Long.
> 
> The orElseThrow() method is functionally identical to get(). It has some 
> affinity with the existing orElseThrow(exceptionSupplier) method, being 
> equivalent to orElseThrow(NoSuchElementException::new).
> 
> The get() method is functionally unchanged. It is NOT being deprecated, 
> although some wording in the doc has been added to point to orElseThrow() as 
> the "preferred alternative." This is part of a (slow) migration away from 
> Optional.get(), which has an obvious, attractive name that is often misused. 
> These issues have been discussed on this list previously:
> 
>http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040484.html

Quite a discussion. Good thing you opted not to name it 
“Optional.dudeCallIsPresentBeforeCallingMe().”

Brian

Re: RFR(s): 8060192: Add default method Collection.toArray(generator)

[Martin Buchholz]

(I'm still trying to love this new API)

The changes to the jsr166 tck are fine.

I'm not convinced that the new implementation for ArrayList is progress.
The current implementation of toArray(T[]) does

// Make a new array of a's runtime type, but my contents:
return (T[]) Arrays.copyOf(elementData, size, a.getClass());

which does not have to pay the cost of zeroing the array, so is potentially
faster.  (depends on whether the VM provides cheap pre-zeroed memory?! what
does jmh say?)

If we're not getting type safety and not getting significantly better
performance, all we have is a more natural API.  But toArray(IntFunction)
also seems not very natural to me, and we'll have to live with the
toArray(new String[0]) wart forever anyways.  (But is it really so bad?)
 (Maybe toArray(Class) is actually more natural?)

On Thu, Dec 7, 2017 at 2:58 PM, Stuart Marks 
wrote:

> [Martin: please review changes to the JSR 166 TCK.]
>
> Another updated webrev for this changeset:
>
> http://cr.openjdk.java.net/~smarks/reviews/8060192/webrev.2/
>
> This includes an override of toArray(IntFunction) in the implementation of
> Arrays.asList(), as requested by Tagir Valeev.
>
> Overrides are also added for various wrapper classes in j.u.Collections.
> Turns out there's a test test/jdk/java/util/Collections/Wrappers.java
> that checks to ensure that the wrappers don't inherit any default methods.
> (This has been a source of bugs in the past.)
>
> Significantly, this webrev also includes changes to several tests in the
> JSR 166 TCK. The issue is that these tests have cases for null handling,
> where they call
>
> coll.toArray(null)
>
> to ensure that NPE is thrown. Given that this method is now overloaded:
>
> toArray(T[])
> toArray(IntFunction)
>
> passing null is now ambiguous and thus generates a compiler error. I've
> modified the tests to call toArray((Object[])null) which is a bit of a
> stopgap. I can't think of anything obviously better to do, though.
>
> s'marks
>

Re: RFR(s): 8060192: Add default method Collection.toArray(generator)

[Stuart Marks]

On 12/7/17 3:50 PM, Jonathan Bluett-Duncan wrote:

Looking over 
http://cr.openjdk.java.net/~smarks/reviews/8060192/webrev.2/src/java.base/share/classes/java/util/ArrayList.java.cdiff.html, 
I'm wondering if the method `ArrayList.toArray(IntFunction)` should have an 
`@Override` to make it clear that it's overriding the default method in 
Collection. What do you think?


I guess it wouldn't hurt. I had thought about adding it, but most of the methods 
in ArrayList that override implementations from supertypes *don't* have 
@Override. Looking more closely, it appears that the ones that override 
interface default methods *do* have @Override.


I don't think there's a rule that says that @Override should be used when 
overriding interface default methods but not when overriding implementations 
from a superclass or when implementing an abstract interface method. Frankly, I 
think the JDK is just sloppy here. Most of the existing implementations didn't 
use @Override -- possibly because they were introduced before annotations 
existed -- and later on, whoever implemented the overrides of the Java 8 default 
methods decided to add @Override.


s'marks

RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Stuart Marks]

Hi all,

Please review this changeset that introduces a new no-arg method orElseThrow() 
to Optional, as a preferred alternative to the get() method.


Corresponding methods are also added to OptionalDouble, Int, and Long.

The orElseThrow() method is functionally identical to get(). It has some 
affinity with the existing orElseThrow(exceptionSupplier) method, being 
equivalent to orElseThrow(NoSuchElementException::new).


The get() method is functionally unchanged. It is NOT being deprecated, although 
some wording in the doc has been added to point to orElseThrow() as the 
"preferred alternative." This is part of a (slow) migration away from 
Optional.get(), which has an obvious, attractive name that is often misused. 
These issues have been discussed on this list previously:


http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040484.html

Please note that much of that discussion was about the then-proposed deprecation 
of Optional.get(), which is NOT part of this proposal. There are no plans to 
deprecate Optional.get() at this time. This proposal ONLY introduces a new 
method orElseThrow() that is identical in function to get().


Bug report:

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

Webrev:

http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.1/

Thanks,

s'marks

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

[mandy chung]

On 12/7/17 6:38 AM, Daniel Fuchs wrote:

Hi,

Please find below a patch for:

8191033 Regression in logging.properties: specifying .handlers= for
    root logger (instead of handlers=) no longer works
https://bugs.openjdk.java.net/browse/JDK-8191033

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8191033/webrev.00/


Looks good to me.

Mandy

Re: RFR: jsr166 jdk integration 2017-12-XX

[Chris Hegarty]

++1

-Chris

> On 7 Dec 2017, at 18:03, Paul Sandoz  wrote:
> 
> +1
> 
> Paul.
> 
>> On 7 Dec 2017, at 09:14, Martin Buchholz  wrote:
>> 
>> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html
>> 
>> With the change to the openjdk release model, jsr166 integrations will 
>> simply be "jdk integrations" instead of e.g. "jdk10 integrations" from now 
>> on.  (Nevertheless, we try to get in important bug fixes before jdk release 
>> gates shut)
>> 
>> 8193174: SubmissionPublisher invokes the Subscriber's onComplete before all 
>> of its submitted items have been published
>> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/SubmissionPublisher-race/index.html
>> https://bugs.openjdk.java.net/browse/JDK-8193174
>> 
>> 8192943: Optimize atomic accumulators using VarHandle getAndSet
>> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/getAndSet/index.html
>> https://bugs.openjdk.java.net/browse/JDK-8192943
>> 
>> 8192944: Miscellaneous changes imported from jsr166 CVS 2017-12-XX
>> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/miscellaneous/index.html
>> https://bugs.openjdk.java.net/browse/JDK-8192944
>> 
> 

Re: RFR(s): 8060192: Add default method Collection.toArray(generator)

[Jonathan Bluett-Duncan]

Hi Stuart,

Looking over
http://cr.openjdk.java.net/~smarks/reviews/8060192/webrev.2/src/java.base/share/classes/java/util/ArrayList.java.cdiff.html,
I'm wondering if the method `ArrayList.toArray(IntFunction)` should have an
`@Override` to make it clear that it's overriding the default method in
Collection. What do you think?

Cheers,
Jonathan

On 7 December 2017 at 22:58, Stuart Marks  wrote:

> [Martin: please review changes to the JSR 166 TCK.]
>
> Another updated webrev for this changeset:
>
> http://cr.openjdk.java.net/~smarks/reviews/8060192/webrev.2/
>
> This includes an override of toArray(IntFunction) in the implementation of
> Arrays.asList(), as requested by Tagir Valeev.
>
> Overrides are also added for various wrapper classes in j.u.Collections.
> Turns out there's a test test/jdk/java/util/Collections/Wrappers.java
> that checks to ensure that the wrappers don't inherit any default methods.
> (This has been a source of bugs in the past.)
>
> Significantly, this webrev also includes changes to several tests in the
> JSR 166 TCK. The issue is that these tests have cases for null handling,
> where they call
>
> coll.toArray(null)
>
> to ensure that NPE is thrown. Given that this method is now overloaded:
>
> toArray(T[])
> toArray(IntFunction)
>
> passing null is now ambiguous and thus generates a compiler error. I've
> modified the tests to call toArray((Object[])null) which is a bit of a
> stopgap. I can't think of anything obviously better to do, though.
>
> s'marks
>

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

[Claes Redestad]

On 2017-12-07 01:12, Claes Redestad wrote:
SubList is now final, inherits from AbstractImmutableList instead of 
AbstractList and reuses more of the shared code.


I also moved 'implements Serializable' from AbstractImmutableList to 
List12 and ListN respectively to not
change the behavior that List.of(0,1) is serializable while 
List.of(0,1).subList(0,1) isn't.


While doing this, I realized a few issues with my patch around the 
subList mechanics:


- I had "optimized" AbstractImmutable::subList to return self or 
emptyList if appropriate, which sounded nice, but is in fact an 
incompatible change (some subLists become Serializable).
- the ListFactories test didn't explicitly verify that sublists 
retrieved from various List.of() impls aren't Serializable. Added tests 
to check no sub-list is Serializable,
and as a bonus this test now verifies that List.of(...).subList(...) 
behave like their List.of(..) counterparts in most other ways.
- fixed range check in AbstractImmutableList.listIterator(0) to not 
throw IOOBE if the list is empty


http://cr.openjdk.java.net/~redestad/8193128/open.02/

Thanks!

/Claes

Re: RFR 4358774: Add null InputStream and OutputStream

[Brian Burkhalter]

Hi Sergey,

On Dec 7, 2017, at 3:14 PM, Sergey Bylokhov  wrote:

>> http://cr.openjdk.java.net/~bpb/4358774/webrev.02/.
> 
> Why the
>Objects.requireNonNull(b);
> should be called before
>Objects.checkFromIndexSize(off, len, b.length);
> since the second call will gen NPW at b.length anyway?

Good point. I’ll update after I see whether any other updates will be needed. I 
think this situation probably obtains in some other files as well.

Thanks,

Brian

Re: RFR 4358774: Add null InputStream and OutputStream

[Sergey Bylokhov]

On 07/12/2017 14:55, Brian Burkhalter wrote:

Hi Roger,

I agree. It does not seem that whatever performance improvement might accrue 
from not using the Objects methods is offset by the increased code readability 
in addition to mitigating the risks mentioned in [1]. I have reinstated this 
approach in http://cr.openjdk.java.net/~bpb/4358774/webrev.02/.


Why the
Objects.requireNonNull(b);
should be called before
Objects.checkFromIndexSize(off, len, b.length);
since the second call will gen NPW at b.length anyway?



Thanks,

Brian

On Dec 7, 2017, at 2:04 PM, Roger Riggs  wrote:



For checking indices, I think you should leverage the work done for 
java.util.Objects.checkFromIndexSize(...)
as optimized for this purpose in 8135248.  That was extensively designed and 
reviewed for optimal performance.

Regards, Roger

[1] https://bugs.openjdk.java.net/browse/JDK-8135248





--
Best regards, Sergey.

Re: RFR(s): 8060192: Add default method Collection.toArray(generator)

[Stuart Marks]

[Martin: please review changes to the JSR 166 TCK.]

Another updated webrev for this changeset:

http://cr.openjdk.java.net/~smarks/reviews/8060192/webrev.2/

This includes an override of toArray(IntFunction) in the implementation of 
Arrays.asList(), as requested by Tagir Valeev.


Overrides are also added for various wrapper classes in j.u.Collections. Turns 
out there's a test test/jdk/java/util/Collections/Wrappers.java that checks to 
ensure that the wrappers don't inherit any default methods. (This has been a 
source of bugs in the past.)


Significantly, this webrev also includes changes to several tests in the JSR 166 
TCK. The issue is that these tests have cases for null handling, where they call


coll.toArray(null)

to ensure that NPE is thrown. Given that this method is now overloaded:

toArray(T[])
toArray(IntFunction)

passing null is now ambiguous and thus generates a compiler error. I've modified 
the tests to call toArray((Object[])null) which is a bit of a stopgap. I can't 
think of anything obviously better to do, though.


s'marks

Re: RFR 4358774: Add null InputStream and OutputStream

[Brian Burkhalter]

Hi Roger,

I agree. It does not seem that whatever performance improvement might accrue 
from not using the Objects methods is offset by the increased code readability 
in addition to mitigating the risks mentioned in [1]. I have reinstated this 
approach in http://cr.openjdk.java.net/~bpb/4358774/webrev.02/.

Thanks,

Brian

On Dec 7, 2017, at 2:04 PM, Roger Riggs  wrote:


> For checking indices, I think you should leverage the work done for 
> java.util.Objects.checkFromIndexSize(...)
> as optimized for this purpose in 8135248.  That was extensively designed and 
> reviewed for optimal performance.
> 
> Regards, Roger
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8135248

Re: RFR 4358774: Add null InputStream and OutputStream

[Roger Riggs]

Hi Brian,

For checking indices, I think you should leverage the work done for 
java.util.Objects.checkFromIndexSize(...)
as optimized for this purpose in 8135248.  That was extensively designed 
and reviewed for optimal performance.


Regards, Roger

[1] https://bugs.openjdk.java.net/browse/JDK-8135248

On 12/7/2017 2:27 PM, Brian Burkhalter wrote:

Updated patch:

http://cr.openjdk.java.net/~bpb/4358774/webrev.01/ 



On Dec 7, 2017, at 6:08 AM, Alan Bateman > wrote:


If nothing else, a private ensureOpen method would make it easier to 
read so that the "if (closed) throw ..." isn't needed in every method.


Done.

On Dec 6, 2017, at 7:03 PM, Vitaly Davidovich > wrote:


From a performance angle, I'd be more concerned with the calls to 
Objects.xyz() methods there. Unless something has changed in the JIT 
recently, those are susceptible to profile pollution and can cause 
missed optimizations.  I'd inline those methods manually to give 
these methods their own profiles.


Done.

Thanks,

Brian

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

[John Rose]

On Dec 7, 2017, at 12:05 PM, Remi Forax  wrote:
> 
> have a bit saying if an object is fully initialized or not, this bit will be 
> true at the end of the constructor and can be set for the de-serialization too

Yes, that's the hard part of fixing final.  Sometimes we call this the "slushy 
bit".
It would almost always be false, but would be true for those unusual objects
which are in the process of deserialization, and also (perhaps) objects which
may escape into the optimizer while undergoing normal construction. The
latter version of "slushy" is probably a statically computable condition,
unlike "slushy during deserialization".

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

[Remi Forax]

You have also the problem of people doing a lot of things in the constructor, 
by example

class Foo {
  @Stable final int x;

  Foo() {
m();
x = 3;
m();
  }

  void m() {
for(...) {
  // use x here
}
  }
}

here, m() can be JITed with x equals to 0 as a constant and the code of m() be 
re-use for the second call even if the value of x has changed in the middle.
in that case, either you need to maintain dependency between the JITed code and 
the field 'x' or have a bit saying if an object is fully initialized or not, 
this bit will be true at the end of the constructor and can be set for the 
de-serialization too.

Rémi

- Mail original -
> De: "John Rose" 
> À: "Paul Sandoz" 
> Cc: "core-libs-dev" 
> Envoyé: Jeudi 7 Décembre 2017 20:50:01
> Objet: Re: [10?] RFR: 8193128: Reduce number of implementation classes 
> returned by List/Set/Map.of()

>> On Dec 6, 2017, at 5:16 PM, Paul Sandoz  wrote:
>> 
>> It can, since final fields are not treated as really final (unless in
>> java.lang.invoke package, where it’s as if all such fields are annotated with
>> @Stable). C2 will treat the field value a constant value if the collection is
>> held in a static final field.
> 
> We could tweak the semantics of @Stable to omit the zero corner case for a 
> final
> field. Then the annotation on a non-array field would mean “trust this final”.
> 
> It would be yet another stop gap before that far-off day when we fix finals 
> (and
> arrays). Such new uses of @Stable would have to be evaluated for any
> interaction with deserialization, so it’s not a simple decision.

Draft JEP: Enhanced pseudo-random number generators

[Guy Steele]

Please check out a new draft JEP 
https://bugs.openjdk.java.net/browse/JDK-8193209 
 that proposes new interface 
types and implementations for pseudo-random number generators.

—Guy

Re: [10?] RFR: 8193128: Reduce number of implementation classes returned by List/Set/Map.of()

[John Rose]

> On Dec 6, 2017, at 5:16 PM, Paul Sandoz  wrote:
> 
> It can, since final fields are not treated as really final (unless in 
> java.lang.invoke package, where it’s as if all such fields are annotated with 
> @Stable). C2 will treat the field value a constant value if the collection is 
> held in a static final field.

We could tweak the semantics of @Stable to omit the zero corner case for a 
final field. Then the annotation on a non-array field would mean “trust this 
final”.

It would be yet another stop gap before that far-off day when we fix finals 
(and arrays). Such new uses of @Stable would have to be evaluated for any 
interaction with deserialization, so it’s not a simple decision. 

Re: RFR 4358774: Add null InputStream and OutputStream

[Brian Burkhalter]

Updated patch:

http://cr.openjdk.java.net/~bpb/4358774/webrev.01/

On Dec 7, 2017, at 6:08 AM, Alan Bateman  wrote:

> If nothing else, a private ensureOpen method would make it easier to read so 
> that the "if (closed) throw ..." isn't needed in every method.

Done.

On Dec 6, 2017, at 7:03 PM, Vitaly Davidovich  wrote:

> From a performance angle, I'd be more concerned with the calls to 
> Objects.xyz() methods there.  Unless something has changed in the JIT 
> recently, those are susceptible to profile pollution and can cause missed 
> optimizations.  I'd inline those methods manually to give these methods their 
> own profiles.

Done.

Thanks,

Brian

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

[Martin Buchholz]

I'm not a logging expert, but this change Looks Good To Me.

---

A bunch of "handler" should be changed to "handlers"

+// Verify that exactly one of the two handler is a custom.Handler
+// Verify that exactly one of the two handler is a
custom.DotHandler
+// Verify that the two handler have an id of '1'

--

This code makes me think you use the identity of the Longs, but it seems
you don't, so I would just get rid of these and use e.g. 3L instead of
THREE.

+public static final Long ONE = 1L;
+public static final Long TWO = 2L;
+public static final Long THREE = 3L;

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

[Martin Buchholz]

You went to the trouble of checking test.src.  Shouldn't  CONFIG_FILE be
found relative to SRC_DIR?

+public static final Path SRC_DIR =
+Paths.get(System.getProperty("test.src", "src"));
+public static final Path USER_DIR =
+Paths.get(System.getProperty("user.dir", "."));
+public static final Path CONFIG_FILE = Paths.get("logging.properties");

Re: Finalization and dead references: another proposal

[Peter Levart]

On 12/07/2017 07:22 PM, Peter Levart wrote:



On 12/07/2017 06:46 PM, Vitaly Davidovich wrote:


So no magic here. Just API.

This is an API version of Hans’s #3 approach. As he said, there’s 
performance overhead and nothing guarantees that the referent is kept 
alive - that’s an implementation artifact.


I think without the VM knowing about these things intrinsically it’s 
not a 100% reliable solution because it’s not concretely requesting a 
certain behavior.


I would say that for JNI "there’s performance overhead XOR nothing 
guarantees that the referent is kept alive", because the overhead is 
caused by reference parameter management that guarantees that the 
referents are kept alive while the native method executes and may 
access them. Can we live with such overhead? I don't know. Would have 
to measure if it really presents a problem.


The magic would have to be performed by intrinsified method(s) (Unsafe 
for example), but they are just few and developed by select group of 
developers.


The main problem I think is not the overhead of passing referents 
together with addresses to JNI functions and their overheads, but lack 
of enforcement for this to be performed consistently. It's just too 
easy to split the native resource from the referent that keeps it into 
two parts with each having separate lifetime. API with value types 
could enforce such pairs (address, referent) to be kept together until 
they hit the final leaf operation which is typically a native method 
where the referent(s) are either automatically kept alive or would 
have to be manually, but such methods are in minority.


Regards, Peter



There's a quick solution to get rid of that overhead. Each native method 
(or intrinsified leaf method such as those on Unsafe) can have a java 
front end. They are in minority and kept under control. Similar to 
argument validation front-ends for intrinsified methods. For example:


public class Unsafe {

    public Address allocateMemory(long bytes, Object referent) {
        return Address.create(allocateMemory(bytes), referent);
    }

    public void setMemory(Address address, long bytes, byte value) {
        setMemory(/* access check absent equivalent of */ 
address.address, bytes, value);
        Reference.reachabilityFence(/* access check absent equivalent 
of */ address.referent);

    }
    ...


Regards, Peter

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

[Martin Buchholz]

On Thu, Dec 7, 2017 at 11:00 AM, Martin Buchholz 
wrote:

> You went to the trouble of checking test.src.  Shouldn't  CONFIG_FILE be
> found relative to SRC_DIR?
>
>
Oh never mind, I now see you did

+Path initialProps = SRC_DIR.resolve(CONFIG_FILE);
+Path loggingProps = USER_DIR.resolve(CONFIG_FILE);



> +public static final Path SRC_DIR =
> +Paths.get(System.getProperty("test.src", "src"));
> +public static final Path USER_DIR =
> +Paths.get(System.getProperty("user.dir", "."));
> +public static final Path CONFIG_FILE = Paths.get("logging.properties"
> );
>
>

Re: Finalization and dead references: another proposal

[Peter Levart]

Hi Remi,

On 12/07/2017 07:12 PM, Remi Forax wrote:

So no magic here. Just API.

This is an API version of Hans’s #3 approach.  As he said, there’s
performance overhead and nothing guarantees that the referent is kept alive
- that’s an implementation artifact.

it's not even true, the implementation do not even guarantee that the referent 
field will be kept on stack,
a value type can disappear completely if its field are not used,
so without a reachabilityFence somewhere, it will not work.



What do you mean by saying "the implementation do not even guarantee 
that the referent field will be kept on stack".


If a referent is passed through a reference typed parameter to the JNI 
method (there's no other way actually), then such referent must be kept 
alive for the entire time the native method executes, because native 
method may call methods or access fields on such referent. Because 
there's no cross-native-baundary optimization going on currently, JNI 
must guarantee that. If/when there will be cross-native-baundary 
optimization, I suppose native side would have to have an equivalent to 
reachabilityFence too ...


Regards, Peter

Re: RFR(XXS): 8182307 - Error during JRMP connection establishment

[Daniel D. Daugherty]

On 12/7/17 1:08 PM, serguei.spit...@oracle.com wrote:

Hi Dan,

The fix looks good to me.
Nice, you have caught it.


Thanks for the review!



Do you want this fixed in 10 or 11?
I thought that the jdk/hs is for 11 now.
Is it correct?


Please see Mark R's e-mail with the subject line of
"JDK 10 enters Rampdown Phase One in one week". In short,
the RDP1 deadline applies to jdk/jdk, jdk/hs and jdk/client
all at the same time.

Dan




Thanks,
Serguei


On 12/7/17 09:09, Daniel D. Daugherty wrote:

Roger,

Thanks for the review!

Dan

P.S.
I'm planning to push this fix to jdk/hs since the only sightings
have been in jdk/hs testing or in projects that are parented to
jdk/hs repos... Hope that's okay...


On 12/7/17 12:07 PM, Roger Riggs wrote:

+1

On 12/7/2017 12:00 PM, Gerald Thornbrugh wrote:

Hi Dan,

Your fix looks good.

Jerry


Adding core-libs-dev@... since this is RMI code. Thanks Alan!

Folks, this review spans three OpenJDK aliases so Thunderbird's
reply-to-list feature won't get it right. This is one of the
few times that reply-to-all is the right thing to do...

Dan

On 12/7/17 11:38 AM, Daniel D. Daugherty wrote:

Greetings,

I have a small fix for a very intermittent ServerSocket related test
failure:

    JDK-8182307: Error during JRMP connection establishment
https://bugs.openjdk.java.net/browse/JDK-8182307

The fix is copied from Jerry's work on the following bugs:

    JDK-8182757 JDWP: Socket Transport handshake hangs on Solaris
https://bugs.openjdk.java.net/browse/JDK-8182757

    JDK-8178676 nsk/jvmti/AttachOnDemand/attach045 fails with 
Exception

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

For the gory details of the reasons for this fix please see
Jerry's bugs.

Webrev URL: 
http://cr.openjdk.java.net/~dcubed/8182307-webrev/jdk10-0/



I observed this test failure one time in my Thread-SMR work and have
been including this fix in months of Thread-SMR stress testing. 
It was
also included in both JPRT and Mach5 tier[1-5] testing of the 
Thread-SMR

changes.

Thanks, in advance, for any comments, suggestions or questions.

Dan

Re: RFR(XXS): 8182307 - Error during JRMP connection establishment

[serguei.spit...@oracle.com]

On 12/7/17 10:08, serguei.spit...@oracle.com wrote:

Hi Dan,

The fix looks good to me.
Nice, you have caught it.

Do you want this fixed in 10 or 11?
I thought that the jdk/hs is for 11 now.
Is it correct?


Never mind.
I've just found a message from Jesper the jdk/hs is used for 10 pushes 
for one more week.


Thanks,
Serguei



Thanks,
Serguei


On 12/7/17 09:09, Daniel D. Daugherty wrote:

Roger,

Thanks for the review!

Dan

P.S.
I'm planning to push this fix to jdk/hs since the only sightings
have been in jdk/hs testing or in projects that are parented to
jdk/hs repos... Hope that's okay...


On 12/7/17 12:07 PM, Roger Riggs wrote:

+1

On 12/7/2017 12:00 PM, Gerald Thornbrugh wrote:

Hi Dan,

Your fix looks good.

Jerry


Adding core-libs-dev@... since this is RMI code. Thanks Alan!

Folks, this review spans three OpenJDK aliases so Thunderbird's
reply-to-list feature won't get it right. This is one of the
few times that reply-to-all is the right thing to do...

Dan

On 12/7/17 11:38 AM, Daniel D. Daugherty wrote:

Greetings,

I have a small fix for a very intermittent ServerSocket related test
failure:

    JDK-8182307: Error during JRMP connection establishment
https://bugs.openjdk.java.net/browse/JDK-8182307

The fix is copied from Jerry's work on the following bugs:

    JDK-8182757 JDWP: Socket Transport handshake hangs on Solaris
https://bugs.openjdk.java.net/browse/JDK-8182757

    JDK-8178676 nsk/jvmti/AttachOnDemand/attach045 fails with 
Exception

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

For the gory details of the reasons for this fix please see
Jerry's bugs.

Webrev URL: 
http://cr.openjdk.java.net/~dcubed/8182307-webrev/jdk10-0/



I observed this test failure one time in my Thread-SMR work and have
been including this fix in months of Thread-SMR stress testing. 
It was
also included in both JPRT and Mach5 tier[1-5] testing of the 
Thread-SMR

changes.

Thanks, in advance, for any comments, suggestions or questions.

Dan

Re: Finalization and dead references: another proposal

[Vitaly Davidovich]

On Thu, Dec 7, 2017 at 1:22 PM, Peter Levart  wrote:

>
>
> On 12/07/2017 06:46 PM, Vitaly Davidovich wrote:
>
> So no magic here. Just API.
>
> This is an API version of Hans’s #3 approach.  As he said, there’s
> performance overhead and nothing guarantees that the referent is kept alive
> - that’s an implementation artifact.
>
> I think without the VM knowing about these things intrinsically it’s not a
> 100% reliable solution because it’s not concretely requesting a certain
> behavior.
>
>
> I would say that for JNI "there’s performance overhead XOR nothing
> guarantees that the referent is kept alive", because the overhead is caused
> by reference parameter management that guarantees that the referents are
> kept alive while the native method executes and may access them. Can we
> live with such overhead? I don't know. Would have to measure if it really
> presents a problem.
>
I'd say the bigger overhead is in JNI argument marshaling.  But JNI is
already costly so I don't know.

>
> The magic would have to be performed by intrinsified method(s) (Unsafe for
> example), but they are just few and developed by select group of developers.
>
> The main problem I think is not the overhead of passing referents together
> with addresses to JNI functions and their overheads, but lack of
> enforcement for this to be performed consistently. It's just too easy to
> split the native resource from the referent that keeps it into two parts
> with each having separate lifetime. API with value types could enforce such
> pairs (address, referent) to be kept together until they hit the final leaf
> operation which is typically a native method where the referent(s) are
> either automatically kept alive or would have to be manually, but such
> methods are in minority.
>
I agree that the main problem is how to not "detach" these things by
accident.  To that end, you want to bundle them together as tightly as
possible, and prevent de-encapsulation by the users (or at least make it
harder and more explicit).  This is why I threw value types out there since
I think they might provide a better solution than an annotation, which I
suspect will be very easy to accidentally circumvent.

But ultimately, you really want the JVM to understand about "foreign
resources/pointers" in a 1st class/principled manner, which suggests a type
based approach (I'm assuming a language approach is off the table :)).
Since this type would be requesting special handling by the JVM, it's hard
to make it library/API only.  Or rather, it would be hard, if not
impossible, to make it library/API only but then not suffer performance
consequences.  You also don't want to accidentally omit or put the
reachabilityFence() in the wrong place or on the wrong object, both things
possible with one careless refactoring (gone wrong).

>
> Regards, Peter
>
>

Re: Finalization and dead references: another proposal

[Peter Levart]

On 12/07/2017 06:46 PM, Vitaly Davidovich wrote:


So no magic here. Just API.

This is an API version of Hans’s #3 approach.  As he said, there’s 
performance overhead and nothing guarantees that the referent is kept 
alive - that’s an implementation artifact.


I think without the VM knowing about these things intrinsically it’s 
not a 100% reliable solution because it’s not concretely requesting a 
certain behavior.


I would say that for JNI "there’s performance overhead XOR nothing 
guarantees that the referent is kept alive", because the overhead is 
caused by reference parameter management that guarantees that the 
referents are kept alive while the native method executes and may access 
them. Can we live with such overhead? I don't know. Would have to 
measure if it really presents a problem.


The magic would have to be performed by intrinsified method(s) (Unsafe 
for example), but they are just few and developed by select group of 
developers.


The main problem I think is not the overhead of passing referents 
together with addresses to JNI functions and their overheads, but lack 
of enforcement for this to be performed consistently. It's just too easy 
to split the native resource from the referent that keeps it into two 
parts with each having separate lifetime. API with value types could 
enforce such pairs (address, referent) to be kept together until they 
hit the final leaf operation which is typically a native method where 
the referent(s) are either automatically kept alive or would have to be 
manually, but such methods are in minority.


Regards, Peter

Re: Finalization and dead references: another proposal

[Vitaly Davidovich]

On Thu, Dec 7, 2017 at 1:12 PM, Remi Forax  wrote:

> - Mail original -
> > De: "Vitaly Davidovich" 
> > À: "Peter Levart" 
> > Cc: "core-libs-dev" 
> > Envoyé: Jeudi 7 Décembre 2017 18:46:41
> > Objet: Re: Finalization and dead references: another proposal
>
> > On Thu, Dec 7, 2017 at 12:28 PM Peter Levart 
> wrote:
> >
> >> Hi,
> >>
> >> On 12/07/2017 03:27 AM, Vitaly Davidovich wrote:
> >> > So kind of the opposite of WeakReference - a SuperStrongReference :).
> >> >
> >> > Kidding aside, it seems like the way you’d want to encapsulate this at
> >> the
> >> > language level is via a type that the JVM intrinsically knows about;
> in
> >> > this way it’s similar to the reference types today.
> >> >
> >> > An annotation probably does the trick when the value doesn’t escape
> from
> >> > the enclosing instance but I’ve no idea if that assumption covers
> enough
> >> > code to warrant this approach.  AFAICT, if the value escapes into an
> >> > instance of another type that doesn’t annotate its field then all bets
> >> are
> >> > off.
> >> >
> >> > Having a wrapper type would at least make it harder to leak the
> native
> >> > handle vs the annotation approach.  But of course the wrapper comes
> with
> >> > footprint and indirection costs.  Maybe Valhalla could allow exposing
> >> some
> >> > magic value type that’s a zero-cost wrapper but preserves the type
> >> > information the JIT can track?
> >>
> >> There is a middle-ground. By (ab)using value types only and no special
> >> JIT magic.
> >>
> >> DirectBuffer(s) for example, could return the address not in a long, but
> >> in a value type like the following (using valhalla MVT speak):
> >>
> >> public __ByValue final class Address {
> >>  private final long address;
> >>  private final Object referent;
> >>
> >>  public _ValueFactory static Address create(long address, Object
> >> referent) {
> >>  Address a = __MakeDefault Address();
> >>  a.address = address;
> >>  a.referent = referent;
> >>  return a;
> >>  }
> >> }
> >>
> >>
> >> DirectByteBuffer for example, would have the following address() method:
> >>
> >> private long address;
> >>
> >> public Address address() {
> >>  return Address.create(address, this);
> >> }
> >>
> >> Notice that 'address' field of Address value is encapsulated, so Java
> >> code can't access it directly nor it needs to. Native / Unsafe methods
> >> would be taking Address values instead of long(s), making sure the
> >> embedded referent is kept reachable.
> >>
> >> This is equivalent to passing the DirectBuffer reference to the native
> >> method(s) together with the address value, but enforced by the API so
> >> the programmer can not make a mistake. There's a lot of arithmetic going
> >> on with addresses, inside and outside of DirectBuffer implementations.
> >> Such arithmetic would have to be performed by the Address value type
> >> itself, making sure the results of operations on Address values are
> >> Address values that maintain the same referent. For example:
> >>
> >> public __ByValue final class Address {
> >> ...
> >>  public _ValueFactory Address plus(long offset) {
> >>  Address a = __MakeDefault Address();
> >>  a.address = address + offset;
> >>  a.referent = referent;
> >>  return a;
> >>  }
> >> ...
> >>
> >> So no magic here. Just API.
> >
> > This is an API version of Hans’s #3 approach.  As he said, there’s
> > performance overhead and nothing guarantees that the referent is kept
> alive
> > - that’s an implementation artifact.
>
> it's not even true, the implementation do not even guarantee that the
> referent field will be kept on stack,
> a value type can disappear completely if its field are not used,
> so without a reachabilityFence somewhere, it will not work.
>
Not sure what you mean here.  Even if the value type is scalar replaced
(which I'd actually hope to be the case for something like this, but that's
an aside), it doesn't change anything - that referent is passed into a
black hole (i.e. the JNI call) that, as of today, the JIT cannot reason
around.  As such, the referent is kept alive across the call.  That's #3
that Hans mentioned.  The value type aspect doesn't change anything other
than not requiring a separate heap object to be the wrapper.

>
> >
> > I think without the VM knowing about these things intrinsically it’s not
> a
> > 100% reliable solution because it’s not concretely requesting a certain
> > behavior.
>
> here is another approach based on the Peter idea,
> what we want is that in order to access to an address, you have to send an
> object that will be kept until the end of the call because the is a call to
> reachabilityFence at the end, so
>
> class DirectByteBuffer {
>   private long address;
>
>   void compute(LongConsumer consumer) {
> 

Re: Finalization and dead references: another proposal

[Remi Forax]

- Mail original -
> De: "Vitaly Davidovich" 
> À: "Peter Levart" 
> Cc: "core-libs-dev" 
> Envoyé: Jeudi 7 Décembre 2017 18:46:41
> Objet: Re: Finalization and dead references: another proposal

> On Thu, Dec 7, 2017 at 12:28 PM Peter Levart  wrote:
> 
>> Hi,
>>
>> On 12/07/2017 03:27 AM, Vitaly Davidovich wrote:
>> > So kind of the opposite of WeakReference - a SuperStrongReference :).
>> >
>> > Kidding aside, it seems like the way you’d want to encapsulate this at
>> the
>> > language level is via a type that the JVM intrinsically knows about; in
>> > this way it’s similar to the reference types today.
>> >
>> > An annotation probably does the trick when the value doesn’t escape from
>> > the enclosing instance but I’ve no idea if that assumption covers enough
>> > code to warrant this approach.  AFAICT, if the value escapes into an
>> > instance of another type that doesn’t annotate its field then all bets
>> are
>> > off.
>> >
>> > Having a wrapper type would at least make it harder to leak the  native
>> > handle vs the annotation approach.  But of course the wrapper comes with
>> > footprint and indirection costs.  Maybe Valhalla could allow exposing
>> some
>> > magic value type that’s a zero-cost wrapper but preserves the type
>> > information the JIT can track?
>>
>> There is a middle-ground. By (ab)using value types only and no special
>> JIT magic.
>>
>> DirectBuffer(s) for example, could return the address not in a long, but
>> in a value type like the following (using valhalla MVT speak):
>>
>> public __ByValue final class Address {
>>  private final long address;
>>  private final Object referent;
>>
>>  public _ValueFactory static Address create(long address, Object
>> referent) {
>>  Address a = __MakeDefault Address();
>>  a.address = address;
>>  a.referent = referent;
>>  return a;
>>  }
>> }
>>
>>
>> DirectByteBuffer for example, would have the following address() method:
>>
>> private long address;
>>
>> public Address address() {
>>  return Address.create(address, this);
>> }
>>
>> Notice that 'address' field of Address value is encapsulated, so Java
>> code can't access it directly nor it needs to. Native / Unsafe methods
>> would be taking Address values instead of long(s), making sure the
>> embedded referent is kept reachable.
>>
>> This is equivalent to passing the DirectBuffer reference to the native
>> method(s) together with the address value, but enforced by the API so
>> the programmer can not make a mistake. There's a lot of arithmetic going
>> on with addresses, inside and outside of DirectBuffer implementations.
>> Such arithmetic would have to be performed by the Address value type
>> itself, making sure the results of operations on Address values are
>> Address values that maintain the same referent. For example:
>>
>> public __ByValue final class Address {
>> ...
>>  public _ValueFactory Address plus(long offset) {
>>  Address a = __MakeDefault Address();
>>  a.address = address + offset;
>>  a.referent = referent;
>>  return a;
>>  }
>> ...
>>
>> So no magic here. Just API.
> 
> This is an API version of Hans’s #3 approach.  As he said, there’s
> performance overhead and nothing guarantees that the referent is kept alive
> - that’s an implementation artifact.

it's not even true, the implementation do not even guarantee that the referent 
field will be kept on stack,
a value type can disappear completely if its field are not used,
so without a reachabilityFence somewhere, it will not work.

> 
> I think without the VM knowing about these things intrinsically it’s not a
> 100% reliable solution because it’s not concretely requesting a certain
> behavior.

here is another approach based on the Peter idea,
what we want is that in order to access to an address, you have to send an 
object that will be kept until the end of the call because the is a call to 
reachabilityFence at the end, so

class DirectByteBuffer {
  private long address;

  void compute(LongConsumer consumer) {
consumer.accept(address);
Reference.reachabilityFence(this);
  }
}


so with the example of Hans,
  private static native void nativeDoSomething(long nativePtr, long 
anotherNativePtr);

a call to nativeDoSomething, is
  buffer1.compute(address1 -> buffer2.compute(address2 -> 
nativeDoSomething(address1, address2)));

so at least in term of API it's doable, the question is how to be sure that the 
JIT will inline the whole thing.

> 
>>
>>
>> Regards, Peter
>>
>>
>> --
> Sent from my phone

cheers,
Rémi

Re: RFR(XXS): 8182307 - Error during JRMP connection establishment

[serguei.spit...@oracle.com]

Hi Dan,

The fix looks good to me.
Nice, you have caught it.

Do you want this fixed in 10 or 11?
I thought that the jdk/hs is for 11 now.
Is it correct?

Thanks,
Serguei


On 12/7/17 09:09, Daniel D. Daugherty wrote:

Roger,

Thanks for the review!

Dan

P.S.
I'm planning to push this fix to jdk/hs since the only sightings
have been in jdk/hs testing or in projects that are parented to
jdk/hs repos... Hope that's okay...


On 12/7/17 12:07 PM, Roger Riggs wrote:

+1

On 12/7/2017 12:00 PM, Gerald Thornbrugh wrote:

Hi Dan,

Your fix looks good.

Jerry


Adding core-libs-dev@... since this is RMI code. Thanks Alan!

Folks, this review spans three OpenJDK aliases so Thunderbird's
reply-to-list feature won't get it right. This is one of the
few times that reply-to-all is the right thing to do...

Dan

On 12/7/17 11:38 AM, Daniel D. Daugherty wrote:

Greetings,

I have a small fix for a very intermittent ServerSocket related test
failure:

    JDK-8182307: Error during JRMP connection establishment
https://bugs.openjdk.java.net/browse/JDK-8182307

The fix is copied from Jerry's work on the following bugs:

    JDK-8182757 JDWP: Socket Transport handshake hangs on Solaris
https://bugs.openjdk.java.net/browse/JDK-8182757

    JDK-8178676 nsk/jvmti/AttachOnDemand/attach045 fails with 
Exception

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

For the gory details of the reasons for this fix please see
Jerry's bugs.

Webrev URL: 
http://cr.openjdk.java.net/~dcubed/8182307-webrev/jdk10-0/



I observed this test failure one time in my Thread-SMR work and have
been including this fix in months of Thread-SMR stress testing. It 
was
also included in both JPRT and Mach5 tier[1-5] testing of the 
Thread-SMR

changes.

Thanks, in advance, for any comments, suggestions or questions.

Dan

Re: RFR: jsr166 jdk integration 2017-12-XX

[Paul Sandoz]

+1

Paul.

> On 7 Dec 2017, at 09:14, Martin Buchholz  wrote:
> 
> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html
>  
> 
> 
> With the change to the openjdk release model, jsr166 integrations will simply 
> be "jdk integrations" instead of e.g. "jdk10 integrations" from now on.  
> (Nevertheless, we try to get in important bug fixes before jdk release gates 
> shut)
> 
> 8193174: SubmissionPublisher invokes the Subscriber's onComplete before all 
> of its submitted items have been published
> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/SubmissionPublisher-race/index.html
>  
> 
> https://bugs.openjdk.java.net/browse/JDK-8193174 
> 
> 
> 8192943: Optimize atomic accumulators using VarHandle getAndSet
> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/getAndSet/index.html
>  
> 
> https://bugs.openjdk.java.net/browse/JDK-8192943 
> 
> 
> 8192944: Miscellaneous changes imported from jsr166 CVS 2017-12-XX
> http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/miscellaneous/index.html
>  
> 
> https://bugs.openjdk.java.net/browse/JDK-8192944 
> 
> 

Re: Finalization and dead references: another proposal

[Vitaly Davidovich]

On Thu, Dec 7, 2017 at 12:28 PM Peter Levart  wrote:

> Hi,
>
> On 12/07/2017 03:27 AM, Vitaly Davidovich wrote:
> > So kind of the opposite of WeakReference - a SuperStrongReference :).
> >
> > Kidding aside, it seems like the way you’d want to encapsulate this at
> the
> > language level is via a type that the JVM intrinsically knows about; in
> > this way it’s similar to the reference types today.
> >
> > An annotation probably does the trick when the value doesn’t escape from
> > the enclosing instance but I’ve no idea if that assumption covers enough
> > code to warrant this approach.  AFAICT, if the value escapes into an
> > instance of another type that doesn’t annotate its field then all bets
> are
> > off.
> >
> > Having a wrapper type would at least make it harder to leak the  native
> > handle vs the annotation approach.  But of course the wrapper comes with
> > footprint and indirection costs.  Maybe Valhalla could allow exposing
> some
> > magic value type that’s a zero-cost wrapper but preserves the type
> > information the JIT can track?
>
> There is a middle-ground. By (ab)using value types only and no special
> JIT magic.
>
> DirectBuffer(s) for example, could return the address not in a long, but
> in a value type like the following (using valhalla MVT speak):
>
> public __ByValue final class Address {
>  private final long address;
>  private final Object referent;
>
>  public _ValueFactory static Address create(long address, Object
> referent) {
>  Address a = __MakeDefault Address();
>  a.address = address;
>  a.referent = referent;
>  return a;
>  }
> }
>
>
> DirectByteBuffer for example, would have the following address() method:
>
> private long address;
>
> public Address address() {
>  return Address.create(address, this);
> }
>
> Notice that 'address' field of Address value is encapsulated, so Java
> code can't access it directly nor it needs to. Native / Unsafe methods
> would be taking Address values instead of long(s), making sure the
> embedded referent is kept reachable.
>
> This is equivalent to passing the DirectBuffer reference to the native
> method(s) together with the address value, but enforced by the API so
> the programmer can not make a mistake. There's a lot of arithmetic going
> on with addresses, inside and outside of DirectBuffer implementations.
> Such arithmetic would have to be performed by the Address value type
> itself, making sure the results of operations on Address values are
> Address values that maintain the same referent. For example:
>
> public __ByValue final class Address {
> ...
>  public _ValueFactory Address plus(long offset) {
>  Address a = __MakeDefault Address();
>  a.address = address + offset;
>  a.referent = referent;
>  return a;
>  }
> ...
>
> So no magic here. Just API.

This is an API version of Hans’s #3 approach.  As he said, there’s
performance overhead and nothing guarantees that the referent is kept alive
- that’s an implementation artifact.

I think without the VM knowing about these things intrinsically it’s not a
100% reliable solution because it’s not concretely requesting a certain
behavior.

>
>
> Regards, Peter
>
>
> --
Sent from my phone

Re: RFR 4358774: Add null InputStream and OutputStream

[Pavel Rappo]

> On Dec 6, 2017, at 11:11 AM, Jonathan Gibbons  
> wrote:
> 
>> "null" is a significant term in the Java ecosystem, and the relationship 
>> here, to /dev/null or NUL seems somewhat tenuous.
>> 
>> Have any other names been considered?  At least for the InputStream, calling 
>> it an "empty stream" seems more intuitive than a "null stream".

Jon, I think there's an established name for this kind of objects: 

https://en.wikipedia.org/wiki/Null_object_pattern

> On 6 Dec 2017, at 22:28, Brian Burkhalter  wrote:
> 
> The name “emptyStream()” was considered for InputStream and 
> “discardingStream()” for OutputStream. It was thought that “null” or “empty” 
> would be more likely to be found by developers due to familiarity. FWIW there 
> is precedent in third party libraries for the “null” names.
> 
> Brian

+1

Re: Finalization and dead references: another proposal

[Peter Levart]

Hi,

On 12/07/2017 03:27 AM, Vitaly Davidovich wrote:

So kind of the opposite of WeakReference - a SuperStrongReference :).

Kidding aside, it seems like the way you’d want to encapsulate this at the
language level is via a type that the JVM intrinsically knows about; in
this way it’s similar to the reference types today.

An annotation probably does the trick when the value doesn’t escape from
the enclosing instance but I’ve no idea if that assumption covers enough
code to warrant this approach.  AFAICT, if the value escapes into an
instance of another type that doesn’t annotate its field then all bets are
off.

Having a wrapper type would at least make it harder to leak the  native
handle vs the annotation approach.  But of course the wrapper comes with
footprint and indirection costs.  Maybe Valhalla could allow exposing some
magic value type that’s a zero-cost wrapper but preserves the type
information the JIT can track?


There is a middle-ground. By (ab)using value types only and no special 
JIT magic.


DirectBuffer(s) for example, could return the address not in a long, but 
in a value type like the following (using valhalla MVT speak):


public __ByValue final class Address {
    private final long address;
    private final Object referent;

    public _ValueFactory static Address create(long address, Object 
referent) {

        Address a = __MakeDefault Address();
        a.address = address;
        a.referent = referent;
        return a;
    }
}


DirectByteBuffer for example, would have the following address() method:

private long address;

public Address address() {
    return Address.create(address, this);
}

Notice that 'address' field of Address value is encapsulated, so Java 
code can't access it directly nor it needs to. Native / Unsafe methods 
would be taking Address values instead of long(s), making sure the 
embedded referent is kept reachable.


This is equivalent to passing the DirectBuffer reference to the native 
method(s) together with the address value, but enforced by the API so 
the programmer can not make a mistake. There's a lot of arithmetic going 
on with addresses, inside and outside of DirectBuffer implementations. 
Such arithmetic would have to be performed by the Address value type 
itself, making sure the results of operations on Address values are 
Address values that maintain the same referent. For example:


public __ByValue final class Address {
...
    public _ValueFactory Address plus(long offset) {
    Address a = __MakeDefault Address();
        a.address = address + offset;
        a.referent = referent;
        return a;
    }
...

So no magic here. Just API.

Regards, Peter

Review Request: JDK-8193192: jdeps --generate-module-info does not look at module path

[mandy chung]

jdeps currently finds modules from the module path for analysis based on 
the value specified via --add-modules option.  When the input classes 
depend on a module on the given module path, it will report missing 
dependences until the module is specified in --add-modules option.  This 
fixes this issue and jdeps should look at the modules on the module path 
when analyzing classes (except jdeps -m option).


http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8193192/webrev.00

thanks
Mandy

RFR: jsr166 jdk integration 2017-12-XX

[Martin Buchholz]

http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/overview.html

With the change to the openjdk release model, jsr166 integrations will
simply be "jdk integrations" instead of e.g. "jdk10 integrations" from now
on.  (Nevertheless, we try to get in important bug fixes before jdk release
gates shut)

8193174: SubmissionPublisher invokes the Subscriber's onComplete before all
of its submitted items have been published
http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/SubmissionPublisher-race/index.html
https://bugs.openjdk.java.net/browse/JDK-8193174

8192943: Optimize atomic accumulators using VarHandle getAndSet
http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/getAndSet/index.html
https://bugs.openjdk.java.net/browse/JDK-8192943

8192944: Miscellaneous changes imported from jsr166 CVS 2017-12-XX
http://cr.openjdk.java.net/~martin/webrevs/jdk/jsr166-integration/miscellaneous/index.html
https://bugs.openjdk.java.net/browse/JDK-8192944

Re: RFR(XXS): 8182307 - Error during JRMP connection establishment

[Daniel D. Daugherty]

Roger,

Thanks for the review!

Dan

P.S.
I'm planning to push this fix to jdk/hs since the only sightings
have been in jdk/hs testing or in projects that are parented to
jdk/hs repos... Hope that's okay...


On 12/7/17 12:07 PM, Roger Riggs wrote:

+1

On 12/7/2017 12:00 PM, Gerald Thornbrugh wrote:

Hi Dan,

Your fix looks good.

Jerry


Adding core-libs-dev@... since this is RMI code. Thanks Alan!

Folks, this review spans three OpenJDK aliases so Thunderbird's
reply-to-list feature won't get it right. This is one of the
few times that reply-to-all is the right thing to do...

Dan

On 12/7/17 11:38 AM, Daniel D. Daugherty wrote:

Greetings,

I have a small fix for a very intermittent ServerSocket related test
failure:

    JDK-8182307: Error during JRMP connection establishment
https://bugs.openjdk.java.net/browse/JDK-8182307

The fix is copied from Jerry's work on the following bugs:

    JDK-8182757 JDWP: Socket Transport handshake hangs on Solaris
https://bugs.openjdk.java.net/browse/JDK-8182757

    JDK-8178676 nsk/jvmti/AttachOnDemand/attach045 fails with 
Exception

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

For the gory details of the reasons for this fix please see
Jerry's bugs.

Webrev URL: http://cr.openjdk.java.net/~dcubed/8182307-webrev/jdk10-0/


I observed this test failure one time in my Thread-SMR work and have
been including this fix in months of Thread-SMR stress testing. It was
also included in both JPRT and Mach5 tier[1-5] testing of the 
Thread-SMR

changes.

Thanks, in advance, for any comments, suggestions or questions.

Dan

Re: RFR(XXS): 8182307 - Error during JRMP connection establishment

[Roger Riggs]

+1

On 12/7/2017 12:00 PM, Gerald Thornbrugh wrote:

Hi Dan,

Your fix looks good.

Jerry


Adding core-libs-dev@... since this is RMI code. Thanks Alan!

Folks, this review spans three OpenJDK aliases so Thunderbird's
reply-to-list feature won't get it right. This is one of the
few times that reply-to-all is the right thing to do...

Dan

On 12/7/17 11:38 AM, Daniel D. Daugherty wrote:

Greetings,

I have a small fix for a very intermittent ServerSocket related test
failure:

    JDK-8182307: Error during JRMP connection establishment
    https://bugs.openjdk.java.net/browse/JDK-8182307

The fix is copied from Jerry's work on the following bugs:

    JDK-8182757 JDWP: Socket Transport handshake hangs on Solaris
    https://bugs.openjdk.java.net/browse/JDK-8182757

    JDK-8178676 nsk/jvmti/AttachOnDemand/attach045 fails with Exception
    https://bugs.openjdk.java.net/browse/JDK-8178676

For the gory details of the reasons for this fix please see
Jerry's bugs.

Webrev URL: http://cr.openjdk.java.net/~dcubed/8182307-webrev/jdk10-0/


I observed this test failure one time in my Thread-SMR work and have
been including this fix in months of Thread-SMR stress testing. It was
also included in both JPRT and Mach5 tier[1-5] testing of the 
Thread-SMR

changes.

Thanks, in advance, for any comments, suggestions or questions.

Dan

Re: RFR(XXS): 8182307 - Error during JRMP connection establishment

[Daniel D. Daugherty]

Jerry,

Thanks for the review!

Dan


On 12/7/17 12:00 PM, Gerald Thornbrugh wrote:

Hi Dan,

Your fix looks good.

Jerry


Adding core-libs-dev@... since this is RMI code. Thanks Alan!

Folks, this review spans three OpenJDK aliases so Thunderbird's
reply-to-list feature won't get it right. This is one of the
few times that reply-to-all is the right thing to do...

Dan

On 12/7/17 11:38 AM, Daniel D. Daugherty wrote:

Greetings,

I have a small fix for a very intermittent ServerSocket related test
failure:

    JDK-8182307: Error during JRMP connection establishment
    https://bugs.openjdk.java.net/browse/JDK-8182307

The fix is copied from Jerry's work on the following bugs:

    JDK-8182757 JDWP: Socket Transport handshake hangs on Solaris
    https://bugs.openjdk.java.net/browse/JDK-8182757

    JDK-8178676 nsk/jvmti/AttachOnDemand/attach045 fails with Exception
    https://bugs.openjdk.java.net/browse/JDK-8178676

For the gory details of the reasons for this fix please see
Jerry's bugs.

Webrev URL: http://cr.openjdk.java.net/~dcubed/8182307-webrev/jdk10-0/


I observed this test failure one time in my Thread-SMR work and have
been including this fix in months of Thread-SMR stress testing. It was
also included in both JPRT and Mach5 tier[1-5] testing of the 
Thread-SMR

changes.

Thanks, in advance, for any comments, suggestions or questions.

Dan

Re: RFR(XXS): 8182307 - Error during JRMP connection establishment

[Gerald Thornbrugh]

Hi Dan,

Your fix looks good.

Jerry


Adding core-libs-dev@... since this is RMI code. Thanks Alan!

Folks, this review spans three OpenJDK aliases so Thunderbird's
reply-to-list feature won't get it right. This is one of the
few times that reply-to-all is the right thing to do...

Dan

On 12/7/17 11:38 AM, Daniel D. Daugherty wrote:

Greetings,

I have a small fix for a very intermittent ServerSocket related test
failure:

    JDK-8182307: Error during JRMP connection establishment
    https://bugs.openjdk.java.net/browse/JDK-8182307

The fix is copied from Jerry's work on the following bugs:

    JDK-8182757 JDWP: Socket Transport handshake hangs on Solaris
    https://bugs.openjdk.java.net/browse/JDK-8182757

    JDK-8178676 nsk/jvmti/AttachOnDemand/attach045 fails with Exception
    https://bugs.openjdk.java.net/browse/JDK-8178676

For the gory details of the reasons for this fix please see
Jerry's bugs.

Webrev URL: http://cr.openjdk.java.net/~dcubed/8182307-webrev/jdk10-0/


I observed this test failure one time in my Thread-SMR work and have
been including this fix in months of Thread-SMR stress testing. It was
also included in both JPRT and Mach5 tier[1-5] testing of the Thread-SMR
changes.

Thanks, in advance, for any comments, suggestions or questions.

Dan

Re: RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

[Martin Buchholz]

configure => configured
+// For backward compatibility: add any handlers
configure using

Re: RFR(XXS): 8182307 - Error during JRMP connection establishment

[Daniel D. Daugherty]

Adding core-libs-dev@... since this is RMI code. Thanks Alan!

Folks, this review spans three OpenJDK aliases so Thunderbird's
reply-to-list feature won't get it right. This is one of the
few times that reply-to-all is the right thing to do...

Dan

On 12/7/17 11:38 AM, Daniel D. Daugherty wrote:

Greetings,

I have a small fix for a very intermittent ServerSocket related test
failure:

    JDK-8182307: Error during JRMP connection establishment
    https://bugs.openjdk.java.net/browse/JDK-8182307

The fix is copied from Jerry's work on the following bugs:

    JDK-8182757 JDWP: Socket Transport handshake hangs on Solaris
    https://bugs.openjdk.java.net/browse/JDK-8182757

    JDK-8178676 nsk/jvmti/AttachOnDemand/attach045 fails with Exception
    https://bugs.openjdk.java.net/browse/JDK-8178676

For the gory details of the reasons for this fix please see
Jerry's bugs.

Webrev URL: http://cr.openjdk.java.net/~dcubed/8182307-webrev/jdk10-0/


I observed this test failure one time in my Thread-SMR work and have
been including this fix in months of Thread-SMR stress testing. It was
also included in both JPRT and Mach5 tier[1-5] testing of the Thread-SMR
changes.

Thanks, in advance, for any comments, suggestions or questions.

Dan

Re: RFR(XS) : 8181118 : update java/time tests to use RandomFactory from the top level testlibrary

[Roger Riggs]

Hi Igor,

On 12/7/2017 11:18 AM, Igor Ignatyev wrote:



Also, please break the line (w/ '\`) for exclusiveAccess so side-by-side diffs 
fit on the screen.

it's not really related to this patch, so if you don't mind of course, I'd 
prefer to make that refactoring in a separate RFE.
True, but it is incidental here and is not worth doing as a separate 
RFE.  Just trying to keep the overhead low.
if you're not going to do it now; don't bother creating an RFE; it just 
more noise.


Thanks, Roger


Thanks, Roger


On 12/6/2017 7:35 PM, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev/8181118/webrev.00

Re: Review Request: JDK-8193159: Reduce the number of classes loaded due to NativeLibrary

[Martin Buchholz]

+1

Re: RFR JDK-8187485: Update Zip implementation to use Cleaner, not finalizers

[mandy chung]

On 12/4/17 3:14 PM, Xueming Shen wrote:


issue: https://bugs.openjdk.java.net/browse/JDK-8185582
webrev: http://cr.openjdk.java.net/~sherman/8185582/webrev



ZStreamRef.java 79 static ZStreamRef get(Object owner, LongSupplier 
addr, LongConsumer end) {
It may be better to have two factory methods that take the specific type 
(Deflater & Inflater) instead of this single method taking Object owner.
ZipFile.java 701 // List of cached Inflater objects for decompression 
702 Deque inflaterCache;should this be a volatile field?

TestCleaner.java27  * @summary Check the resources of Inflater, Deflater 
and ZipFile are always
  28  *  cleaned/released when the instane is not unreachable

typo: s/instane/instance and s/not unreachable/unreachable

Otherwise looks good.
Mandy

Re: RFR(XS) : 8181118 : update java/time tests to use RandomFactory from the top level testlibrary

[Igor Ignatyev]

Hi Roger,

> On Dec 7, 2017, at 7:27 AM, Roger Riggs  wrote:
> 
> Hi Igor,
> 
> Looks fine.
> 
> Is the current jtreg b09?
no, the latest the greatest jtreg is 4.2-b10. usually we specify the minimal 
required version in TEST.ROOT, not the latest available.
> 
> Also, please break the line (w/ '\`) for exclusiveAccess so side-by-side 
> diffs fit on the screen.
it's not really related to this patch, so if you don't mind of course, I'd 
prefer to make that refactoring in a separate RFE.
> 
> Thanks, Roger
> 
> 
> On 12/6/2017 7:35 PM, Igor Ignatyev wrote:
>> http://cr.openjdk.java.net/~iignatyev/8181118/webrev.00
>>> 115 lines changed: 0 ins; 108 del; 7 mod;
>> Hi all,
>> 
>> could you please review this small fix for java/time tests? RandomFactory 
>> has been moved to the top level testlibrary by 8180805[1], but java/time 
>> weren't updated when b/c jtreg didn't use external root for libraries listed 
>> in lib.dirs test property file[2]. it has been fixed in jtreg4.2b09, now 
>> java/time tests can be updated to use top level testlibrary and @deprecated 
>> RandomFactory (from test/jdk/lib/testlibrary) can be removed.
>> 
>> webrev: http://cr.openjdk.java.net/~iignatyev/8181118/webrev.00
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8181118
>> testing: java/time/jck and java/time/tests
>> 
>> [1] https://bugs.openjdk.java.net/browse/JDK-8180805
>> [2] https://bugs.openjdk.java.net/browse/CODETOOLS-7901987
>> 
>> Thanks,
>> -- Igor
> 

Re: Review Request: JDK-8193159: Reduce the number of classes loaded due to NativeLibrary

[mandy chung]

On 12/7/17 5:39 AM, Alan Bateman wrote:



On 07/12/2017 05:04, mandy chung wrote:



On 12/6/17 6:08 PM, Martin Buchholz wrote:
Google decided that LinkedList is almost never the best choice, so 
any use of LinkedList is discouraged.


ArrayDeque's backing array never shrinks, so you might want to give 
it an explicit initial size.




The initial size is 16 which is okay.  I can make it smaller instead:

-    static Deque nativeLibraryContext = new 
LinkedList<>();
+    static Deque nativeLibraryContext = new 
ArrayDeque<>(8);
This looks okay. It seems unlikely that hundreds of native libraries 
will be loaded, then unloaded, and the backing array being a memory 
concern.


Yup, it's not a big concern.   I believe the number of elements in this 
deque is typically very small number.   This deque is used to keep the 
native libraries are being loaded and its context used by JNI FindClass 
when called from JNI_OnLoad.


Mandy

Re: Finalization and dead references: another proposal

[Peter Levart]

Hi,

I'm trying to read the rules and imagine cases where they would not work...


/A field declared to be reachability-sensitive is reachability-safe. An 
access a to a reachability-safe field of object p inside a (possibly 
static) method in p’s class, results in the introduction of 
reachabilityFence()s according to the following rules://

//
//For every local reference variable v declared immediately inside 
lexical scope s, if s contains such an access a, where p is obtained by 
dereferencing v, then Reference.reachabilityFence(v) will be executed 
just before either (1) the exit of the scope s, or (2) just before any 
assignment to v. For present purposes, “this” is treated as a variable 
declared at method scope, as if it were an explicit parameter.//
//If the object containing the field f is allocated in the same 
full-expression as the access a, then Reference.reachabilityFence(p), 
where p is a reference to the object containing f, is executed at the 
end of the full expression.//

//
//The second case covers expressions like//
//
//    nativeDoSomething(new Foo(...).nativeHandle)//
//
//where again the newly allocated object could otherwise be cleaned 
before nativeHandle is used.//

/

1st case that comes to mind and is a real case in the JDK is the 
existence of sun.nio.ch.DirectBuffer internal (not exported) interface 
implemented by NIO direct buffer(s) with method:


    long address();

Utility method like the following:

    static void erase(ByteBuffer bb) {
    unsafe.setMemory(((DirectBuffer)bb).address(), bb.capacity(), 
(byte)0);

    }

...won't get reachabilityFence(bb) before return, will it? Calling a 
method on a bb doesn't count as a dereference of a reachablity-sensitive 
field of bb. Unless methods could also be annotated with the same 
annotation. This would only work for instance methods that return a 
native resource kept in the same instance or some instance that is 
reachable from this instance.


Native resources are typically encapsulated, but there are various 
levels of encapsulation. DirectBuffer is an example of module level 
encapsulation by non-exported public interface implemented by 
package-private classes.


2nd case is artificial. But, since we have streams, lambdas, 
optionals... It's easy to fool above rules even if the annotation is put 
on the DirectBuffer#address() method ...


static void doSomethingWith1stNonNull(DirectBuffer... dbs) {
    Stream.of(dbs)
        .filter(Objects::nonNull)
        .mapToLong(DirectBuffer::address)
        .findFirst()
        .ifPresent(addr -> {
            ... do something with addr ...
        });
}

Even with imperative programming, one can play with scopes:

static void doSomethingWith1stNonNull(DirectBuffer... dbs) {
    long addr = 0;
    for (DirectBuffer db : dbs) {
        if (db != null) {
            addr = db.address();
            break;
        }
    }
    if (addr != 0) {
        ... do something with addr ...
    }
}

Or, hope that this would work (but the above rules don't cover it):

static void doSomethingWithEither(DirectBuffer db1, DirectBuffer db2) {
    long addr = ((some condition) ? db1 : db2).address();
    ... do something with addr ...
}


But I can imagine that teaching users to not do such foolish things 
would be easy. The rules are simple:
- always dereference reachablity-sensitive resources directly through 
local reference variables.
- make sure that the local reference variable that you extracted 
reachablity-sensitive resource from, is in scope while you perform 
operations on the resource.


Regards, Peter


On 12/07/2017 01:38 AM, Hans Boehm wrote:

We're still trying to deal with a fair amount of code that implicitly
assumes that finalization or similar clean-up will not occur while a
pointer to the affected object is in scope. Which is of course not true.

As a reminder, the canonical offending usage (anti-)pattern with
(deprecated, but easier to write) finalizers is

class Foo {
 private long mPtrToNativeFoo;

 private static native void nativeDeallocate(long nativePtr);
 private static native void nativeDoSomething(long nativePtr, long
anotherNativePtr);

 protected void finalize() { ... nativeDeallocate(mPtrToNativeFoo); ... }

 public void doSomething(Foo another) { ...
nativeDoSomething(mPtrToNativeFoo, another.mPtrToNativeFoo) ... }
 ...
}

This is subtly incorrect in that, while executing the final call to
doSomething() on a particular object, just after retrieving mPtrToNativeFoo
and another.mPtrToNativeFoo, but before invoking nativeDoSomething(), the
garbage collector may run, and "this" and "another" may be finalized,
deallocating the native objects their mPtrToNativeFoos refer to.
When nativeDoSomething() finally does run, it may see dangling pointers.

Examples using java.lang.ref or Cleaners (or even WeakHashMap, if you must)
instead of finalize() are as easy to construct, but a bit longer. (The
finalizer version is also arguably 

Re: [RFR] 8160768: Add capability to custom resolve host/domain names within the default JDNI LDAP provider

[vyom tewari]

Hi Rob,

Latest code looks good to me.

minor bit.

LdapDnsProviderService.java

Line: 71 , while loop condition is bit complex, we can simplify it little bit 
as follows.This will make code more readable and easy to understand.


while (iterator.hasNext())
    {
    LdapDnsProvider p = iterator.next();
    result = p.lookupEndpoints(url, env);
    if(result != null && !result.getEndpoints().isEmpty()){
    break;
    }
    }
It is just a personal preference you can ignore it if you don't like it.

Thanks,
Vyom

On Wednesday 06 December 2017 11:54 PM, Rob McKenna wrote:

Thanks Vyom, these should be fixed in:

http://cr.openjdk.java.net/~robm/8160768/webrev.07/

Comments inline:

On 06/12/17 22:14, vyom tewari wrote:

Hi Rob,

Please find below comments.

DefaultLdapDnsProvider.java

  line 35:     convert it to for-each code will be more readable.

LdapDnsProviderService.java

  line 21: constant name does not follow Java naming convention, there are
other places as well please fix it.

getInstance() is already synchronized why you need another "synchronized"
block ?

Ah, neglected to remove the outer synchronized keyword, thanks.


Line 70: Please use result.getEndpoints().isEmpty() in place of
result.getEndpoints().size() == 0

LdapDnsProvider.java

constructor LdapDnsProvider() calls LdapDnsProvider(Void unused) which does
nothing, can you explain why LdapDnsProvider()  calls LdapDnsProvider(Void
unused) ?

I've copied this pattern from the System.LoggerFinder api and I had
forgotten what it was for too:

http://www.oracle.com/technetwork/java/seccodeguide-139067.html#7

Section 7.3.


LdapDnsProviderResult.java

   Private field  domainName & endpoints both can be final.

LdapDnsProviderTest.java

Fix the tag Order it is not correct. correct order is as follows.

/**
  * @test
  * @bug 8160768
  * @summary ctx provider tests for ldap
  * @modules java.naming/com.sun.jndi.ldap
  * @compile dnsprovider/TestDnsProvider.java
  * @run main/othervm LdapDnsProviderTest
  * @run main/othervm LdapDnsProviderTest nosm
  * @run main/othervm LdapDnsProviderTest smnodns
  * @run main/othervm LdapDnsProviderTest smdns
  */

Line 82,83 you don't need System.out.println(e); e.printStackTrace();

I'm going to leave these in as I've had a few failing tests in the past
where I've needed to push diagnostic changes like this to determine the
root cause.

 -Rob


Line 70: you don't need this try block

Line 96 : constant name does not follow Java naming convention

Line 105: you can use try-with-resources this will avoid some boilerplate.

Thanks,
Vyom

On Tuesday 05 December 2017 11:18 PM, Rob McKenna wrote:

As this enhancement is limited to JDK10 this frees us up to explore a
different approach.

http://cr.openjdk.java.net/~robm/8160768/webrev.06/

In this webrev we're using the Service Provider Interface to load an
implementation of the new LdapDnsProvider class. Implementations of this
class are responsible for resolving DNS endpoints for a given ldap url
should the default implementation be insufficient in a particular
environment.

Note: if a security manager is installed the ldapDnsProvider
RuntimePermission must be granted.

 -Rob

Re: RFR(XS) : 8181118 : update java/time tests to use RandomFactory from the top level testlibrary

[Roger Riggs]

Hi Igor,

Looks fine.

Is the current jtreg b09?

Also, please break the line (w/ '\`) for exclusiveAccess so side-by-side 
diffs fit on the screen.


Thanks, Roger


On 12/6/2017 7:35 PM, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev/8181118/webrev.00

115 lines changed: 0 ins; 108 del; 7 mod;

Hi all,

could you please review this small fix for java/time tests? RandomFactory has 
been moved to the top level testlibrary by 8180805[1], but java/time weren't 
updated when b/c jtreg didn't use external root for libraries listed in 
lib.dirs test property file[2]. it has been fixed in jtreg4.2b09, now java/time 
tests can be updated to use top level testlibrary and @deprecated RandomFactory 
(from test/jdk/lib/testlibrary) can be removed.

webrev: http://cr.openjdk.java.net/~iignatyev/8181118/webrev.00
JBS: https://bugs.openjdk.java.net/browse/JDK-8181118
testing: java/time/jck and java/time/tests

[1] https://bugs.openjdk.java.net/browse/JDK-8180805
[2] https://bugs.openjdk.java.net/browse/CODETOOLS-7901987

Thanks,
-- Igor

Re: RFR 4358774: Add null InputStream and OutputStream

[Vitaly Davidovich]

On Thu, Dec 7, 2017 at 9:40 AM Roger Riggs  wrote:

> Hmm...
>
> Seems like a lot of fine tuning for a function that has a low profile and
> no
> performance data...

These are known issues in the current JIT optimizer.  We all wish we didn’t
have to do this stuff.  If we want the best chance of these noop impls to
actually be close to noop then it warrants consideration.  If that’s not a
concern then sure, nothing to discuss.  That’s all :).

I also wouldn’t say “it’s a lot of fine tuning” - these are simple
mechanical things.

>
>
> $.02, Roger
>
> On 12/6/2017 10:03 PM, Vitaly Davidovich wrote:
> > On Wed, Dec 6, 2017 at 4:01 PM, Jason Mehrens  >
> > wrote:
> >
> >> Brian,
> >>
> >> My understand is JDK-6533165 is moving the bytes that are rarely invoked
> >> to a cold method.
> >>
> >> Therefore:
> >> 
> >> if (closed) {
> >> throw new IOException("Stream closed");
> >> }
> >> ==becomes===
> >> if (closed) {
> >> throw sc();
> >> }
> >>
> >> private static IOException sc() {
> >>  return new IOException("Stream closed");
> >> }
> >> 
> >>
> >> Since there is no string concatenation in the IOE message there are
> only a
> >> few bytes that can be shaved off.  I didn't benchmark it either case
> but I
> >> would assume it would matter for the nullOutputStream since the write
> >> methods could be invoked multiple times.
> >>
> >  From a performance angle, I'd be more concerned with the calls to
> > Objects.xyz() methods there.  Unless something has changed in the JIT
> > recently, those are susceptible to profile pollution and can cause missed
> > optimizations.  I'd inline those methods manually to give these methods
> > their own profiles.
> >
> >> Jason
> >> 
> >> From: Brian Burkhalter 
> >> Sent: Wednesday, December 6, 2017 2:05 PM
> >> To: Jason Mehrens
> >> Cc: core-libs-dev
> >> Subject: Re: RFR 4358774: Add null InputStream and OutputStream
> >>
> >> Jason,
> >>
> >> On Dec 6, 2017, at 11:54 AM, Jason Mehrens  >> mailto:jason_mehr...@hotmail.com>> wrote:
> >>
> >> For nullInputStream would it make any sense to use the
> >> ByteArrayInputStream with a (private static) empty byte array?  Maybe
> >> 'return new ByteArrayInputStream("".getBytes());'?  One side effect is
> >> that mark support returns true.
> >>
> >> As we are trying to retain the behavior of closed streams throwing
> >> IOExceptions I don’t think that BAIS would work here.
> >>
> >> Does it make sense to follow the advice inhttps://bugs.openjdk.java.
> >> net/browse/JDK-6533165 with regards to streams being closed?
> >>
> >> I don’t know exactly what you intend here. In the linked issue there is
> >> information to impart, namely the index and the size. Here there is only
> >> the indication that the stream is closed. It’s not clear to me what
> could
> >> be done here.
> >>
> >> Thanks,
> >>
> >> Brian
> >>
>
> --
Sent from my phone

Re: Change in properties for logging: deliberate?

[Daniel Fuchs]

Hi Jeremy,

I just proposed a patch on core-libs-dev.

best regards,

-- daniel

On 06/12/2017 01:17, Jeremy Manson wrote:

Hey folks,

Any thoughts on a timeline for this? We're just having to decide what to 
do internally.  If a patch is likely to arrive in the next month or so, 
then we'll probably wait, but if not, we should probably figure out a 
workaround.


(I'm not trying to be too pushy - we can certainly figure out a workaround.)

Jeremy

Re: RFR 4358774: Add null InputStream and OutputStream

[Roger Riggs]

Hmm...

Seems like a lot of fine tuning for a function that has a low profile and no
performance data...

$.02, Roger

On 12/6/2017 10:03 PM, Vitaly Davidovich wrote:

On Wed, Dec 6, 2017 at 4:01 PM, Jason Mehrens 
wrote:


Brian,

My understand is JDK-6533165 is moving the bytes that are rarely invoked
to a cold method.

Therefore:

if (closed) {
throw new IOException("Stream closed");
}
==becomes===
if (closed) {
throw sc();
}

private static IOException sc() {
 return new IOException("Stream closed");
}


Since there is no string concatenation in the IOE message there are only a
few bytes that can be shaved off.  I didn't benchmark it either case but I
would assume it would matter for the nullOutputStream since the write
methods could be invoked multiple times.


 From a performance angle, I'd be more concerned with the calls to
Objects.xyz() methods there.  Unless something has changed in the JIT
recently, those are susceptible to profile pollution and can cause missed
optimizations.  I'd inline those methods manually to give these methods
their own profiles.


Jason

From: Brian Burkhalter 
Sent: Wednesday, December 6, 2017 2:05 PM
To: Jason Mehrens
Cc: core-libs-dev
Subject: Re: RFR 4358774: Add null InputStream and OutputStream

Jason,

On Dec 6, 2017, at 11:54 AM, Jason Mehrens > wrote:

For nullInputStream would it make any sense to use the
ByteArrayInputStream with a (private static) empty byte array?  Maybe
'return new ByteArrayInputStream("".getBytes());'?  One side effect is
that mark support returns true.

As we are trying to retain the behavior of closed streams throwing
IOExceptions I don’t think that BAIS would work here.

Does it make sense to follow the advice inhttps://bugs.openjdk.java.
net/browse/JDK-6533165 with regards to streams being closed?

I don’t know exactly what you intend here. In the linked issue there is
information to impart, namely the index and the size. Here there is only
the indication that the stream is closed. It’s not clear to me what could
be done here.

Thanks,

Brian

RFR: 8191033: Regression in logging.properties: specifying .handlers= for root logger (instead of handlers=) no longer works

[Daniel Fuchs]

Hi,

Please find below a patch for:

8191033 Regression in logging.properties: specifying .handlers= for
root logger (instead of handlers=) no longer works
https://bugs.openjdk.java.net/browse/JDK-8191033

webrev:
http://cr.openjdk.java.net/~dfuchs/webrev_8191033/webrev.00/

The patch restores the behavior that was observed for JDK 8.

The test fails with jdk 9.0.1, but passes with jdk1.8.0_131 and
with this patch applied to a clone of http://hg.openjdk.java.net/jdk/jdk

best regards,

-- daniel

Re: RFR 4358774: Add null InputStream and OutputStream

[Alan Bateman]

On 06/12/2017 21:01, Jason Mehrens wrote:

Brian,

My understand is JDK-6533165 is moving the bytes that are rarely invoked to a 
cold method.

Therefore:

if (closed) {
throw new IOException("Stream closed");
}
==becomes===
if (closed) {
throw sc();
}

private static IOException sc() {
 return new IOException("Stream closed");
}


If nothing else, a private ensureOpen method would make it easier to 
read so that the "if (closed) throw ..." isn't needed in every method.


-Alan

Re: Review Request: JDK-8193159: Reduce the number of classes loaded due to NativeLibrary

[Claes Redestad]

On 2017-12-07 14:39, Alan Bateman wrote:



On 07/12/2017 05:04, mandy chung wrote:



On 12/6/17 6:08 PM, Martin Buchholz wrote:
Google decided that LinkedList is almost never the best choice, so 
any use of LinkedList is discouraged.


ArrayDeque's backing array never shrinks, so you might want to give 
it an explicit initial size.




The initial size is 16 which is okay.  I can make it smaller instead:

-    static Deque nativeLibraryContext = new 
LinkedList<>();
+    static Deque nativeLibraryContext = new 
ArrayDeque<>(8);


+1

This looks okay. It seems unlikely that hundreds of native libraries 
will be loaded, then unloaded, and the backing array being a memory 
concern.


It also needs to load said hundreds of native libraries in parallel for 
this context queue to grow in the first place, right?


So I'm not concerned, but on that tangent I have been wondering for some 
time why ArrayDeque doesn't have a
trimToSize method like ArrayList does. Theoretically it'd be nice to 
have some means to allow periodically taking a
look at persistent queues like this one and trim the backing arrays down 
if they ever outgrow their purpose. I guess

it's just never become a real problem...

/Claes

Re: Review Request: JDK-8193159: Reduce the number of classes loaded due to NativeLibrary

[Alan Bateman]

On 07/12/2017 05:04, mandy chung wrote:



On 12/6/17 6:08 PM, Martin Buchholz wrote:
Google decided that LinkedList is almost never the best choice, so 
any use of LinkedList is discouraged.


ArrayDeque's backing array never shrinks, so you might want to give 
it an explicit initial size.




The initial size is 16 which is okay.  I can make it smaller instead:

-    static Deque nativeLibraryContext = new 
LinkedList<>();
+    static Deque nativeLibraryContext = new 
ArrayDeque<>(8);
This looks okay. It seems unlikely that hundreds of native libraries 
will be loaded, then unloaded, and the backing array being a memory concern.


-Alan

Re: Add EnumMap.keyType() and EnumSet.elementType()

[Stephen Colebourne]

I'm surprised I've never run into this. This seems like a simple and
useful change.
Stephen

On 7 December 2017 at 11:40, Andrej Golovnin  wrote:
> Hi all,
>
> it would be nice if we would have access to the class of the enum type
> used to create an EnumMap or an EnumSet.
>
> This is usefull when you write a custom serialization library and
> would like to serialize/deserialize an empty EnumMap or an empty
> EnumSet. For the empty EnumSet there is a workaround to get the enum
> class:
>
> EnumSet empty = EnumSet.noneOf(MyEnum.class);
> EnumSet tmp = EnumSet.complementOf(empty);
> Class elementType = tmp.iterator().next().getClass();
>
> But this only works when the enum class has at least one enum. For
> EnumMap there is no such workaround at all and we have to use the
> Reflection API. And you know the warnings since Java 9 :-) :
>
> WARNING: An illegal reflective access operation has occurred
> WARNING: Illegal reflective access by c.r.r.k.EnumMapSerializer to
> field java.util.EnumMap.keyType
> WARNING: Please consider reporting this to the maintainers of
> c.r.r.k.EnumMapSerializer
> WARNING: Use --illegal-access=warn to enable warnings of further
> illegal reflective access operations
> WARNING: All illegal access operations will be denied in a future release
>
> So to avoid this warning and to avoid that our application stops to
> work with a future release of Java I would like to propose to add the
> following two methods:
>
> EnumMap:
> /**
>  * Returns the {@code Class} object of the key type for this enum map.
>  *
>  * @return the {@code Class} object of the key type for this enum map.
>  *
>  * @since 10
>  */
> public Class keyType()
>
>
> EnumSet:
> /**
>  * Returns the {@code Class} object of all the elements of this set.
>  *
>  * @return the {@code Class} object of all the elements of this set.
>  *
>  * @since 10
>  */
> public Class elementType()
>
> The suggested change is attached as diff.
>
> Best reagrds,
> Andrej Golovnin

Add EnumMap.keyType() and EnumSet.elementType()

[Andrej Golovnin]

Hi all,

it would be nice if we would have access to the class of the enum type
used to create an EnumMap or an EnumSet.

This is usefull when you write a custom serialization library and
would like to serialize/deserialize an empty EnumMap or an empty
EnumSet. For the empty EnumSet there is a workaround to get the enum
class:

EnumSet empty = EnumSet.noneOf(MyEnum.class);
EnumSet tmp = EnumSet.complementOf(empty);
Class elementType = tmp.iterator().next().getClass();

But this only works when the enum class has at least one enum. For
EnumMap there is no such workaround at all and we have to use the
Reflection API. And you know the warnings since Java 9 :-) :

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by c.r.r.k.EnumMapSerializer to
field java.util.EnumMap.keyType
WARNING: Please consider reporting this to the maintainers of
c.r.r.k.EnumMapSerializer
WARNING: Use --illegal-access=warn to enable warnings of further
illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

So to avoid this warning and to avoid that our application stops to
work with a future release of Java I would like to propose to add the
following two methods:

EnumMap:
/**
 * Returns the {@code Class} object of the key type for this enum map.
 *
 * @return the {@code Class} object of the key type for this enum map.
 *
 * @since 10
 */
public Class keyType()


EnumSet:
/**
 * Returns the {@code Class} object of all the elements of this set.
 *
 * @return the {@code Class} object of all the elements of this set.
 *
 * @since 10
 */
public Class elementType()

The suggested change is attached as diff.

Best reagrds,
Andrej Golovnin
diff --git a/src/java.base/share/classes/java/util/EnumMap.java 
b/src/java.base/share/classes/java/util/EnumMap.java
--- a/src/java.base/share/classes/java/util/EnumMap.java
+++ b/src/java.base/share/classes/java/util/EnumMap.java
@@ -179,6 +179,17 @@
 }
 }
 
+/**
+ * Returns the {@code Class} object of the key type for this enum map.
+ *
+ * @return the {@code Class} object of the key type for this enum map.
+ *
+ * @since 10
+ */
+public Class keyType() {
+return keyType;
+}
+
 // Query Operations
 
 /**
diff --git a/src/java.base/share/classes/java/util/EnumSet.java 
b/src/java.base/share/classes/java/util/EnumSet.java
--- a/src/java.base/share/classes/java/util/EnumSet.java
+++ b/src/java.base/share/classes/java/util/EnumSet.java
@@ -365,6 +365,17 @@
 }
 
 /**
+ * Returns the {@code Class} object of all the elements of this set.
+ *
+ * @return the {@code Class} object of all the elements of this set.
+ *
+ * @since 10
+ */
+public Class elementType() {
+return elementType;
+}
+
+/**
  * Adds the specified range to this enum set, which is empty prior
  * to the call.
  */

FW: RFR(M): 8189102: All tools should support -?, -h and --help

[Lindenmaier, Goetz]

Hi,

... missed some lists in my first post ...

I prepared a fifth webrev for this change.  Please review.

It incorporates the changes requested by the CSR reviewers 
(not to remove docuemtation of '-help' where is was documented
before) and the changes proposed by Kumar:
http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.05/

See also the information in the webrev  itself, there are also patch files
with the incremental builds.

This change contains fixes for some langtool tests.
I ran the following test suites on it:
hotspot, jdk, langtools, nashorn, jaxp, most of them on 
all the platforms we build.

Best regards,
  Goetz.




> -Original Message-
> From: Lindenmaier, Goetz
> Sent: Mittwoch, 11. Oktober 2017 22:07
> To: hotspot-dev developers 
> Subject: RFR(M): 8189102: All tools should support -?, -h and --help
> 
> Hi
> 
> The tools in jdk should all show the same behavior wrt. help flags.
> This change normalizes the help flags of a row of the tools in the jdk.
> Java accepts -?, -h and --help, thus I changed the tools to support
> these, too.  Some tools exited with '1' after displaying the help message,
> I turned this to '0'.
> 
> Maybe this is not the right mailing list for this, please advise.
> 
> Please review this change. I please need a sponsor.
> http://cr.openjdk.java.net/~goetz/wr17/8189102-helpMessage/webrev.01/
> 
> In detail, this fixes the help message of the following tools:
> jar  -? -h --help;  added -?.
> jarsigner-? -h --help;  added --help. -help accepted but not documented.
> javac-?--help;  added -?. Removed -help. -h is taken for other 
> purpose
> javadoc  -? -h --help;  added -h -?. Removed -help
> javap-? -h --help;  added -h. -help accepted but no more documented.
> jcmd -? -h --help;  added -? --help. -help accepted but no more
> documented. Changed return value to '0'
> jdb  -? -h --help;  added -? -h --help. -help accepted but no more
> documented.
> jdeprscan-? -h --help;  added -?
> jinfo-? -h --help;  added -? --help. -help accepted but no more
> documented.
> jjs -h --help;  Replaced -help by --help. Adding more not straight
> forward.
> jps  -? -h --help;  added -? --help. -help accepted but no more
> documented.
> jshell   -? -h --help;  added -?
> jstat-? -h --help;  added -h --help. -help accepted but no more
> documented.
> 
> Best regards,
>   Goetz.