Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

2016-02-03 Thread Alan Bateman

On 03/02/2016 03:17, Iris Clark wrote:

Hi, Mandy.

Thanks so much for pushing the changeset for the initial implementation
of jdk.Version!

Good to have this in but now we need to decide on where it should live. 
It's JDK-specific so we'll need it exported by a JDK module rather than 
java.base.


-Alan


Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-02-03 Thread Alan Bateman



On 03/02/2016 02:24, Steve Drach wrote:
I created a new webrev, 
http://cr.openjdk.java.net/~sdrach/8132734/webrev.05/ 
, that 
implements what I outlined above.  In particular I enhanced the 
JarEntryIterator class and I added additional commentary to the 
entries() and stream() methods.  I also added a new test, 
MultiReleaseJarIterators, to test entries() and stream().


I think having stream and entries do this is right although I would like 
to see some performance data if possible. Also I would expect that if a 
JAR file is not mult-release but the library opens it with 
Release.RUNTIME to perform the same as opening the JAR file with the 
Release-less constructors.


I think the javadoc will need to also need to make it clear whether 
entries with names starting with META-INF/versions/ are returned.


I see you've added @since 9 to the existing methods, I assume you didn't 
mean to do this.


At some point then we need to discuss how RUNTIME_VERSION is computed. 
Iris (via Mandy) has pushed jdk.Version to jdk9/dev but having it 
exported by java.base conflicts with the design principles in JEP 200. 
Moving it to another module means that code in java.base cannot use it 
and thus the JAR file can't use it.


-Alan.



Re: RFR JDK-8141491: Unaligned memory access in Bits.c

2016-02-03 Thread Andrew Haley
On 02/02/16 19:25, Mikael Vidstedt wrote:
> Please review this change which introduces a Copy::conjoint_swap and an 
> Unsafe.copySwapMemory method to call it from Java, along with the 
> necessary changes to have java.nio.Bits call it instead of the Bits.c code.

There doesn't seem to be any way to use a byte-swap instruction
in the swapping code.  This will make it unnecessarily slow.

Andrew.



Re: RFR:JDK-8146747:java.time.Duration.toNanos() and toMillis() exception on negative durations

2016-02-03 Thread Stephen Colebourne
Yes, looks fine
thanks
Stephen

On 1 February 2016 at 06:18, nadeesh tv  wrote:
> Hi all,
>
> Please review following
>
> Bug Id : https://bugs.openjdk.java.net/browse/JDK-8146747
>
> Solution: Handled the negative duration separately
>
> webrev : http://cr.openjdk.java.net/~ntv/8146747/webrev.00/
>
> --
> Thanks and Regards,
> Nadeesh TV
>


Re: RFR JDK-7071819: To support Extended Grapheme Clusters in Regex

2016-02-03 Thread Peter Levart

Hi Sherman,

I don't currently have any idea how to squeeze the hashtable any more 
further. It is already very compact in my opinion. But I noticed a data 
race that could result in navigating the half-initialized data 
structure. It is a classical unsafe publication bug. It has been present 
before in get(int cp) and it is present now in both getName(int cp) and 
getCodePoint(String name). For example:


 151 static int getCodePoint(String name) {
 152 byte[] strPool = null;
 153 if (refStrPool == null || (strPool = refStrPool.get()) == 
null) {

 154 strPool = initNamePool();
 155 }

vs.

111 refStrPool = new SoftReference<>(strPool);

...the static refStrPool field is not marked volatile.

One way to fix this is to mark field volatile and then rearrange code in 
getName/getCodePoint to only read from it once by introducing a local 
var. The other would be to change the line 111 into something like:


SoftReference rsp = new SoftReference<>(strPool);
unsafe.storeFence();
refStrPool = rsp;

...*and* also rearrange code in getName/getCodePoint to only read from 
field once by introducing a local var.



Regards, Peter

On 02/02/2016 10:25 PM, Xueming Shen wrote:

Hi,

Have not heard any feedback on this one so far. I'm adding
a little more to make it attractive for reviewers :-)

On top of the \N now the webrev includes the proposal to add
two more matchers, \X for unicode extended grapheme cluster
and \b{g} for the corresponding boundary.

Issue: https://bugs.openjdk.java.net/browse/JDK-7071819
Issue: https://bugs.openjdk.java.net/browse/JDK-8147531
webrev: http://cr.openjdk.java.net/~sherman/8147531_7071819/webrev/

Thanks!
Sherman

On 01/18/2016 11:52 PM, Xueming Shen wrote:

Hi,

Please help review the change to add \N support in regex.

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

This is one of the items we were planning to address via JEP111
http://openjdk.java.net/jeps/111
https://bugs.openjdk.java.net/browse/JDK-8046101

Some of the constructs had been added already in early release. I'm
planning to address the rest as individual rfe separately.

Thanks,
Sherman






Re: RFR JDK-7071819: To support Extended Grapheme Clusters in Regex

2016-02-03 Thread Peter Levart

Hi Again,

I also have a comment on the implementation of the hash table and it's 
GC-ness. You keep the string pool under a SoftReference because it is 
the biggest object so it makes most sense to clear it when heap becomes 
full. Other arrays are kept strongly reachable, but you re-generate them 
nevertheless when string pool is cleared by GC. Would it make sense to:


- define all int[] arrays (including strPool) as final instance 
variables of CharacterName

- have one static field:

private static SoftReference refInstance;

- convert initNamePool() into a private constructor.

- convert public static getName/getCodePoint into public instance methods

- introduce public static method:

public static CharacterName getInstance() {
SoftReference ref = refInstance;
CharacterName instance;
if (ref != null && ((instance = ref.get) != null) {
return instance;
}
instance = new CharacterName();
refInstance = new SoftReference<>(instance);
return instance;
}

...in this arrangement, you don't need volatile fields or explicit 
fences, as all instance fields can be final and JMM guarantees safe 
publication in this case.



Regards, Peter

On 02/03/2016 12:07 PM, Peter Levart wrote:

Hi Sherman,

I don't currently have any idea how to squeeze the hashtable any more 
further. It is already very compact in my opinion. But I noticed a 
data race that could result in navigating the half-initialized data 
structure. It is a classical unsafe publication bug. It has been 
present before in get(int cp) and it is present now in both 
getName(int cp) and getCodePoint(String name). For example:


 151 static int getCodePoint(String name) {
 152 byte[] strPool = null;
 153 if (refStrPool == null || (strPool = refStrPool.get()) == 
null) {

 154 strPool = initNamePool();
 155 }

vs.

111 refStrPool = new SoftReference<>(strPool);

...the static refStrPool field is not marked volatile.

One way to fix this is to mark field volatile and then rearrange code 
in getName/getCodePoint to only read from it once by introducing a 
local var. The other would be to change the line 111 into something like:


SoftReference rsp = new SoftReference<>(strPool);
unsafe.storeFence();
refStrPool = rsp;

...*and* also rearrange code in getName/getCodePoint to only read from 
field once by introducing a local var.



Regards, Peter

On 02/02/2016 10:25 PM, Xueming Shen wrote:

Hi,

Have not heard any feedback on this one so far. I'm adding
a little more to make it attractive for reviewers :-)

On top of the \N now the webrev includes the proposal to add
two more matchers, \X for unicode extended grapheme cluster
and \b{g} for the corresponding boundary.

Issue: https://bugs.openjdk.java.net/browse/JDK-7071819
Issue: https://bugs.openjdk.java.net/browse/JDK-8147531
webrev: http://cr.openjdk.java.net/~sherman/8147531_7071819/webrev/

Thanks!
Sherman

On 01/18/2016 11:52 PM, Xueming Shen wrote:

Hi,

Please help review the change to add \N support in regex.

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

This is one of the items we were planning to address via JEP111
http://openjdk.java.net/jeps/111
https://bugs.openjdk.java.net/browse/JDK-8046101

Some of the constructs had been added already in early release. I'm
planning to address the rest as individual rfe separately.

Thanks,
Sherman








Re: RFR:JDK-8146747:java.time.Duration.toNanos() and toMillis() exception on negative durations

2016-02-03 Thread ShinyaYoshida
Hi Nadeesh,
Almost LGTM!(But I'm not a reviewer;) )
However I've noticed that you don't use NANOS_PER_SECOND at L1223 and L1246.
Is there some reason not to use it?

Regards,
shinyafox(Shinya Yoshida)

2016-02-01 15:18 GMT+09:00 nadeesh tv :

> Hi all,
>
> Please review following
>
> Bug Id : https://bugs.openjdk.java.net/browse/JDK-8146747 <
> https://bugs.openjdk.java.net/browse/JDK-8146747>
>
> Solution: Handled the negative duration separately
>
> webrev : http://cr.openjdk.java.net/~ntv/8146747/webrev.00/ <
> http://cr.openjdk.java.net/%7Entv/8146747/webrev.00/>
>
> --
> Thanks and Regards,
> Nadeesh TV
>
>


Re: RFR:JDK-8146747:java.time.Duration.toNanos() and toMillis() exception on negative durations

2016-02-03 Thread nadeesh tv

Hi Shinya,
Thnx. I will update it.
Regards,
Nadeesh
On 2/3/2016 5:41 PM, ShinyaYoshida wrote:

Hi Nadeesh,
Almost LGTM!(But I'm not a reviewer;) )
However I've noticed that you don't use NANOS_PER_SECOND at L1223 and 
L1246.

Is there some reason not to use it?

Regards,
shinyafox(Shinya Yoshida)

2016-02-01 15:18 GMT+09:00 nadeesh tv >:


Hi all,

Please review following

Bug Id : https://bugs.openjdk.java.net/browse/JDK-8146747


Solution: Handled the negative duration separately

webrev : http://cr.openjdk.java.net/~ntv/8146747/webrev.00/



-- 
Thanks and Regards,

Nadeesh TV




--
Thanks and Regards,
Nadeesh TV



Re: RFR-8148838: Stream.flatMap(...).spliterator() cannot properly split after tryAdvance()

2016-02-03 Thread Tagir F. Valeev
Hello!

Here's updated webrev:
http://cr.openjdk.java.net/~tvaleev/webrev/8148838/r2/

PS> My inclination is to keep this simple and not support splitting after 
partial traversal.

PS> Sometimes splitting after partial traversal might be more complex
PS> not support (e.g. ArrayList). In other cases it’s more complex to
PS> support and in such cases i would argue it is not worth it since
PS> this kind of functionality is an edge case.

I would still like that this problem is fixed (i.e. support splitting
after advance, not just return null). This is probably an edge case
for JDK, but might be crucial for library writers. As a library writer
I actually had bad times working around this issue. While returning
null instead of incorrect splitting would indeed be helpful, it will
unnecessarily remove parallelization capabilities in some cases. For
example, sometimes it's desired to extract the first element from the
stream and create the stream from the tail. Returning null here would
mean that the resulting stream will never split after that.

To my opinion this fix is not very complex. It just adds several lines
into single method (trySplit()). I added a comment which clarifies the
intention. Other changes like extraction of initPusher() do not add
complexity. Actually they reduce the complexity as four identical
lambdas are merged into one. This patch actually reduces the size of
created class files. With javac 9-ea+99 StreamSpliterators.java is
compiled into 79004 bytes after my patch and 79472 bytes before my
patch. Also this patch does not require primitive specializations and
adds no overhead for common case when traversal is not started.

PS> Testing wise you are right to be concerned about the increase in
PS> test execution time. The lack of flatMap is definitely an
PS> omission. In this case I think it is ok to replace map with
PS> flatMap, as both result in stuff going into the holding buffer,
PS> and we can use the smaller data sets, e.g.
PS> "StreamTestData.small”, that were recently added.

I replaced all the datasets with .small alternatives (I think it's ok
here as WrappingSpliterator behavior does not differ much for various
sources)  and  removed all .map tests. Now the test works faster
(like 25 sec -> 17 sec on by box). Please check.

With best regards,
Tagir Valeev.

PS.

PS> Paul.

>> On 2 Feb 2016, at 09:24, Tagir F. Valeev  wrote:
>> 
>> Please review and sponsor this fix:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8148838
>> http://cr.openjdk.java.net/~tvaleev/webrev/8148838/r1/
>> 
>> When buffer traversal is already started, and split is requested, then
>> the existing buffer should be carefully transferred to the prefix
>> part.
>> 
>> The only problem I see here is the testing time. Due to
>> permuteFunctions() call it increases significantly what I added the
>> flatMap() test. If this becomes an issue, I can write new simple test
>> which tests flatMap() only (without permutations with other
>> intermediate operations).
>> 
>> With best regards,
>> Tagir Valeev.
>> 



RFR-8148748: ArrayList.subList().spliterator() is not late-binding

2016-02-03 Thread Tagir F. Valeev
Hello!

Here's the webrev:
http://cr.openjdk.java.net/~tvaleev/webrev/8148748/r1/

I noticed that list == null checks are never true, thus redundant, so
I removed them. The list field is final and always assigned to the
outer ArrayList instance. So it seems quite natural to make
ArrayListSpliterator non-static instead. Also I added one more field
into ArrayListSpliterator to hold the SubList reference. Hopefully
such footprint increase is acceptable. The offset field is initialized
eagerly as it does not change during the SubList lifetime. Also I
replaced "if (action == null)" checks with "Objects.requireNonNull".

Some more thoughts about forEachRemaining:

To me it seems that
if ((a = lst.elementData) != null)
is a redundant check as well as in current ArrayList implementation
elementData is never null. So it can be replaced with simple
assignment.

Also this check:
 (i = index) >= 0
I don't see the case when i could be negative. Should it be removed as
well? Or probably it's added to aid JIT to elimitate bounds-checking?

With best regards,
Tagir Valeev.

PS> Hi Tagir,

PS> Last-binding while adding some complexity to the implementation
PS> is an important property, as it is to fail-fast on a best effort basis.

PS> When one has mutable collections that can be structurally
PS> modified and lazy views over those collections (Streams/Iterators)
PS> then those views really need to act on the most up to date
PS> structure and not a snapshot (structural  or otherwise) when the
PS> view was created. Otherwise, it is a potential source of bugs
PS> producing incorrect results or exceptions, or another form of
PS> complexity for the implementation to maintain integrity under such 
conditions.

PS> We took a short-cut here to reuse the existing top-level
PS> spliterator in a non-late-binding manner by specifying the
PS> absolute fence. We should fix it.

PS> Glancing at the code i believe there is a simple fix. When -1 is
PS> passed as the fence to the ArrayList.ArrayListSpliterator then the
PS> fence is created lazily and should be the result of offset +
PS> list.size e.g. in the constructor:

PS>   hi = fence = offset + lst.size;

PS> The forEach impl also needs updating.

PS> —

PS> Sublists are also tricky beasts. See documentation on List.subList:

PS> * The semantics of the list returned by this method become undefined if
PS> * the backing list (i.e., this list) is structurally modified in
PS> * any way other than via the returned list.  (Structural modifications are
PS> * those that change the size of this list, or otherwise perturb it in such
PS> * a fashion that iterations in progress may yield incorrect results.)

PS> Paul.

>> On 29 Jan 2016, at 05:32, Tagir F. Valeev  wrote:
>> 
>> Hello!
>> 
>> When reviewing 8079136 I noticed that
>> ArrayList.subList().spliterator() is not late-binding:
>> 
>> List list = new ArrayList<>(Arrays.asList(1,2,3,4)).subList(0, 3);
>> Stream s = list.stream();
>> list.add(5);
>> s.findFirst();
>> --> Exception in thread "main" java.util.ConcurrentModificationException
>> 
>> This works correctly for other list implementation (for example,
>> replacing ArrayList with LinkedList makes this code working). As
>> Collection.spliterator() spec says:
>> 
>>> In order to preserve expected laziness behavior for the stream() and
>>> parallelStream() methods, spliterators should either have the
>>> characteristic of IMMUTABLE or CONCURRENT, or be late-binding. If
>>> none of these is practical, the overriding class should describe the
>>> spliterator's documented policy of binding and structural
>>> interference, and should override the stream() and parallelStream()
>>> methods to create streams using a Supplier of the spliterator
>> 
>> None of these is true for ArrayList.subList().spliterator(): it's not
>> IMMUTABLE, not CONCURRENT, not late-binding, the binding policy is not
>> documented and stream()/parallelStream() methods are not overridden.
>> 
>> Is this an issue?
>> 
>> Actually to my opinion late-binding spliterator behavior is a mistake.
>> Nobody cares about it as nobody modifies the source between stream
>> creation and terminal operation call. On the other hand, this
>> requirement adds implementation complexity and I already found the
>> second late-binding problem in existing code (the first one was with
>> Pattern.splitAsStream). Also it's not documented that Stream.concat
>> may bind the late-binding source, but nobody cares anyways. Life would
>> be easier without late-binding spliterators.
>> 
>> With best regards,
>> Tagir Valeev.
>> 

===8<===End of original message text===



-- 
Best regards,
 Tagirmailto:amae...@gmail.com



Re: RFR-8148838: Stream.flatMap(...).spliterator() cannot properly split after tryAdvance()

2016-02-03 Thread Paul Sandoz
Hi Tagir,

Test updates look good, thanks, that should reduce the test times on some of 
our test machines.

I still disagree and pushing back on the support for splitting after partial 
traversal. It’s not a pattern i think we should encourage and propagate so such 
behaviour can be generally relied upon, and predominantly for edge cases. 
That’s where the complexity string is being pulled on. While your fix in 
isolation is not terribly complex, it is more complex than the alternative 
(which was actually the intent of the current impl, we just forget to include 
the check).

Paul.

> On 3 Feb 2016, at 12:19, Tagir F. Valeev  wrote:
> 
> Hello!
> 
> Here's updated webrev:
> http://cr.openjdk.java.net/~tvaleev/webrev/8148838/r2/
> 
> PS> My inclination is to keep this simple and not support splitting after 
> partial traversal.
> 
> PS> Sometimes splitting after partial traversal might be more complex
> PS> not support (e.g. ArrayList). In other cases it’s more complex to
> PS> support and in such cases i would argue it is not worth it since
> PS> this kind of functionality is an edge case.
> 
> I would still like that this problem is fixed (i.e. support splitting
> after advance, not just return null). This is probably an edge case
> for JDK, but might be crucial for library writers. As a library writer
> I actually had bad times working around this issue. While returning
> null instead of incorrect splitting would indeed be helpful, it will
> unnecessarily remove parallelization capabilities in some cases. For
> example, sometimes it's desired to extract the first element from the
> stream and create the stream from the tail. Returning null here would
> mean that the resulting stream will never split after that.
> 
> To my opinion this fix is not very complex. It just adds several lines
> into single method (trySplit()). I added a comment which clarifies the
> intention. Other changes like extraction of initPusher() do not add
> complexity. Actually they reduce the complexity as four identical
> lambdas are merged into one. This patch actually reduces the size of
> created class files. With javac 9-ea+99 StreamSpliterators.java is
> compiled into 79004 bytes after my patch and 79472 bytes before my
> patch. Also this patch does not require primitive specializations and
> adds no overhead for common case when traversal is not started.
> 
> PS> Testing wise you are right to be concerned about the increase in
> PS> test execution time. The lack of flatMap is definitely an
> PS> omission. In this case I think it is ok to replace map with
> PS> flatMap, as both result in stuff going into the holding buffer,
> PS> and we can use the smaller data sets, e.g.
> PS> "StreamTestData.small”, that were recently added.
> 
> I replaced all the datasets with .small alternatives (I think it's ok
> here as WrappingSpliterator behavior does not differ much for various
> sources)  and  removed all .map tests. Now the test works faster
> (like 25 sec -> 17 sec on by box). Please check.
> 
> With best regards,
> Tagir Valeev.
> 
> PS.
> 
> PS> Paul.
> 
>>> On 2 Feb 2016, at 09:24, Tagir F. Valeev  wrote:
>>> 
>>> Please review and sponsor this fix:
>>> 
>>> https://bugs.openjdk.java.net/browse/JDK-8148838
>>> http://cr.openjdk.java.net/~tvaleev/webrev/8148838/r1/
>>> 
>>> When buffer traversal is already started, and split is requested, then
>>> the existing buffer should be carefully transferred to the prefix
>>> part.
>>> 
>>> The only problem I see here is the testing time. Due to
>>> permuteFunctions() call it increases significantly what I added the
>>> flatMap() test. If this becomes an issue, I can write new simple test
>>> which tests flatMap() only (without permutations with other
>>> intermediate operations).
>>> 
>>> With best regards,
>>> Tagir Valeev.
>>> 
> 



RFR(XS) 8148785: Update class file version to 53 for JDK-9

2016-02-03 Thread harold seigel

Hi,

Please review this small change to fix bug 8148785.  The fix bumps the 
class file version to 53 for JDK-9.


Open webrevs:
  http://cr.openjdk.java.net/~hseigel/bug_8148785.jdk/
  http://cr.openjdk.java.net/~hseigel/bug_8148785.hs/

JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8148785

The fix was tested with JCK Lang, VM, and API tests, UTE quick and split 
verifier tests, and the hotspot, JDK vm, java/io, java/lang, and 
java/util JTreg tests, and the new test.


Thanks, Harold


Re: RFR(XS) 8148785: Update class file version to 53 for JDK-9

2016-02-03 Thread Aleksey Shipilev
On 02/03/2016 04:16 PM, harold seigel wrote:
> Open webrevs:
>   http://cr.openjdk.java.net/~hseigel/bug_8148785.jdk/
>   http://cr.openjdk.java.net/~hseigel/bug_8148785.hs/

+1

> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8148785

Description seems to imply we change the compiled class file versions to
53.0. But this change only makes VM to accept 53.0, it does not actually
produce 53.0 files yet. Are we sure the patches do what 8148785 intended?

Anyhow, there seems to be little risk with this particular change.

-Aleksey




RFR: JDK-8148955: libjimage.so compiled with wrong flags

2016-02-03 Thread Erik Joelsson
The library libjimage.so consists of C++ source files, but is currently 
setup to compile using CFLAGS_JDKLIB instead of CXXFLAGS_JDKLIB. On 
Solaris, these variables contain quite a few differences due to the 
compilers taking very different arguments. This makes the build process 
very noisy in the build log.


When correcting the flags, the proper warnings were also enabled. I took 
the liberty of fixing the warning complaining about the variable "i" 
hiding a variable in the outer scope.


Bug: https://bugs.openjdk.java.net/browse/JDK-8148955
Webrev: http://cr.openjdk.java.net/~erikj/8148955/webrev.jdk.01/

/Erik


Re: RFR: JDK-8148955: libjimage.so compiled with wrong flags

2016-02-03 Thread Roger Riggs

+1  (at least for the jdk code)


Thanks Erik

On 02/03/2016 08:50 AM, Erik Joelsson wrote:
The library libjimage.so consists of C++ source files, but is 
currently setup to compile using CFLAGS_JDKLIB instead of 
CXXFLAGS_JDKLIB. On Solaris, these variables contain quite a few 
differences due to the compilers taking very different arguments. This 
makes the build process very noisy in the build log.


When correcting the flags, the proper warnings were also enabled. I 
took the liberty of fixing the warning complaining about the variable 
"i" hiding a variable in the outer scope.


Bug: https://bugs.openjdk.java.net/browse/JDK-8148955
Webrev: http://cr.openjdk.java.net/~erikj/8148955/webrev.jdk.01/

/Erik




Re: RFR(XS) 8148785: Update class file version to 53 for JDK-9

2016-02-03 Thread harold seigel

Hi Aleksey,

Thanks for the review.  The plan is to first change the runtime to 
accept version 53 files and then change the tools to generate them. 
Hopefully, this will reduce incompatibility problems.  See JDK-8148651 
 for details.


Thanks, Harold

On 2/3/2016 8:36 AM, Aleksey Shipilev wrote:

On 02/03/2016 04:16 PM, harold seigel wrote:

Open webrevs:
   http://cr.openjdk.java.net/~hseigel/bug_8148785.jdk/
   http://cr.openjdk.java.net/~hseigel/bug_8148785.hs/

+1


JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8148785

Description seems to imply we change the compiled class file versions to
53.0. But this change only makes VM to accept 53.0, it does not actually
produce 53.0 files yet. Are we sure the patches do what 8148785 intended?

Anyhow, there seems to be little risk with this particular change.

-Aleksey






Re: Stream.foldLeft, one more time (8133680)

2016-02-03 Thread Paul Sandoz
Hi Tagir,

I think this is reasonable, no objections.

Naming-wise reduceLeft might fit better.

Those default methods look good. We might be able to do a little better 
tweaking stuff in ForEachOps to avoid the array box, for some increase in code 
i.e. a clone of ForEachOps.ForEachOp.

Testing-wise see that for take/dropWhile for testing default methods.

Paul.

> On 29 Jan 2016, at 14:37, Tagir F. Valeev  wrote:
> 
> Hello!
> 
> What do you think, is it still reasonable to add foldLeft operation to
> JDK-9 Stream API (foldLeft only, no foldRight)?
> 
> I propose the following signatures:
> 
> interface Stream {
>  public  U foldLeft(U seed, BiFunction accumulator);
>  public Optional foldLeft(BinaryOperator accumulator);
> }
> 
> interface IntStream {
>  public int foldLeft(int seed, IntBinaryOperator accumulator);
>  public OptionalInt foldLeft(IntBinaryOperator accumulator);
> }
> 
> interface LongStream {
>  public OptionalLong foldLeft(LongBinaryOperator accumulator);
>  public long foldLeft(long seed, LongBinaryOperator accumulator);
> }
> 
> interface DoubleStream {
>  public OptionalDouble foldLeft(DoubleBinaryOperator accumulator);
>  public double foldLeft(double seed, DoubleBinaryOperator accumulator);
> }
> 
> They are similar to the corresponding reduce operations, but it's not
> required for accumulator to be associative and it's not required for
> seed to have identity properties. It should be clearly stated in
> JavaDoc that for associative accumulator reduce should be used
> instead.
> 
> All methods can be easily implemented as default methods in the
> interfaces. For example, DoubleStream::foldLeft:
> 
> public double foldLeft(double seed, DoubleBinaryOperator accumulator) {
>double[] box = new double[] { seed };
>forEachOrdered(t -> box[0] = accumulator.applyAsDouble(box[0], t));
>return box[0];
> }
> 
> I can take this issue (if nobody works on it already), but in order
> not to do the useless work I want to be sure that there are no strong
> objections against such feature.
> 
> With best regards,
> Tagir Valeev.
> 



Re: RFR(XS) 8148785: Update class file version to 53 for JDK-9

2016-02-03 Thread Alan Bateman



On 03/02/2016 14:12, harold seigel wrote:

Hi Aleksey,

Thanks for the review.  The plan is to first change the runtime to 
accept version 53 files and then change the tools to generate them. 
Hopefully, this will reduce incompatibility problems.  See JDK-8148651 
 for details.
Right, there are several things that need to happen and I think we need 
hotspot accepting 53.0 before javac starts to generate them.


The webrevs look okay to me, except it might be better to drop "New 
Module attribute" from the comment in classfileParser.cpp until JSR 376 
is further along and we line up the trucks to bring the module system 
into JDK 9.


-Alan


Re: RFR(XS) 8148785: Update class file version to 53 for JDK-9

2016-02-03 Thread harold seigel

Hi Alan,

Thanks for looking at the change.   I'll drop the comment before 
checking it in.


Harold

On 2/3/2016 9:32 AM, Alan Bateman wrote:



On 03/02/2016 14:12, harold seigel wrote:

Hi Aleksey,

Thanks for the review.  The plan is to first change the runtime to 
accept version 53 files and then change the tools to generate them. 
Hopefully, this will reduce incompatibility problems.  See 
JDK-8148651  for 
details.
Right, there are several things that need to happen and I think we 
need hotspot accepting 53.0 before javac starts to generate them.


The webrevs look okay to me, except it might be better to drop "New 
Module attribute" from the comment in classfileParser.cpp until JSR 
376 is further along and we line up the trucks to bring the module 
system into JDK 9.


-Alan




Re: RFR: JDK-8148955: libjimage.so compiled with wrong flags

2016-02-03 Thread Alan Bateman



On 03/02/2016 13:50, Erik Joelsson wrote:
The library libjimage.so consists of C++ source files, but is 
currently setup to compile using CFLAGS_JDKLIB instead of 
CXXFLAGS_JDKLIB. On Solaris, these variables contain quite a few 
differences due to the compilers taking very different arguments. This 
makes the build process very noisy in the build log.


When correcting the flags, the proper warnings were also enabled. I 
took the liberty of fixing the warning complaining about the variable 
"i" hiding a variable in the outer scope.


Bug: https://bugs.openjdk.java.net/browse/JDK-8148955
Webrev: http://cr.openjdk.java.net/~erikj/8148955/webrev.jdk.01/


This looks okay to me.

-Alan.


Re: RFR-8148748: ArrayList.subList().spliterator() is not late-binding

2016-02-03 Thread Martin Buchholz
On Wed, Feb 3, 2016 at 4:20 AM, Tagir F. Valeev  wrote:

> Some more thoughts about forEachRemaining:
>
> To me it seems that
> if ((a = lst.elementData) != null)
> is a redundant check as well as in current ArrayList implementation
> elementData is never null. So it can be replaced with simple
> assignment.

The null check for something that is provably non-null is a sign of
the hand of Doug Lea.
I'm often tempted to try to remove them from j.u.concurrent as well,
but I resist.
They may help hotspot generate better code.


Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

2016-02-03 Thread Thomas Stüfe
On Wed, Feb 3, 2016 at 4:05 AM, David Holmes 
wrote:

> On 3/02/2016 8:08 AM, Stuart Marks wrote:
>
>> Hi Roger,
>>
>> It will be good to get this into the JDK. Lots of people have been
>> asking for this.
>>
>
> I think this API is a big mistake. The primary usecase seems to be
> control-C interception for utilities like jshell. Adding a general purpose
> signal raising and handling mechanism to the JDK does not seem like a good
> solution to me. While you would need to use signal management under the
> covers I think it would be much cleaner to expose an API that actually
> captures what it is you want here: a mechanism to manage "interrupt" and
> "terminate" events at the VM level, in a clean cross-platform way.
>
>
I agree with David. One problem I see is that it is difficult to write
portable java applications with this API. Not only are WIndows and Posix
are very different, but also there are also sublte differences between
Posix platforms. For instance, in the jbs SIGTRAP was mentioned as a
possible signal someone wanted to raise, but SIGTRAP is used by the JIT in
the powerpc port. So applications using Signal.of("SIGTRAP") would run fine
on x86, but cause a crash on powerpc.

Kind Regards, Thomas

Aside: If you want to see some prior art in this area look at
> PosixSignalHandler API in the Real-Time Specification for Java.
>
> Which reminds me - do you propose to support the POSIX real-time signals?
>
> David
> -
>
>
> I have a few comments on the API.
>>
>> 1) Is there a way to query the set of signals supported? This might be a
>> Set returned by a static method, for example. I agree that
>> signal strings outside this set shouldn't be supported.
>>
>> 2) The Signal class spec mentions SIGINT, SIGHUP, and SIGTERM
>> explicitly. Are these required to be implemented on all platforms, or
>> just on "unix-like" platforms, are they just examples? What signals are
>> available on Windows?
>>
>> 3) raise() is spec'd to throw an exception if there's no handler
>> registered. But wouldn't it make sense to allow it if the default
>> handler is registered?
>>
>> 4) In an earlier message you said that the Signal object is a
>> capability, so the security check is on getting a reference. It seems to
>> me that setting a handler is in a different category from raising a
>> signal; this suggests to me that using the same object as a capability
>> for both should be rethought.
>>
>> 5) I don't understand the asymmetry between register() and unregister().
>> Your earlier exchanges with Chris and with Gerard touched on this,
>> specifically, the requirement that the caller pass unregister() a
>> reference to the old handler in order for unregistration to work. You
>> had said this was safer, if there are uncoordinated pieces of code
>> attempting to set/unset signal handlers.
>>
>> It looks to me like this API is really about maintaining process global
>> state consisting of a single handler -- user-specified or default -- for
>> each supported signal. (I agree that it shouldn't try to have a stack or
>> a chain of handlers.) There are a few other things that are global like
>> this, such as the security manager and policy, System.setIn/Out/Err, and
>> so forth. As such, uncoordinated access to the signal API is pretty much
>> broken no matter what. Thus I don't think it makes sense to have a
>> CAS-like protocol for unregistering a handler, to protect against the
>> case where "somebody else" might have registered a handler different
>> from yours.
>>
>> Something like this might make sense:
>>
>>  void register(Consumer handler);
>>  void unregister();
>>
>> The register() call would be pretty much as currently specified; the
>> unregister() call would restore the default handler. Alternatively,
>> register(null) could be used instead of unregister(), but this is quite
>> minor.
>>
>> Thanks,
>>
>> s'marks
>>
>>
>>
>>
>>
>> On 2/1/16 8:02 AM, Roger Riggs wrote:
>>
>>> Please review an API addition to handle signals such as SIGINT,
>>> SIGHUP, and
>>> SIGTERM.
>>> This JEP 260 motivated alternative to sun.misc.Signal supports the use
>>> case for
>>> interactive applications that need to handle Control-C and other signals.
>>>
>>> The new java.util.Signal class provides a settable primary signal
>>> handler and a
>>> default
>>> signal handler.  The primary signal handler can be unregistered and
>>> handling is
>>> restored
>>> to the default signal handler.  System initialization registers
>>> default signal
>>> handlers
>>> to terminate on SIGINT, SIGHUP, and SIGTERM.  Use of the Signal API
>>> requires
>>> a permission if a SecurityManager is set.
>>>
>>> The sun.misc.Signal implementation is modified to be layered on a common
>>> thread and dispatch mechanism. The VM handling of native signals is
>>> not affected.
>>> The command option to reduce signal use by the runtime with -Xrs is
>>> unmodified.
>>>
>>> The changes to hotspot are minimal to rename the hardcoded callback to
>>> the Java
>>> Si

Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

2016-02-03 Thread Chris Hegarty

On 03/02/16 15:20, Thomas Stüfe wrote:

On Wed, Feb 3, 2016 at 4:05 AM, David Holmes 
wrote:


On 3/02/2016 8:08 AM, Stuart Marks wrote:


Hi Roger,

It will be good to get this into the JDK. Lots of people have been
asking for this.



I think this API is a big mistake. The primary usecase seems to be
control-C interception for utilities like jshell. Adding a general purpose
signal raising and handling mechanism to the JDK does not seem like a good
solution to me. While you would need to use signal management under the
covers I think it would be much cleaner to expose an API that actually
captures what it is you want here: a mechanism to manage "interrupt" and
"terminate" events at the VM level, in a clean cross-platform way.



I agree with David. One problem I see is that it is difficult to write
portable java applications with this API. Not only are WIndows and Posix
are very different, but also there are also sublte differences between
Posix platforms. For instance, in the jbs SIGTRAP was mentioned as a
possible signal someone wanted to raise, but SIGTRAP is used by the JIT in
the powerpc port. So applications using Signal.of("SIGTRAP") would run fine
on x86, but cause a crash on powerpc.


I accept the sentiment of your mail, but I suspect that
Signal.of("SIGTRAP") would throw UOE with this API, and not crash
the VM, otherwise it is a bug.

-Chris.


Kind Regards, Thomas

Aside: If you want to see some prior art in this area look at

PosixSignalHandler API in the Real-Time Specification for Java.

Which reminds me - do you propose to support the POSIX real-time signals?

David
-


I have a few comments on the API.


1) Is there a way to query the set of signals supported? This might be a
Set returned by a static method, for example. I agree that
signal strings outside this set shouldn't be supported.

2) The Signal class spec mentions SIGINT, SIGHUP, and SIGTERM
explicitly. Are these required to be implemented on all platforms, or
just on "unix-like" platforms, are they just examples? What signals are
available on Windows?

3) raise() is spec'd to throw an exception if there's no handler
registered. But wouldn't it make sense to allow it if the default
handler is registered?

4) In an earlier message you said that the Signal object is a
capability, so the security check is on getting a reference. It seems to
me that setting a handler is in a different category from raising a
signal; this suggests to me that using the same object as a capability
for both should be rethought.

5) I don't understand the asymmetry between register() and unregister().
Your earlier exchanges with Chris and with Gerard touched on this,
specifically, the requirement that the caller pass unregister() a
reference to the old handler in order for unregistration to work. You
had said this was safer, if there are uncoordinated pieces of code
attempting to set/unset signal handlers.

It looks to me like this API is really about maintaining process global
state consisting of a single handler -- user-specified or default -- for
each supported signal. (I agree that it shouldn't try to have a stack or
a chain of handlers.) There are a few other things that are global like
this, such as the security manager and policy, System.setIn/Out/Err, and
so forth. As such, uncoordinated access to the signal API is pretty much
broken no matter what. Thus I don't think it makes sense to have a
CAS-like protocol for unregistering a handler, to protect against the
case where "somebody else" might have registered a handler different
from yours.

Something like this might make sense:

  void register(Consumer handler);
  void unregister();

The register() call would be pretty much as currently specified; the
unregister() call would restore the default handler. Alternatively,
register(null) could be used instead of unregister(), but this is quite
minor.

Thanks,

s'marks





On 2/1/16 8:02 AM, Roger Riggs wrote:


Please review an API addition to handle signals such as SIGINT,
SIGHUP, and
SIGTERM.
This JEP 260 motivated alternative to sun.misc.Signal supports the use
case for
interactive applications that need to handle Control-C and other signals.

The new java.util.Signal class provides a settable primary signal
handler and a
default
signal handler.  The primary signal handler can be unregistered and
handling is
restored
to the default signal handler.  System initialization registers
default signal
handlers
to terminate on SIGINT, SIGHUP, and SIGTERM.  Use of the Signal API
requires
a permission if a SecurityManager is set.

The sun.misc.Signal implementation is modified to be layered on a common
thread and dispatch mechanism. The VM handling of native signals is
not affected.
The command option to reduce signal use by the runtime with -Xrs is
unmodified.

The changes to hotspot are minimal to rename the hardcoded callback to
the Java
Signal dispatcher.

Please review and comment on the API and implementation.

javadoc:
http://cr.ope

Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

2016-02-03 Thread Thomas Stüfe
Hi Chris,

On Wed, Feb 3, 2016 at 4:22 PM, Chris Hegarty 
wrote:

> On 03/02/16 15:20, Thomas Stüfe wrote:
>
>> On Wed, Feb 3, 2016 at 4:05 AM, David Holmes 
>> wrote:
>>
>> On 3/02/2016 8:08 AM, Stuart Marks wrote:
>>>
>>> Hi Roger,

 It will be good to get this into the JDK. Lots of people have been
 asking for this.


>>> I think this API is a big mistake. The primary usecase seems to be
>>> control-C interception for utilities like jshell. Adding a general
>>> purpose
>>> signal raising and handling mechanism to the JDK does not seem like a
>>> good
>>> solution to me. While you would need to use signal management under the
>>> covers I think it would be much cleaner to expose an API that actually
>>> captures what it is you want here: a mechanism to manage "interrupt" and
>>> "terminate" events at the VM level, in a clean cross-platform way.
>>>
>>>
>>> I agree with David. One problem I see is that it is difficult to write
>> portable java applications with this API. Not only are WIndows and Posix
>> are very different, but also there are also sublte differences between
>> Posix platforms. For instance, in the jbs SIGTRAP was mentioned as a
>> possible signal someone wanted to raise, but SIGTRAP is used by the JIT in
>> the powerpc port. So applications using Signal.of("SIGTRAP") would run
>> fine
>> on x86, but cause a crash on powerpc.
>>
>
> I accept the sentiment of your mail, but I suspect that
> Signal.of("SIGTRAP") would throw UOE with this API, and not crash
> the VM, otherwise it is a bug.
>
>
Of course, you are right. From a user standpoint the result is the same,
though.

..Thomas



> -Chris.
>
>
> Kind Regards, Thomas
>>
>> Aside: If you want to see some prior art in this area look at
>>
>>> PosixSignalHandler API in the Real-Time Specification for Java.
>>>
>>> Which reminds me - do you propose to support the POSIX real-time signals?
>>>
>>> David
>>> -
>>>
>>>
>>> I have a few comments on the API.
>>>

 1) Is there a way to query the set of signals supported? This might be a
 Set returned by a static method, for example. I agree that
 signal strings outside this set shouldn't be supported.

 2) The Signal class spec mentions SIGINT, SIGHUP, and SIGTERM
 explicitly. Are these required to be implemented on all platforms, or
 just on "unix-like" platforms, are they just examples? What signals are
 available on Windows?

 3) raise() is spec'd to throw an exception if there's no handler
 registered. But wouldn't it make sense to allow it if the default
 handler is registered?

 4) In an earlier message you said that the Signal object is a
 capability, so the security check is on getting a reference. It seems to
 me that setting a handler is in a different category from raising a
 signal; this suggests to me that using the same object as a capability
 for both should be rethought.

 5) I don't understand the asymmetry between register() and unregister().
 Your earlier exchanges with Chris and with Gerard touched on this,
 specifically, the requirement that the caller pass unregister() a
 reference to the old handler in order for unregistration to work. You
 had said this was safer, if there are uncoordinated pieces of code
 attempting to set/unset signal handlers.

 It looks to me like this API is really about maintaining process global
 state consisting of a single handler -- user-specified or default -- for
 each supported signal. (I agree that it shouldn't try to have a stack or
 a chain of handlers.) There are a few other things that are global like
 this, such as the security manager and policy, System.setIn/Out/Err, and
 so forth. As such, uncoordinated access to the signal API is pretty much
 broken no matter what. Thus I don't think it makes sense to have a
 CAS-like protocol for unregistering a handler, to protect against the
 case where "somebody else" might have registered a handler different
 from yours.

 Something like this might make sense:

   void register(Consumer handler);
   void unregister();

 The register() call would be pretty much as currently specified; the
 unregister() call would restore the default handler. Alternatively,
 register(null) could be used instead of unregister(), but this is quite
 minor.

 Thanks,

 s'marks





 On 2/1/16 8:02 AM, Roger Riggs wrote:

 Please review an API addition to handle signals such as SIGINT,
> SIGHUP, and
> SIGTERM.
> This JEP 260 motivated alternative to sun.misc.Signal supports the use
> case for
> interactive applications that need to handle Control-C and other
> signals.
>
> The new java.util.Signal class provides a settable primary signal
> handler and a
> default
> signal handler.  The primary signal handler can be

Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

2016-02-03 Thread Thomas Stüfe
On Wed, Feb 3, 2016 at 4:20 PM, Thomas Stüfe 
wrote:

>
>
> On Wed, Feb 3, 2016 at 4:05 AM, David Holmes 
> wrote:
>
>> On 3/02/2016 8:08 AM, Stuart Marks wrote:
>>
>>> Hi Roger,
>>>
>>> It will be good to get this into the JDK. Lots of people have been
>>> asking for this.
>>>
>>
>> I think this API is a big mistake. The primary usecase seems to be
>> control-C interception for utilities like jshell. Adding a general purpose
>> signal raising and handling mechanism to the JDK does not seem like a good
>> solution to me. While you would need to use signal management under the
>> covers I think it would be much cleaner to expose an API that actually
>> captures what it is you want here: a mechanism to manage "interrupt" and
>> "terminate" events at the VM level, in a clean cross-platform way.
>>
>>
> I agree with David. One problem I see is that it is difficult to write
> portable java applications with this API. Not only are WIndows and Posix
> are very different, but also there are also sublte differences between
> Posix platforms. For instance, in the jbs SIGTRAP was mentioned as a
> possible signal someone wanted to raise, but SIGTRAP is used by the JIT in
> the powerpc port. So applications using Signal.of("SIGTRAP") would run fine
> on x86, but cause a crash on powerpc.
>
> Kind Regards, Thomas
>
>
Just occurred to me that a second, subtle problem may be that once java
developers start using signals for their own application, we are barred
from using the same signals in the future for our own purposes, even if we
do not use them now. At least not without breaking those java applications.

..Thomas



> Aside: If you want to see some prior art in this area look at
>> PosixSignalHandler API in the Real-Time Specification for Java.
>>
>> Which reminds me - do you propose to support the POSIX real-time signals?
>>
>> David
>> -
>>
>>
>> I have a few comments on the API.
>>>
>>> 1) Is there a way to query the set of signals supported? This might be a
>>> Set returned by a static method, for example. I agree that
>>> signal strings outside this set shouldn't be supported.
>>>
>>> 2) The Signal class spec mentions SIGINT, SIGHUP, and SIGTERM
>>> explicitly. Are these required to be implemented on all platforms, or
>>> just on "unix-like" platforms, are they just examples? What signals are
>>> available on Windows?
>>>
>>> 3) raise() is spec'd to throw an exception if there's no handler
>>> registered. But wouldn't it make sense to allow it if the default
>>> handler is registered?
>>>
>>> 4) In an earlier message you said that the Signal object is a
>>> capability, so the security check is on getting a reference. It seems to
>>> me that setting a handler is in a different category from raising a
>>> signal; this suggests to me that using the same object as a capability
>>> for both should be rethought.
>>>
>>> 5) I don't understand the asymmetry between register() and unregister().
>>> Your earlier exchanges with Chris and with Gerard touched on this,
>>> specifically, the requirement that the caller pass unregister() a
>>> reference to the old handler in order for unregistration to work. You
>>> had said this was safer, if there are uncoordinated pieces of code
>>> attempting to set/unset signal handlers.
>>>
>>> It looks to me like this API is really about maintaining process global
>>> state consisting of a single handler -- user-specified or default -- for
>>> each supported signal. (I agree that it shouldn't try to have a stack or
>>> a chain of handlers.) There are a few other things that are global like
>>> this, such as the security manager and policy, System.setIn/Out/Err, and
>>> so forth. As such, uncoordinated access to the signal API is pretty much
>>> broken no matter what. Thus I don't think it makes sense to have a
>>> CAS-like protocol for unregistering a handler, to protect against the
>>> case where "somebody else" might have registered a handler different
>>> from yours.
>>>
>>> Something like this might make sense:
>>>
>>>  void register(Consumer handler);
>>>  void unregister();
>>>
>>> The register() call would be pretty much as currently specified; the
>>> unregister() call would restore the default handler. Alternatively,
>>> register(null) could be used instead of unregister(), but this is quite
>>> minor.
>>>
>>> Thanks,
>>>
>>> s'marks
>>>
>>>
>>>
>>>
>>>
>>> On 2/1/16 8:02 AM, Roger Riggs wrote:
>>>
 Please review an API addition to handle signals such as SIGINT,
 SIGHUP, and
 SIGTERM.
 This JEP 260 motivated alternative to sun.misc.Signal supports the use
 case for
 interactive applications that need to handle Control-C and other
 signals.

 The new java.util.Signal class provides a settable primary signal
 handler and a
 default
 signal handler.  The primary signal handler can be unregistered and
 handling is
 restored
 to the default signal handler.  System initialization registers
 default signal

Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

2016-02-03 Thread Chris Hegarty

On 03/02/16 15:43, Thomas Stüfe wrote:

On Wed, Feb 3, 2016 at 4:20 PM, Thomas Stüfe 
wrote:


On Wed, Feb 3, 2016 at 4:05 AM, David Holmes 
wrote:


On 3/02/2016 8:08 AM, Stuart Marks wrote:


Hi Roger,

It will be good to get this into the JDK. Lots of people have been
asking for this.



I think this API is a big mistake. The primary usecase seems to be
control-C interception for utilities like jshell. Adding a general purpose
signal raising and handling mechanism to the JDK does not seem like a good
solution to me. While you would need to use signal management under the
covers I think it would be much cleaner to expose an API that actually
captures what it is you want here: a mechanism to manage "interrupt" and
"terminate" events at the VM level, in a clean cross-platform way.



I agree with David. One problem I see is that it is difficult to write
portable java applications with this API. Not only are WIndows and Posix
are very different, but also there are also sublte differences between
Posix platforms. For instance, in the jbs SIGTRAP was mentioned as a
possible signal someone wanted to raise, but SIGTRAP is used by the JIT in
the powerpc port. So applications using Signal.of("SIGTRAP") would run fine
on x86, but cause a crash on powerpc.

Kind Regards, Thomas



Just occurred to me that a second, subtle problem may be that once java
developers start using signals for their own application, we are barred
from using the same signals in the future for our own purposes, even if we
do not use them now. At least not without breaking those java applications.


This is an excellent point. The API should include appropriate wording
to indicate that there is no guarantee that an invocation of
Signal.of(...) that succeeds in one release, is guaranteed to succeed
in another, future, release.

-Chris.


..Thomas




Aside: If you want to see some prior art in this area look at

PosixSignalHandler API in the Real-Time Specification for Java.

Which reminds me - do you propose to support the POSIX real-time signals?

David
-


I have a few comments on the API.


1) Is there a way to query the set of signals supported? This might be a
Set returned by a static method, for example. I agree that
signal strings outside this set shouldn't be supported.

2) The Signal class spec mentions SIGINT, SIGHUP, and SIGTERM
explicitly. Are these required to be implemented on all platforms, or
just on "unix-like" platforms, are they just examples? What signals are
available on Windows?

3) raise() is spec'd to throw an exception if there's no handler
registered. But wouldn't it make sense to allow it if the default
handler is registered?

4) In an earlier message you said that the Signal object is a
capability, so the security check is on getting a reference. It seems to
me that setting a handler is in a different category from raising a
signal; this suggests to me that using the same object as a capability
for both should be rethought.

5) I don't understand the asymmetry between register() and unregister().
Your earlier exchanges with Chris and with Gerard touched on this,
specifically, the requirement that the caller pass unregister() a
reference to the old handler in order for unregistration to work. You
had said this was safer, if there are uncoordinated pieces of code
attempting to set/unset signal handlers.

It looks to me like this API is really about maintaining process global
state consisting of a single handler -- user-specified or default -- for
each supported signal. (I agree that it shouldn't try to have a stack or
a chain of handlers.) There are a few other things that are global like
this, such as the security manager and policy, System.setIn/Out/Err, and
so forth. As such, uncoordinated access to the signal API is pretty much
broken no matter what. Thus I don't think it makes sense to have a
CAS-like protocol for unregistering a handler, to protect against the
case where "somebody else" might have registered a handler different
from yours.

Something like this might make sense:

  void register(Consumer handler);
  void unregister();

The register() call would be pretty much as currently specified; the
unregister() call would restore the default handler. Alternatively,
register(null) could be used instead of unregister(), but this is quite
minor.

Thanks,

s'marks





On 2/1/16 8:02 AM, Roger Riggs wrote:


Please review an API addition to handle signals such as SIGINT,
SIGHUP, and
SIGTERM.
This JEP 260 motivated alternative to sun.misc.Signal supports the use
case for
interactive applications that need to handle Control-C and other
signals.

The new java.util.Signal class provides a settable primary signal
handler and a
default
signal handler.  The primary signal handler can be unregistered and
handling is
restored
to the default signal handler.  System initialization registers
default signal
handlers
to terminate on SIGINT, SIGHUP, and SIGTERM.  Use of the Signal API
requires
a permission if a S

Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

2016-02-03 Thread Roger Riggs

Hi,

The current wording is explicit about signal support being 
implementation and os dependent,

it can say its release dependent too.

Roger



On 2/3/2016 11:00 AM, Chris Hegarty wrote:

On 03/02/16 15:43, Thomas Stüfe wrote:

On Wed, Feb 3, 2016 at 4:20 PM, Thomas Stüfe 
wrote:


On Wed, Feb 3, 2016 at 4:05 AM, David Holmes 
wrote:


On 3/02/2016 8:08 AM, Stuart Marks wrote:


Hi Roger,

It will be good to get this into the JDK. Lots of people have been
asking for this.



I think this API is a big mistake. The primary usecase seems to be
control-C interception for utilities like jshell. Adding a general 
purpose
signal raising and handling mechanism to the JDK does not seem like 
a good
solution to me. While you would need to use signal management under 
the

covers I think it would be much cleaner to expose an API that actually
captures what it is you want here: a mechanism to manage 
"interrupt" and

"terminate" events at the VM level, in a clean cross-platform way.



I agree with David. One problem I see is that it is difficult to write
portable java applications with this API. Not only are WIndows and 
Posix

are very different, but also there are also sublte differences between
Posix platforms. For instance, in the jbs SIGTRAP was mentioned as a
possible signal someone wanted to raise, but SIGTRAP is used by the 
JIT in
the powerpc port. So applications using Signal.of("SIGTRAP") would 
run fine

on x86, but cause a crash on powerpc.

Kind Regards, Thomas



Just occurred to me that a second, subtle problem may be that once java
developers start using signals for their own application, we are barred
from using the same signals in the future for our own purposes, even 
if we
do not use them now. At least not without breaking those java 
applications.


This is an excellent point. The API should include appropriate wording
to indicate that there is no guarantee that an invocation of
Signal.of(...) that succeeds in one release, is guaranteed to succeed
in another, future, release.

-Chris.


..Thomas




Aside: If you want to see some prior art in this area look at

PosixSignalHandler API in the Real-Time Specification for Java.

Which reminds me - do you propose to support the POSIX real-time 
signals?


David
-


I have a few comments on the API.


1) Is there a way to query the set of signals supported? This 
might be a

Set returned by a static method, for example. I agree that
signal strings outside this set shouldn't be supported.

2) The Signal class spec mentions SIGINT, SIGHUP, and SIGTERM
explicitly. Are these required to be implemented on all platforms, or
just on "unix-like" platforms, are they just examples? What 
signals are

available on Windows?

3) raise() is spec'd to throw an exception if there's no handler
registered. But wouldn't it make sense to allow it if the default
handler is registered?

4) In an earlier message you said that the Signal object is a
capability, so the security check is on getting a reference. It 
seems to

me that setting a handler is in a different category from raising a
signal; this suggests to me that using the same object as a 
capability

for both should be rethought.

5) I don't understand the asymmetry between register() and 
unregister().

Your earlier exchanges with Chris and with Gerard touched on this,
specifically, the requirement that the caller pass unregister() a
reference to the old handler in order for unregistration to work. You
had said this was safer, if there are uncoordinated pieces of code
attempting to set/unset signal handlers.

It looks to me like this API is really about maintaining process 
global
state consisting of a single handler -- user-specified or default 
-- for
each supported signal. (I agree that it shouldn't try to have a 
stack or
a chain of handlers.) There are a few other things that are global 
like
this, such as the security manager and policy, 
System.setIn/Out/Err, and
so forth. As such, uncoordinated access to the signal API is 
pretty much

broken no matter what. Thus I don't think it makes sense to have a
CAS-like protocol for unregistering a handler, to protect against the
case where "somebody else" might have registered a handler different
from yours.

Something like this might make sense:

  void register(Consumer handler);
  void unregister();

The register() call would be pretty much as currently specified; the
unregister() call would restore the default handler. Alternatively,
register(null) could be used instead of unregister(), but this is 
quite

minor.

Thanks,

s'marks





On 2/1/16 8:02 AM, Roger Riggs wrote:


Please review an API addition to handle signals such as SIGINT,
SIGHUP, and
SIGTERM.
This JEP 260 motivated alternative to sun.misc.Signal supports 
the use

case for
interactive applications that need to handle Control-C and other
signals.

The new java.util.Signal class provides a settable primary signal
handler and a
default
signal handler.  The primary signal handler ca

Re: RFR JDK-8141491: Unaligned memory access in Bits.c

2016-02-03 Thread Mikael Vidstedt


On 2016-02-03 01:43, Andrew Haley wrote:

On 02/02/16 19:25, Mikael Vidstedt wrote:

Please review this change which introduces a Copy::conjoint_swap and an
Unsafe.copySwapMemory method to call it from Java, along with the
necessary changes to have java.nio.Bits call it instead of the Bits.c code.

There doesn't seem to be any way to use a byte-swap instruction
in the swapping code.  This will make it unnecessarily slow.


To be clear, this isn't trying to provide the absolutely most optimal 
copy+swap implementation. It's trying to fix the Bits.c unaligned bug 
and pave the way for further improvements. Further performance 
improvements here are certainly possible, but at this point I'm happy as 
long as the performance is on par (or better) with the Bits.c 
implementation it's replacing.


That said, at least gcc seems to recognize the byte swapping pattern and 
does emit a bswap on linux-x64. I'm not sure about the other platforms 
though.


Cheers,
Mikael



Re: RFR JDK-8141491: Unaligned memory access in Bits.c

2016-02-03 Thread Andrew Haley
On 02/03/2016 04:13 PM, Mikael Vidstedt wrote:
> 
> On 2016-02-03 01:43, Andrew Haley wrote:
>> On 02/02/16 19:25, Mikael Vidstedt wrote:
>>> Please review this change which introduces a Copy::conjoint_swap and an
>>> Unsafe.copySwapMemory method to call it from Java, along with the
>>> necessary changes to have java.nio.Bits call it instead of the Bits.c code.
>> There doesn't seem to be any way to use a byte-swap instruction
>> in the swapping code.  This will make it unnecessarily slow.
> 
> To be clear, this isn't trying to provide the absolutely most optimal 
> copy+swap implementation. It's trying to fix the Bits.c unaligned bug 
> and pave the way for further improvements. Further performance 
> improvements here are certainly possible, but at this point I'm happy as 
> long as the performance is on par (or better) with the Bits.c 
> implementation it's replacing.

Got it, sure.  It's just nice to be able to replace low-level routines
with platform ones.

> That said, at least gcc seems to recognize the byte swapping pattern and 
> does emit a bswap on linux-x64. I'm not sure about the other platforms 
> though.

Oh, very nice.  Right, I'll check that once your patch does in.

Andrew.




Re: RFR JDK-7071819: To support Extended Grapheme Clusters in Regex

2016-02-03 Thread Xueming Shen

Hi Peter,

Thanks for the review and suggestion.

This appears to be a better approach. I was wondering if I should go 
this way to cache

those lookup tables as well, but ...

Webrev has been updated as suggested.

Thanks!
Sherman

On 2/3/16 3:26 AM, Peter Levart wrote:

Hi Again,

I also have a comment on the implementation of the hash table and it's 
GC-ness. You keep the string pool under a SoftReference because it is 
the biggest object so it makes most sense to clear it when heap 
becomes full. Other arrays are kept strongly reachable, but you 
re-generate them nevertheless when string pool is cleared by GC. Would 
it make sense to:


- define all int[] arrays (including strPool) as final instance 
variables of CharacterName

- have one static field:

private static SoftReference refInstance;

- convert initNamePool() into a private constructor.

- convert public static getName/getCodePoint into public instance methods

- introduce public static method:

public static CharacterName getInstance() {
SoftReference ref = refInstance;
CharacterName instance;
if (ref != null && ((instance = ref.get) != null) {
return instance;
}
instance = new CharacterName();
refInstance = new SoftReference<>(instance);
return instance;
}

...in this arrangement, you don't need volatile fields or explicit 
fences, as all instance fields can be final and JMM guarantees safe 
publication in this case.



Regards, Peter

On 02/03/2016 12:07 PM, Peter Levart wrote:

Hi Sherman,

I don't currently have any idea how to squeeze the hashtable any more 
further. It is already very compact in my opinion. But I noticed a 
data race that could result in navigating the half-initialized data 
structure. It is a classical unsafe publication bug. It has been 
present before in get(int cp) and it is present now in both 
getName(int cp) and getCodePoint(String name). For example:


 151 static int getCodePoint(String name) {
 152 byte[] strPool = null;
 153 if (refStrPool == null || (strPool = refStrPool.get()) 
== null) {

 154 strPool = initNamePool();
 155 }

vs.

111 refStrPool = new SoftReference<>(strPool);

...the static refStrPool field is not marked volatile.

One way to fix this is to mark field volatile and then rearrange code 
in getName/getCodePoint to only read from it once by introducing a 
local var. The other would be to change the line 111 into something like:


SoftReference rsp = new SoftReference<>(strPool);
unsafe.storeFence();
refStrPool = rsp;

...*and* also rearrange code in getName/getCodePoint to only read 
from field once by introducing a local var.



Regards, Peter

On 02/02/2016 10:25 PM, Xueming Shen wrote:

Hi,

Have not heard any feedback on this one so far. I'm adding
a little more to make it attractive for reviewers :-)

On top of the \N now the webrev includes the proposal to add
two more matchers, \X for unicode extended grapheme cluster
and \b{g} for the corresponding boundary.

Issue: https://bugs.openjdk.java.net/browse/JDK-7071819
Issue: https://bugs.openjdk.java.net/browse/JDK-8147531
webrev: http://cr.openjdk.java.net/~sherman/8147531_7071819/webrev/

Thanks!
Sherman

On 01/18/2016 11:52 PM, Xueming Shen wrote:

Hi,

Please help review the change to add \N support in regex.

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

This is one of the items we were planning to address via JEP111
http://openjdk.java.net/jeps/111
https://bugs.openjdk.java.net/browse/JDK-8046101

Some of the constructs had been added already in early release. I'm
planning to address the rest as individual rfe separately.

Thanks,
Sherman










Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-02-03 Thread Steve Drach
Thanks for the comments Alan.  Responses in-line.

>> I created a new webrev, 
>> http://cr.openjdk.java.net/~sdrach/8132734/webrev.05/ 
>> , that implements 
>> what I outlined above.  In particular I enhanced the JarEntryIterator class 
>> and I added additional commentary to the entries() and stream() methods.  I 
>> also added a new test, MultiReleaseJarIterators, to test entries() and 
>> stream().
>> 
> I think having stream and entries do this is right although I would like to 
> see some performance data if possible.

I’ll see what I can do.  I suspect the non-multi-release jar will be very 
comparable since there’s just a couple boolean tests that were added for this 
case.

> Also I would expect that if a JAR file is not mult-release but the library 
> opens it with Release.RUNTIME to perform the same as opening the JAR file 
> with the Release-less constructors.

Perhaps.  There is a slightly different path with an additional method call and 
boolean test in this case, but I’ll try to get some metrics here too.

> 
> I think the javadoc will need to also need to make it clear whether entries 
> with names starting with META-INF/versions/ are returned.

It was a bit difficult to explain in a succinct way, but the careful reader 
should be able to infer that the META-INF/versions/ entries are not returned 
when the constructor with the Release argument is invoked.  I’ll try to add 
some additional detail.

> 
> I see you've added @since 9 to the existing methods, I assume you didn't mean 
> to do this.

I did mean to do it, but now that you mention it, I see it was a mistake.  I’ll 
fix that.

> 
> At some point then we need to discuss how RUNTIME_VERSION is computed. Iris 
> (via Mandy) has pushed jdk.Version to jdk9/dev but having it exported by 
> java.base conflicts with the design principles in JEP 200. Moving it to 
> another module means that code in java.base cannot use it and thus the JAR 
> file can't use it.

I guess I need to wait until that settles down a bit.



[DING] Re: [PING] Potential infinite waiting at JMXConnection#createConnection

2016-02-03 Thread KUBOTA Yuji
Hi all,

Could someone please review and sponsor this fix ?
I write the details of this issue again. Please review it.

=Problem=
Potential infinite waiting at TCPChannel#createConnection.

This method flushes the DataOutputStream without the socket
timeout settings when choose stream protocol [1]. If connection lost
or the destination server do not return response during the flush,
this method wait forever because the timeout settings is set the
default value of SO_TIMEOUT, i.e., infinite.

[1]: 
http://hg.openjdk.java.net/jdk9/dev/jdk/file/7adef1c3afd5/src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPChannel.java#l227

I think this issue is rarely, however serious.

=Reproduce=
I write a test program to reproduce. You can reproduce by the below.

* hg clone 
http://icedtea.classpath.org/people/ykubota/fixLoopAtJMXConnectorFactory/
* cd fixLoopAtJMXConnectorFactory; mvn package
* setting "stop_time" at debugcontrol.properties if you need.
* java -cp .:target/debugcontrol-1.0-SNAPSHOT.jar debugcontrol.DebugController

This program keep to wait at TCPChannel#createConnection due to
this issue. After "debugcontroltest.stop_time" ms, this program release
the waiting by sending quit to jdb which is stopping the destination
server. Finally, return 2.

=Solution=
Set timeout by using property-configured value:
sun.rmi.transport.tcp.responseTimeout.

My patch is below.
http://icedtea.classpath.org/people/ykubota/fixLoopAtJMXConnectorFactory/file/e31044f0804f/jdk9.patch

If you run the test program with modified JDK9 by my patch, the test
program will get java.net.SocketTimeoutException after the connection
timeout happen, then return 0.

Thanks,
Yuji.


2016-01-13 23:31 GMT+09:00 KUBOTA Yuji :
> Hi all,
>
> Can somebody please review and sponsor this fix ?
>
> Thanks,
> Yuji
>
> 2016-01-05 17:56 GMT+09:00 KUBOTA Yuji :
>> Hi Jaroslav and core-libs-dev,
>>
>> Thank Jaroslav for your kindness!
>>
>> For core-libs-dev members, links the information about this issue.
>>
>>  * details of problem
>>  http://mail.openjdk.java.net/pipermail/jdk9-dev/2015-April/002152.html
>>
>>  * patch
>>  
>> http://icedtea.classpath.org/people/ykubota/fixLoopAtJMXConnectorFactory/file/e31044f0804f/jdk9.patch
>>
>>  * testcase for reproduce
>>  
>> http://icedtea.classpath.org/people/ykubota/fixLoopAtJMXConnectorFactory/file/e31044f0804f/testProgram
>>  
>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2015-December/018415.html
>>
>> Could you please review these reports?
>> Hope this patch helps to community.
>>
>> Thanks,
>> Yuji
>>
>> 2016-01-04 23:51 GMT+09:00 Jaroslav Bachorik :
>>> Hi Yuji,
>>>
>>> On 4.1.2016 15:14, KUBOTA Yuji wrote:

 Hi all,

 Could you please review this patch?
>>>
>>>
>>> Sorry for the long delay. Shanliang has not been present for some time and
>>> probably this slipped the attention of the others.
>>>
>>> However, core-libs mailing list might be more appropriate place to review
>>> this change since you are dealing with s.r.t.t.TCPChannel
>>> (http://icedtea.classpath.org/people/ykubota/fixLoopAtJMXConnectorFactory/file/e31044f0804f/jdk9.patch)
>>>
>>> Regards,
>>>
>>> -JB-


RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

2016-02-03 Thread Iris Clark
Hi, Alan.

> Good to have this in but now we need to decide on where it 
> should live.  It's JDK-specific so we'll need it exported 
> by a JDK module rather than java.base.

8144062 was next on my list.

Do you have any suggestions for the module?  

Of the existing modules in the jdk repository, jdk.dev seems 
to have the most appropriate name.  It appears to only contain 
jimage so I'm not sure about that module's charter.  

Thanks,
iris

-Original Message-
From: Alan Bateman 
Sent: Wednesday, February 03, 2016 12:53 AM
To: Iris Clark; Mandy Chung
Cc: verona-...@openjdk.java.net; core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

On 03/02/2016 03:17, Iris Clark wrote:
> Hi, Mandy.
>
> Thanks so much for pushing the changeset for the initial 
> implementation of jdk.Version!
>
Good to have this in but now we need to decide on where it should live. 
It's JDK-specific so we'll need it exported by a JDK module rather than 
java.base.

-Alan


[9] RFR: 8144593: Suppress not recognized property/feature warning messages from SAXParser

2016-02-03 Thread Aleksej Efimov

Hi,
Please, help to review the fix for JDK-8144593 bug [1] in jaxp area:
   http://cr.openjdk.java.net/~aefimov/8144593/9/00/
Problem:
JDK produces warnings when non-default parser implementation is used and 
this parser doesn't support jaxp features/properties. The quantity of 
such messages can be enormous for long-running application that performs 
frequent xml transformation or validation operations.
The proposed fix suppresses such messages and prints them only once per 
one JVM invocation for each unique pair of: parser class in use AND 
property name that is not supported by this parser.


Testing results:
JDK regression tests (with new regression tests for transformation and 
validation operations), JCK and JPRT shows no xml tests failures.


Best Regards,
Aleksej

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


RFR (S) 8148936: Adapt UUID.toString() to Compact Strings

2016-02-03 Thread Aleksey Shipilev
Hi,

JDK-8006627 did the JavaLangAccess hack to improve UUID.toString()
performance:

public String toString() {
char[] chars = new char[36];
jla.formatUnsignedLong(mostSigBits >> 32, 4, chars, 0, 8);
chars[8] = '-';
jla.formatUnsignedLong(mostSigBits >> 16, 4, chars, 9, 4);
chars[13] = '-';
jla.formatUnsignedLong(mostSigBits, 4, chars, 14, 4);
chars[18] = '-';
jla.formatUnsignedLong(leastSigBits >> 48, 4, chars, 19, 4);
chars[23] = '-';
jla.formatUnsignedLong(leastSigBits, 4, chars, 24, 12);
return jla.newStringUnsafe(chars);
}

This is a good performance improvement, but it clashes with Compact
Strings which now have to re-compress the resulting char[] array into
byte[]. And we know that UUID would always produce Latin1 String.

In fact, the entire JavaLangAccess.newStringUnsafe is now not doing what
power users would expect: it *does* allocate now! So, we need to phase
out that internal gateway to avoid confusion. UUID is one of these users
(StringJoiner is another).

This is the proposed fix:
  http://cr.openjdk.java.net/~shade/8148936/webrev.01/

My attempts in exposing the entire String coder business to UUID proved
to be rather ugly, so I opted to just all into a single method, and let
java/lang internals to sort this out.

The patch does restore the post-Compact Strings performance, and even
improves it further. See:
  http://cr.openjdk.java.net/~shade/8148936/notes.txt

Cheers,
-Aleksey



Re: [9] RFR: 8144593: Suppress not recognized property/feature warning messages from SAXParser

2016-02-03 Thread huizhe wang

Looks good Aleksej!

-Joe

On 2/3/2016 10:19 AM, Aleksej Efimov wrote:

Hi,
Please, help to review the fix for JDK-8144593 bug [1] in jaxp area:
   http://cr.openjdk.java.net/~aefimov/8144593/9/00/
Problem:
JDK produces warnings when non-default parser implementation is used 
and this parser doesn't support jaxp features/properties. The quantity 
of such messages can be enormous for long-running application that 
performs frequent xml transformation or validation operations.
The proposed fix suppresses such messages and prints them only once 
per one JVM invocation for each unique pair of: parser class in use 
AND property name that is not supported by this parser.


Testing results:
JDK regression tests (with new regression tests for transformation and 
validation operations), JCK and JPRT shows no xml tests failures.


Best Regards,
Aleksej

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




Re: RFR: 8079136: Accessing a nested sublist leads to StackOverflowError

2016-02-03 Thread Ivan Gerasimov

Hello!

Here's the updated webrev with rangeCheckForAdd() restored in both 
AbstractList and ArrayList.

http://cr.openjdk.java.net/~igerasim/8079136/07/webrev/

The fix was built/tested successfully on all supported platforms.

Sincerely yours,
Ivan

On 02.02.2016 9:55, Martin Buchholz wrote:

On Mon, Feb 1, 2016 at 10:19 PM, Tagir F. Valeev  wrote:


I have a doubt about replacing rangeCheckForAdd. What if size() ==
Integer.MAX_VALUE? This is not an issue for ArrayList as it's limited
by MAX_ARRAY_SIZE which is Integer.MAX_VALUE - 8.

Actually, the limit is Integer.MAX_VALUE.  But it only grows beyond
MAX_ARRAY_SIZE if there's no choice.





Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

2016-02-03 Thread Roger Riggs

Hi David,

On 2/2/2016 10:05 PM, David Holmes wrote:

On 3/02/2016 8:08 AM, Stuart Marks wrote:

Hi Roger,

It will be good to get this into the JDK. Lots of people have been
asking for this.


I think this API is a big mistake. The primary usecase seems to be 
control-C interception for utilities like jshell. Adding a general 
purpose signal raising and handling mechanism to the JDK does not seem 
like a good solution to me. While you would need to use signal 
management under the covers I think it would be much cleaner to expose 
an API that actually captures what it is you want here: a mechanism to 
manage "interrupt" and "terminate" events at the VM level, in a clean 
cross-platform way.


Aside: If you want to see some prior art in this area look at 
PosixSignalHandler API in the Real-Time Specification for Java.



Looked at it; it provides access to Posix signals with async delivery.
Is there some specific point of interest there?


Which reminds me - do you propose to support the POSIX real-time signals?
No, I don't believe the VM supports them; it is an implementation 
limitation.


Roger



David
-


I have a few comments on the API.

1) Is there a way to query the set of signals supported? This might be a
Set returned by a static method, for example. I agree that
signal strings outside this set shouldn't be supported.

2) The Signal class spec mentions SIGINT, SIGHUP, and SIGTERM
explicitly. Are these required to be implemented on all platforms, or
just on "unix-like" platforms, are they just examples? What signals are
available on Windows?

3) raise() is spec'd to throw an exception if there's no handler
registered. But wouldn't it make sense to allow it if the default
handler is registered?

4) In an earlier message you said that the Signal object is a
capability, so the security check is on getting a reference. It seems to
me that setting a handler is in a different category from raising a
signal; this suggests to me that using the same object as a capability
for both should be rethought.

5) I don't understand the asymmetry between register() and unregister().
Your earlier exchanges with Chris and with Gerard touched on this,
specifically, the requirement that the caller pass unregister() a
reference to the old handler in order for unregistration to work. You
had said this was safer, if there are uncoordinated pieces of code
attempting to set/unset signal handlers.

It looks to me like this API is really about maintaining process global
state consisting of a single handler -- user-specified or default -- for
each supported signal. (I agree that it shouldn't try to have a stack or
a chain of handlers.) There are a few other things that are global like
this, such as the security manager and policy, System.setIn/Out/Err, and
so forth. As such, uncoordinated access to the signal API is pretty much
broken no matter what. Thus I don't think it makes sense to have a
CAS-like protocol for unregistering a handler, to protect against the
case where "somebody else" might have registered a handler different
from yours.

Something like this might make sense:

 void register(Consumer handler);
 void unregister();

The register() call would be pretty much as currently specified; the
unregister() call would restore the default handler. Alternatively,
register(null) could be used instead of unregister(), but this is quite
minor.

Thanks,

s'marks





On 2/1/16 8:02 AM, Roger Riggs wrote:

Please review an API addition to handle signals such as SIGINT,
SIGHUP, and
SIGTERM.
This JEP 260 motivated alternative to sun.misc.Signal supports the use
case for
interactive applications that need to handle Control-C and other 
signals.


The new java.util.Signal class provides a settable primary signal
handler and a
default
signal handler.  The primary signal handler can be unregistered and
handling is
restored
to the default signal handler.  System initialization registers
default signal
handlers
to terminate on SIGINT, SIGHUP, and SIGTERM.  Use of the Signal API
requires
a permission if a SecurityManager is set.

The sun.misc.Signal implementation is modified to be layered on a 
common

thread and dispatch mechanism. The VM handling of native signals is
not affected.
The command option to reduce signal use by the runtime with -Xrs is
unmodified.

The changes to hotspot are minimal to rename the hardcoded callback to
the Java
Signal dispatcher.

Please review and comment on the API and implementation.

javadoc:
   http://cr.openjdk.java.net/~rriggs/signal-doc/

Webrev:
jdk: http://cr.openjdk.java.net/~rriggs/webrev-signal-8087286/
hotspot: http://cr.openjdk.java.net/~rriggs/webrev-hs-signal-8087286/

Issue:
https://bugs.openjdk.java.net/browse/JDK-8087286

JEP 260:
   https://bugs.openjdk.java.net/browse/JDK-8132928

Thanks, Roger






Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

2016-02-03 Thread Stuart Marks



On 2/2/16 7:05 PM, David Holmes wrote:

On 3/02/2016 8:08 AM, Stuart Marks wrote:

It will be good to get this into the JDK. Lots of people have been
asking for this.


I think this API is a big mistake. The primary usecase seems to be control-C
interception for utilities like jshell. Adding a general purpose signal raising
and handling mechanism to the JDK does not seem like a good solution to me.
While you would need to use signal management under the covers I think it would
be much cleaner to expose an API that actually captures what it is you want
here: a mechanism to manage "interrupt" and "terminate" events at the VM level,
in a clean cross-platform way.


OK, I've looked some at the implementation, and there's more going on here than 
meets the eye.


I was under the impression (or at least I was hoping) that the API would expose 
a carefully curated set of signals that are (a) known to be useful to 
applications, and (b) are safe for the JVM to allow applications to handle. 
Examples of this would include SIGWINCH and SIGTSTP, which are common for Unix 
applications to want to handle, as well as the Control-C (SIGINT) case that 
jshell among others want to handle.


But I tried out the patch and looked through the Hotspot signal handling code, 
and the set of signals exposed is much broader than I would have expected. On 
Mac OS X, the signals for which a handler can be registered include the following:


SIGABRT
SIGALRM
SIGBUS
SIGCHLD
SIGCONT
SIGEMT
SIGHUP
SIGINT
SIGIO
SIGPIPE
SIGPROF
SIGSYS
SIGTERM
SIGTRAP
SIGTSTP
SIGTTIN
SIGTTOU
SIGURG
SIGUSR1
SIGUSR2
SIGVTALRM
SIGWINCH
SIGXCPU
SIGXFSZ

I'm quite surprised by this. It seems quite unwise to expose all of these. 
Perhaps this is what David is concerned about. If so, I'm starting to share his 
concern.


In addition, the signals for which a Signal instance can be gotten via 
Signal.of(), but which cannot be handled (throws UOE), include:


SIGFPE
SIGILL
SIGKILL
SIGQUIT
SIGSEGV
SIGSTOP

It's very strange to expose Signal instances representing these signals when 
they can't be handled. They can't be raised either (at least in Roger's first 
patch) since raising a signal is prohibited if there's no handler installed.


I still think a signal-handling API, even a system-specific one, can be useful 
for a well curated set of signals. But this implementation seems to bring an 
internal interface directly out to the API. That doesn't seem like the right 
approach.


s'marks


ObjectInputStream SPI

2016-02-03 Thread Peter Firmstone
In light of recent examples of gadget deserialization attacks, I believe we 
need an OIS SPI.

While OIS functionality can be overridden, there's no way to ensure this can be 
done for all uses of OIS.

I believe this is necessary for security reasons, to allow Serialization to be 
completely disabled or restricted to only those classes in use by an 
application or reimplemented to allow input validation.

An OIS SPI would be a very simple straightforward solution.

Regards,

Peter Firmstone.

Sent from my Samsung device.
 


Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

2016-02-03 Thread David Holmes

Adding back hotspot-runtime-dev

On 4/02/2016 2:07 AM, Roger Riggs wrote:

Hi,

The current wording is explicit about signal support being
implementation and os dependent,
it can say its release dependent too.


So you think an API that is implementation dependent, OS dependent and 
release dependent, is a good candidate for the Write-Once-Run-Anywhere 
Java platform as a core API addition ???


At best this should be in some side add-on (anyone remember javax.*).

David
-



Roger



On 2/3/2016 11:00 AM, Chris Hegarty wrote:

On 03/02/16 15:43, Thomas Stüfe wrote:

On Wed, Feb 3, 2016 at 4:20 PM, Thomas Stüfe 
wrote:


On Wed, Feb 3, 2016 at 4:05 AM, David Holmes 
wrote:


On 3/02/2016 8:08 AM, Stuart Marks wrote:


Hi Roger,

It will be good to get this into the JDK. Lots of people have been
asking for this.



I think this API is a big mistake. The primary usecase seems to be
control-C interception for utilities like jshell. Adding a general
purpose
signal raising and handling mechanism to the JDK does not seem like
a good
solution to me. While you would need to use signal management under
the
covers I think it would be much cleaner to expose an API that actually
captures what it is you want here: a mechanism to manage
"interrupt" and
"terminate" events at the VM level, in a clean cross-platform way.



I agree with David. One problem I see is that it is difficult to write
portable java applications with this API. Not only are WIndows and
Posix
are very different, but also there are also sublte differences between
Posix platforms. For instance, in the jbs SIGTRAP was mentioned as a
possible signal someone wanted to raise, but SIGTRAP is used by the
JIT in
the powerpc port. So applications using Signal.of("SIGTRAP") would
run fine
on x86, but cause a crash on powerpc.

Kind Regards, Thomas



Just occurred to me that a second, subtle problem may be that once java
developers start using signals for their own application, we are barred
from using the same signals in the future for our own purposes, even
if we
do not use them now. At least not without breaking those java
applications.


This is an excellent point. The API should include appropriate wording
to indicate that there is no guarantee that an invocation of
Signal.of(...) that succeeds in one release, is guaranteed to succeed
in another, future, release.

-Chris.


..Thomas




Aside: If you want to see some prior art in this area look at

PosixSignalHandler API in the Real-Time Specification for Java.

Which reminds me - do you propose to support the POSIX real-time
signals?

David
-


I have a few comments on the API.


1) Is there a way to query the set of signals supported? This
might be a
Set returned by a static method, for example. I agree that
signal strings outside this set shouldn't be supported.

2) The Signal class spec mentions SIGINT, SIGHUP, and SIGTERM
explicitly. Are these required to be implemented on all platforms, or
just on "unix-like" platforms, are they just examples? What
signals are
available on Windows?

3) raise() is spec'd to throw an exception if there's no handler
registered. But wouldn't it make sense to allow it if the default
handler is registered?

4) In an earlier message you said that the Signal object is a
capability, so the security check is on getting a reference. It
seems to
me that setting a handler is in a different category from raising a
signal; this suggests to me that using the same object as a
capability
for both should be rethought.

5) I don't understand the asymmetry between register() and
unregister().
Your earlier exchanges with Chris and with Gerard touched on this,
specifically, the requirement that the caller pass unregister() a
reference to the old handler in order for unregistration to work. You
had said this was safer, if there are uncoordinated pieces of code
attempting to set/unset signal handlers.

It looks to me like this API is really about maintaining process
global
state consisting of a single handler -- user-specified or default
-- for
each supported signal. (I agree that it shouldn't try to have a
stack or
a chain of handlers.) There are a few other things that are global
like
this, such as the security manager and policy,
System.setIn/Out/Err, and
so forth. As such, uncoordinated access to the signal API is
pretty much
broken no matter what. Thus I don't think it makes sense to have a
CAS-like protocol for unregistering a handler, to protect against the
case where "somebody else" might have registered a handler different
from yours.

Something like this might make sense:

  void register(Consumer handler);
  void unregister();

The register() call would be pretty much as currently specified; the
unregister() call would restore the default handler. Alternatively,
register(null) could be used instead of unregister(), but this is
quite
minor.

Thanks,

s'marks





On 2/1/16 8:02 AM, Roger Riggs wrote:


Please review an API addition to handle signals such as SIGINT,

Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

2016-02-03 Thread David Holmes

On 4/02/2016 7:15 AM, Roger Riggs wrote:

Hi David,

On 2/2/2016 10:05 PM, David Holmes wrote:

On 3/02/2016 8:08 AM, Stuart Marks wrote:

Hi Roger,

It will be good to get this into the JDK. Lots of people have been
asking for this.


I think this API is a big mistake. The primary usecase seems to be
control-C interception for utilities like jshell. Adding a general
purpose signal raising and handling mechanism to the JDK does not seem
like a good solution to me. While you would need to use signal
management under the covers I think it would be much cleaner to expose
an API that actually captures what it is you want here: a mechanism to
manage "interrupt" and "terminate" events at the VM level, in a clean
cross-platform way.

Aside: If you want to see some prior art in this area look at
PosixSignalHandler API in the Real-Time Specification for Java.


Looked at it; it provides access to Posix signals with async delivery.
Is there some specific point of interest there?


Just an example of how signal interaction in Java has been handled in 
the past.





Which reminds me - do you propose to support the POSIX real-time signals?

No, I don't believe the VM supports them; it is an implementation
limitation.


Not sure what you mean here. The VM doesn't use them - which kind-of 
makes them ideal for application code use as they won't interfere with 
the "normal" signals which have very specific purposes and mostly are 
not intended for general use.


David


Roger



David
-


I have a few comments on the API.

1) Is there a way to query the set of signals supported? This might be a
Set returned by a static method, for example. I agree that
signal strings outside this set shouldn't be supported.

2) The Signal class spec mentions SIGINT, SIGHUP, and SIGTERM
explicitly. Are these required to be implemented on all platforms, or
just on "unix-like" platforms, are they just examples? What signals are
available on Windows?

3) raise() is spec'd to throw an exception if there's no handler
registered. But wouldn't it make sense to allow it if the default
handler is registered?

4) In an earlier message you said that the Signal object is a
capability, so the security check is on getting a reference. It seems to
me that setting a handler is in a different category from raising a
signal; this suggests to me that using the same object as a capability
for both should be rethought.

5) I don't understand the asymmetry between register() and unregister().
Your earlier exchanges with Chris and with Gerard touched on this,
specifically, the requirement that the caller pass unregister() a
reference to the old handler in order for unregistration to work. You
had said this was safer, if there are uncoordinated pieces of code
attempting to set/unset signal handlers.

It looks to me like this API is really about maintaining process global
state consisting of a single handler -- user-specified or default -- for
each supported signal. (I agree that it shouldn't try to have a stack or
a chain of handlers.) There are a few other things that are global like
this, such as the security manager and policy, System.setIn/Out/Err, and
so forth. As such, uncoordinated access to the signal API is pretty much
broken no matter what. Thus I don't think it makes sense to have a
CAS-like protocol for unregistering a handler, to protect against the
case where "somebody else" might have registered a handler different
from yours.

Something like this might make sense:

 void register(Consumer handler);
 void unregister();

The register() call would be pretty much as currently specified; the
unregister() call would restore the default handler. Alternatively,
register(null) could be used instead of unregister(), but this is quite
minor.

Thanks,

s'marks





On 2/1/16 8:02 AM, Roger Riggs wrote:

Please review an API addition to handle signals such as SIGINT,
SIGHUP, and
SIGTERM.
This JEP 260 motivated alternative to sun.misc.Signal supports the use
case for
interactive applications that need to handle Control-C and other
signals.

The new java.util.Signal class provides a settable primary signal
handler and a
default
signal handler.  The primary signal handler can be unregistered and
handling is
restored
to the default signal handler.  System initialization registers
default signal
handlers
to terminate on SIGINT, SIGHUP, and SIGTERM.  Use of the Signal API
requires
a permission if a SecurityManager is set.

The sun.misc.Signal implementation is modified to be layered on a
common
thread and dispatch mechanism. The VM handling of native signals is
not affected.
The command option to reduce signal use by the runtime with -Xrs is
unmodified.

The changes to hotspot are minimal to rename the hardcoded callback to
the Java
Signal dispatcher.

Please review and comment on the API and implementation.

javadoc:
http://cr.openjdk.java.net/~rriggs/signal-doc/

Webrev:
jdk: http://cr.openjdk.java.net/~rriggs/webrev-signal-8087286/
hotspot: http://c

Re: RFR JDK-8141491: Unaligned memory access in Bits.c

2016-02-03 Thread David Holmes

Hi Mikael,

Can't really comment on the bit-twiddling details.

A couple of minor style nits:

- don't put "return" on a line by itself, include the first part of the 
return expression

- spaces after commas in template definitions/instantiation

The JVM_ENTRY_FROM_LEAF etc was a little mind twisting but seems okay.

Otherwise hotspot and JDK code appear okay.

Thanks,
David

On 3/02/2016 5:25 AM, Mikael Vidstedt wrote:


Please review this change which introduces a Copy::conjoint_swap and an
Unsafe.copySwapMemory method to call it from Java, along with the
necessary changes to have java.nio.Bits call it instead of the Bits.c code.

http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.03/hotspot/webrev/

http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.03/jdk/webrev/

On the jdk/ side I don't think there should be a lot of surprises.
Bits.c is gone and that required a mapfile-vers to be changed
accordingly. I also added a relatively extensive
jdk/internal/misc/Unsafe/CopySwap.java test which exercises all the
various copySwap configurations and verifies that the resulting data is
correct. There are also a handful of negative tests in there.

On the hotspot/ side:

* the copy logic in copy.cpp is leveraging templates to help the C++
compiler produce tight copy loops for the various configurations
{element type, copy direction, src aligned, dst aligned}.
* Unsafe_CopySwapMemory is a leaf to not stall safe points more than
necessary. Only if needed (THROW, copy involves heap objects) will it
enter VM using a new JVM_ENTRY_FROM_LEAF macro.
* JVM_ENTRY_FROM_LEAF calls a new VM_ENTRY_BASE_FROM_LEAF helper macro,
which mimics what VM_ENTRY_BASE does, but also does a
debug_only(ResetNoHandleMark __rnhm;) - this is because
JVM_LEAF/VM_LEAF_BASE does debug_only(NoHandleMark __hm;).

I'm in the process of getting the last performance numbers, but from
what I've seen so far this will outperform the earlier implementation.

Cheers,
Mikeal

On 2016-01-27 17:13, Mikael Vidstedt wrote:


Just an FYI:

I'm working on moving all of this to the Hotspot Copy class and
bridging to it via jdk.internal.misc.Unsafe, removing Bits.c
altogether. The implementation is working, and the preliminary
performance numbers beat the pants off of any of the suggested Bits.c
implementations (yay!).

I'm currently in the progress of getting some unit tests in place for
it all to make sure it covers all the corner cases and then I'll run
some real benchmarks to see if it actually lives up to the expectations.

Cheers,
Mikael

On 2016-01-26 11:13, John Rose wrote:

On Jan 26, 2016, at 11:08 AM, Andrew Haley  wrote:

On 01/26/2016 07:04 PM, John Rose wrote:

Unsafe.copyMemory bottoms out to Copy::conjoint_memory_atomic.
IMO that's a better starting point than memcpy.  Perhaps it can be
given an additional parameter (or overloading) to specify a swap size.

OK, but conjoint_memory_atomic doesn't guarantee that destination
words won't be torn if their source is misaligned: in fact it
guarantees that they will will be.

That's a good point, and argues for a new function with the
stronger guarantee.  Actually, it would be perfectly reasonable
to strengthen the guarantee on the existing function.  I don't
think anyone will care about the slight performance change,
especially since it is probably favorable.  Since it's Unsafe,
they are not supposed to care, either.

— John






RFR: 8148928: java/util/stream/test/**/SequentialOpTest.java timed out intermittently

2016-02-03 Thread Hamlin Li

Hi everyone,

Would you please help to review the fix for bug 
https://bugs.openjdk.java.net/browse/JDK-8148928, 
java/util/stream/test/**/SequentialOpTest.java timed out intermittently.

webrev: http://cr.openjdk.java.net/~mli/8148928/webrev.00/

Thank you
-Hamlin


Re: RFR:JDK-8146747:java.time.Duration.toNanos() and toMillis() exception on negative durations

2016-02-03 Thread nadeesh tv


Hi all,

Please see the updated webrev 
http://cr.openjdk.java.net/~ntv/8146747/webrev.01/

Regards,
Nadeesh
On 2/3/2016 5:48 PM, nadeesh tv wrote:

Hi Shinya,
Thnx. I will update it.
Regards,
Nadeesh
On 2/3/2016 5:41 PM, ShinyaYoshida wrote:

Hi Nadeesh,
Almost LGTM!(But I'm not a reviewer;) )
However I've noticed that you don't use NANOS_PER_SECOND at L1223 and 
L1246.

Is there some reason not to use it?

Regards,
shinyafox(Shinya Yoshida)

2016-02-01 15:18 GMT+09:00 nadeesh tv >:


Hi all,

Please review following

Bug Id : https://bugs.openjdk.java.net/browse/JDK-8146747


Solution: Handled the negative duration separately

webrev : http://cr.openjdk.java.net/~ntv/8146747/webrev.00/



-- Thanks and Regards,
Nadeesh TV






--
Thanks and Regards,
Nadeesh TV



JDK 9 RFR of JDK-8149003: Mark more jdk_core tests as intermittently failing

2016-02-03 Thread Amy Lu

java/net/NetworkInterface/NetworkInterfaceStreamTest.java
java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java
java/rmi/Naming/DefaultRegistryPort.java
java/rmi/transport/closeServerSocket/CloseServerSocket.java

Above tests are known to fail intermittently, this patch is to mark the 
test accordingly with keyword 'intermittent'.


bug: https://bugs.openjdk.java.net/browse/JDK-8149003
webrev: http://cr.openjdk.java.net/~amlu/8149003/webrev.00/

Thanks,
Amy

--- old/test/java/net/NetworkInterface/NetworkInterfaceStreamTest.java  
2016-02-04 14:13:21.0 +0800
+++ new/test/java/net/NetworkInterface/NetworkInterfaceStreamTest.java  
2016-02-04 14:13:21.0 +0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -28,6 +28,7 @@
  * @build java.util.stream.OpTestCase
  * @run testng/othervm NetworkInterfaceStreamTest
  * @run testng/othervm -Djava.net.preferIPv4Stack=true 
NetworkInterfaceStreamTest
+ * @key intermittent
  */
 
 import org.testng.annotations.Test;

--- old/test/java/rmi/Naming/DefaultRegistryPort.java   2016-02-04 
14:13:22.0 +0800
+++ new/test/java/rmi/Naming/DefaultRegistryPort.java   2016-02-04 
14:13:22.0 +0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,7 @@
  *  java.rmi/sun.rmi.transport.tcp
  * @build TestLibrary
  * @run main/othervm DefaultRegistryPort
+ * @key intermittent
  */
 
 /*

--- old/test/java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java 
2016-02-04 14:13:23.0 +0800
+++ new/test/java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java 
2016-02-04 14:13:23.0 +0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,7 @@
  *  java.rmi/sun.rmi.transport.tcp
  * @build TestLibrary Legal LegalRegistryNames_Stub
  * @run main/othervm LegalRegistryNames
+ * @key intermittent
  */
 
 import java.net.InetAddress;

--- old/test/java/rmi/transport/closeServerSocket/CloseServerSocket.java
2016-02-04 14:13:23.0 +0800
+++ new/test/java/rmi/transport/closeServerSocket/CloseServerSocket.java
2016-02-04 14:13:23.0 +0800
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -37,6 +37,7 @@
  *  java.rmi/sun.rmi.transport.tcp
  * @build TestLibrary
  * @run main/othervm CloseServerSocket
+ * @key intermittent
  */
 
 import java.io.IOException;





Re: JDK 9 RFR of JDK-8149003: Mark more jdk_core tests as intermittently failing

2016-02-03 Thread Chris Hegarty
Seems reasonable to me May.

-Chris.

On 4 Feb 2016, at 06:22, Amy Lu  wrote:

> java/net/NetworkInterface/NetworkInterfaceStreamTest.java
> java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java
> java/rmi/Naming/DefaultRegistryPort.java
> java/rmi/transport/closeServerSocket/CloseServerSocket.java
> 
> Above tests are known to fail intermittently, this patch is to mark the test 
> accordingly with keyword 'intermittent'.
> 
> bug: https://bugs.openjdk.java.net/browse/JDK-8149003
> webrev: http://cr.openjdk.java.net/~amlu/8149003/webrev.00/
> 
> Thanks,
> Amy
> 
> --- old/test/java/net/NetworkInterface/NetworkInterfaceStreamTest.java
> 2016-02-04 14:13:21.0 +0800
> +++ new/test/java/net/NetworkInterface/NetworkInterfaceStreamTest.java
> 2016-02-04 14:13:21.0 +0800
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
> + * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights 
> reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -28,6 +28,7 @@
>  * @build java.util.stream.OpTestCase
>  * @run testng/othervm NetworkInterfaceStreamTest
>  * @run testng/othervm -Djava.net.preferIPv4Stack=true 
> NetworkInterfaceStreamTest
> + * @key intermittent
>  */
>  import org.testng.annotations.Test;
> --- old/test/java/rmi/Naming/DefaultRegistryPort.java 2016-02-04 
> 14:13:22.0 +0800
> +++ new/test/java/rmi/Naming/DefaultRegistryPort.java 2016-02-04 
> 14:13:22.0 +0800
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights 
> reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -33,6 +33,7 @@
>  *  java.rmi/sun.rmi.transport.tcp
>  * @build TestLibrary
>  * @run main/othervm DefaultRegistryPort
> + * @key intermittent
>  */
>  /*
> --- old/test/java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java   
> 2016-02-04 14:13:23.0 +0800
> +++ new/test/java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java   
> 2016-02-04 14:13:23.0 +0800
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights 
> reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -33,6 +33,7 @@
>  *  java.rmi/sun.rmi.transport.tcp
>  * @build TestLibrary Legal LegalRegistryNames_Stub
>  * @run main/othervm LegalRegistryNames
> + * @key intermittent
>  */
>  import java.net.InetAddress;
> --- old/test/java/rmi/transport/closeServerSocket/CloseServerSocket.java  
> 2016-02-04 14:13:23.0 +0800
> +++ new/test/java/rmi/transport/closeServerSocket/CloseServerSocket.java  
> 2016-02-04 14:13:23.0 +0800
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights 
> reserved.
> + * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights 
> reserved.
>  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>  *
>  * This code is free software; you can redistribute it and/or modify it
> @@ -37,6 +37,7 @@
>  *  java.rmi/sun.rmi.transport.tcp
>  * @build TestLibrary
>  * @run main/othervm CloseServerSocket
> + * @key intermittent
>  */
>  import java.io.IOException;
> 
> 



Re: JDK 9 RFR of JDK-8149003: Mark more jdk_core tests as intermittently failing

2016-02-03 Thread Chris Hegarty
s/May/Amy ;-)

-Chris.

On 4 Feb 2016, at 06:23, Chris Hegarty  wrote:

> Seems reasonable to me May.
> 
> -Chris.
> 
> On 4 Feb 2016, at 06:22, Amy Lu  wrote:
> 
>> java/net/NetworkInterface/NetworkInterfaceStreamTest.java
>> java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java
>> java/rmi/Naming/DefaultRegistryPort.java
>> java/rmi/transport/closeServerSocket/CloseServerSocket.java
>> 
>> Above tests are known to fail intermittently, this patch is to mark the test 
>> accordingly with keyword 'intermittent'.
>> 
>> bug: https://bugs.openjdk.java.net/browse/JDK-8149003
>> webrev: http://cr.openjdk.java.net/~amlu/8149003/webrev.00/
>> 
>> Thanks,
>> Amy
>> 
>> --- old/test/java/net/NetworkInterface/NetworkInterfaceStreamTest.java   
>> 2016-02-04 14:13:21.0 +0800
>> +++ new/test/java/net/NetworkInterface/NetworkInterfaceStreamTest.java   
>> 2016-02-04 14:13:21.0 +0800
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
>> + * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights 
>> reserved.
>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>> *
>> * This code is free software; you can redistribute it and/or modify it
>> @@ -28,6 +28,7 @@
>> * @build java.util.stream.OpTestCase
>> * @run testng/othervm NetworkInterfaceStreamTest
>> * @run testng/othervm -Djava.net.preferIPv4Stack=true 
>> NetworkInterfaceStreamTest
>> + * @key intermittent
>> */
>> import org.testng.annotations.Test;
>> --- old/test/java/rmi/Naming/DefaultRegistryPort.java2016-02-04 
>> 14:13:22.0 +0800
>> +++ new/test/java/rmi/Naming/DefaultRegistryPort.java2016-02-04 
>> 14:13:22.0 +0800
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights 
>> reserved.
>> + * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights 
>> reserved.
>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>> *
>> * This code is free software; you can redistribute it and/or modify it
>> @@ -33,6 +33,7 @@
>> *  java.rmi/sun.rmi.transport.tcp
>> * @build TestLibrary
>> * @run main/othervm DefaultRegistryPort
>> + * @key intermittent
>> */
>> /*
>> --- old/test/java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java  
>> 2016-02-04 14:13:23.0 +0800
>> +++ new/test/java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java  
>> 2016-02-04 14:13:23.0 +0800
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights 
>> reserved.
>> + * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights 
>> reserved.
>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>> *
>> * This code is free software; you can redistribute it and/or modify it
>> @@ -33,6 +33,7 @@
>> *  java.rmi/sun.rmi.transport.tcp
>> * @build TestLibrary Legal LegalRegistryNames_Stub
>> * @run main/othervm LegalRegistryNames
>> + * @key intermittent
>> */
>> import java.net.InetAddress;
>> --- old/test/java/rmi/transport/closeServerSocket/CloseServerSocket.java 
>> 2016-02-04 14:13:23.0 +0800
>> +++ new/test/java/rmi/transport/closeServerSocket/CloseServerSocket.java 
>> 2016-02-04 14:13:23.0 +0800
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights 
>> reserved.
>> + * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights 
>> reserved.
>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>> *
>> * This code is free software; you can redistribute it and/or modify it
>> @@ -37,6 +37,7 @@
>> *  java.rmi/sun.rmi.transport.tcp
>> * @build TestLibrary
>> * @run main/othervm CloseServerSocket
>> + * @key intermittent
>> */
>> import java.io.IOException;
>> 
>> 
> 



Re: JDK 9 RFR of JDK-8149003: Mark more jdk_core tests as intermittently failing

2016-02-03 Thread joe darcy

Hi Amy,

Looks fine; thanks,

-Joe

On 2/3/2016 10:22 PM, Amy Lu wrote:

java/net/NetworkInterface/NetworkInterfaceStreamTest.java
java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java
java/rmi/Naming/DefaultRegistryPort.java
java/rmi/transport/closeServerSocket/CloseServerSocket.java

Above tests are known to fail intermittently, this patch is to mark 
the test accordingly with keyword 'intermittent'.


bug: https://bugs.openjdk.java.net/browse/JDK-8149003
webrev: http://cr.openjdk.java.net/~amlu/8149003/webrev.00/

Thanks,
Amy

--- old/test/java/net/NetworkInterface/NetworkInterfaceStreamTest.java  
2016-02-04 14:13:21.0 +0800
+++ new/test/java/net/NetworkInterface/NetworkInterfaceStreamTest.java  
2016-02-04 14:13:21.0 +0800
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -28,6 +28,7 @@
   * @build java.util.stream.OpTestCase
   * @run testng/othervm NetworkInterfaceStreamTest
   * @run testng/othervm -Djava.net.preferIPv4Stack=true 
NetworkInterfaceStreamTest
+ * @key intermittent
   */
  
  import org.testng.annotations.Test;

--- old/test/java/rmi/Naming/DefaultRegistryPort.java   2016-02-04 
14:13:22.0 +0800
+++ new/test/java/rmi/Naming/DefaultRegistryPort.java   2016-02-04 
14:13:22.0 +0800
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,7 @@
   *  java.rmi/sun.rmi.transport.tcp
   * @build TestLibrary
   * @run main/othervm DefaultRegistryPort
+ * @key intermittent
   */
  
  /*

--- old/test/java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java 
2016-02-04 14:13:23.0 +0800
+++ new/test/java/rmi/Naming/legalRegistryNames/LegalRegistryNames.java 
2016-02-04 14:13:23.0 +0800
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 1999, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,7 @@
   *  java.rmi/sun.rmi.transport.tcp
   * @build TestLibrary Legal LegalRegistryNames_Stub
   * @run main/othervm LegalRegistryNames
+ * @key intermittent
   */
  
  import java.net.InetAddress;

--- old/test/java/rmi/transport/closeServerSocket/CloseServerSocket.java
2016-02-04 14:13:23.0 +0800
+++ new/test/java/rmi/transport/closeServerSocket/CloseServerSocket.java
2016-02-04 14:13:23.0 +0800
@@ -1,5 +1,5 @@
  /*
- * Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights reserved.
   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
   *
   * This code is free software; you can redistribute it and/or modify it
@@ -37,6 +37,7 @@
   *  java.rmi/sun.rmi.transport.tcp
   * @build TestLibrary
   * @run main/othervm CloseServerSocket
+ * @key intermittent
   */
  
  import java.io.IOException;






Re: RFR JDK-8141491: Unaligned memory access in Bits.c

2016-02-03 Thread John Rose
On Feb 2, 2016, at 11:25 AM, Mikael Vidstedt  wrote:
> Please review this change which introduces a Copy::conjoint_swap and an 
> Unsafe.copySwapMemory method to call it from Java, along with the necessary 
> changes to have java.nio.Bits call it instead of the Bits.c code.
> 
> http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.03/hotspot/webrev/
> http://cr.openjdk.java.net/~mikael/webrevs/8141491/webrev.03/jdk/webrev/

This is very good.

I have some nit-picks:

These days, when we introduce a new intrinsic (@HSIntrCand),
we write the argument checking code separately in a non-intrinsic
bytecode method.  In this case, we don't (yet) have an intrinsic
binding for U.copy*, but we might in the future.  (C intrinsifies
memcpy, which is a precedent.)  In any case, I would prefer
if we could structure the argument checking code in a similar
way, with Unsafe.java containing both copySwapMemory
and a private copySwapMemory0.  Then we can JIT-optimize
the safety checks.

You might as well extend the same treatment to the pre-existing
copyMemory call.  The most important check (and the only one
in U.copyMemory) is to ensure that the size_t operand has not
wrapped around from a Java negative value to a crazy-large
size_t value.  That's the low-hanging fruit.  Checking the pointers
(for null or oob) is more problematic, of course.  Checking consistency
around elemSize is cheap and easy, so I agree that the U.copySM
should do that work also.  Basically, Unsafe can do very basic
checks if there is a tricky user model to enforce, but it mustn't
"sign up" to guard the user against all errors.

Rule of thumb:  Unsafe calls don't throw NPEs, they just SEGV.
And the rare bit that *does* throw (IAE usually) should be placed
into Unsafe.java, not unsafe.cpp.  (The best-practice rule for putting
argument checking code outside of the intrinsic is a newer one,
so Unsafe code might not always do this.)

The comment "Generalizing it would be reasonable, but requires
card marking" is bogus, since we never byte-swap managed pointers.

The test logic will flow a little smoother if your GenericPointer guy,
the onHeap version, stores the appropriate array base offset in his offset 
field.
You won't have to mention p.isOnHeap nearly so much, and the code will
set a slightly better example.

The VM_ENTRY_BASE_FROM_LEAF macro is really cool.

The C++ template code is cool also.  It reminds me of the kind
of work Gosling's "Ace" processor could do, but now it's mainstreamed
for all to use in C++.  We're going to get some of that goodness
in Project Valhalla with specialization logic.

I find it amazing that the right way to code this in C is to
use memcpy for unaligned accesses and byte peek/poke
into registers for byte-swapping operators.  I'm glad we
can write this code *once* for the JVM and JDK.

Possible future work:  If we can get a better handle on
writing vectorizable loops from Java, including Unsafe-based
ones, we can move some of the C code back up to Java.
Perhaps U.copy* calls for very short lengths deserved to
be broken out into small loops of U.get/put* (with alignment).
I think you experimented with this, and there were problems
with the JIT putting fail-safe memory barriers between
U.get/put* calls.  Paul's work on Array.mismatch ran into
similar issues, with the right answer being to write manual
vector code in assembly.

Anyway, you can count me as a reviewer.

Thanks,

— John

Re: RFR: 8148446: (tz) Support tzdata2016a

2016-02-03 Thread Masayoshi Okutsu

Hi Ramanand,

I noticed that the zones in Yakutsk Time [1] seem to have their own 
names, such as "Khandyga Time" for Asia/Khandyga, and you seem to follow 
that convention for Asia/Chita. That causes some mismatch between the 
long names and abbreviations.


I dag out some past tzdata fixes to see how that happened. What I found 
out is that the 2013b fix [2] used "Yakutsk Time", but that the 2013c 
[3] fix used "Khandyga Time". 2013b went to JDK 7 updates and earlier 
ones and 2013c went to 8. JDK 9 inherits the 8 fix.


I prefer to restore the 2013b convention for Asia/Yakutsk, Asia/Chita, 
and Asia/Khandyga to have:


{"Yakutsk Time", "YAKT",
 "Yakutsk Summer Time", "YAKST",
 "Yakutsk Time", "YAKT"}

Thanks,
Masayoshi

[1] https://en.wikipedia.org/wiki/Yakutsk_Time
[2] http://hg.openjdk.java.net/jdk7u/jdk7u-dev/jdk/rev/b8009df64dc8
[3] http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/ae35fdbab949

On 2/2/2016 8:00 PM, Ramanand Patil wrote:

HI all,

Please review the latest TZDATA integration (tzdata2016a) to JDK9.

Bug  - https://bugs.openjdk.java.net/browse/JDK-8148446

Webrev - http://cr.openjdk.java.net/~rpatil/8148446/webrev.00/

  


All the TimeZone related tests are passed after integration.

  


Regards,

Ramanand.




Re: RFR 9: 8087286 Need a way to handle control-C and possibly some other signals

2016-02-03 Thread Thomas Stüfe
>
>
>
>> Which reminds me - do you propose to support the POSIX real-time signals?
>>>
>> No, I don't believe the VM supports them; it is an implementation
>> limitation.
>>
>
> Not sure what you mean here. The VM doesn't use them - which kind-of makes
> them ideal for application code use as they won't interfere with the
> "normal" signals which have very specific purposes and mostly are not
> intended for general use.
>
>
Note that this is also platform dependend, e.g. Mac OS does not support
them. Even for platforms which support them, the semantics differ from
platform to platform, see e.g. http://zinascii.com/2014/crossed-signals.html
.

.. Thomas


> David
>
>
> Roger
>>
>>
>>> David
>>> -
>>>
>>> I have a few comments on the API.

 1) Is there a way to query the set of signals supported? This might be a
 Set returned by a static method, for example. I agree that
 signal strings outside this set shouldn't be supported.

 2) The Signal class spec mentions SIGINT, SIGHUP, and SIGTERM
 explicitly. Are these required to be implemented on all platforms, or
 just on "unix-like" platforms, are they just examples? What signals are
 available on Windows?

 3) raise() is spec'd to throw an exception if there's no handler
 registered. But wouldn't it make sense to allow it if the default
 handler is registered?

 4) In an earlier message you said that the Signal object is a
 capability, so the security check is on getting a reference. It seems to
 me that setting a handler is in a different category from raising a
 signal; this suggests to me that using the same object as a capability
 for both should be rethought.

 5) I don't understand the asymmetry between register() and unregister().
 Your earlier exchanges with Chris and with Gerard touched on this,
 specifically, the requirement that the caller pass unregister() a
 reference to the old handler in order for unregistration to work. You
 had said this was safer, if there are uncoordinated pieces of code
 attempting to set/unset signal handlers.

 It looks to me like this API is really about maintaining process global
 state consisting of a single handler -- user-specified or default -- for
 each supported signal. (I agree that it shouldn't try to have a stack or
 a chain of handlers.) There are a few other things that are global like
 this, such as the security manager and policy, System.setIn/Out/Err, and
 so forth. As such, uncoordinated access to the signal API is pretty much
 broken no matter what. Thus I don't think it makes sense to have a
 CAS-like protocol for unregistering a handler, to protect against the
 case where "somebody else" might have registered a handler different
 from yours.

 Something like this might make sense:

  void register(Consumer handler);
  void unregister();

 The register() call would be pretty much as currently specified; the
 unregister() call would restore the default handler. Alternatively,
 register(null) could be used instead of unregister(), but this is quite
 minor.

 Thanks,

 s'marks





 On 2/1/16 8:02 AM, Roger Riggs wrote:

> Please review an API addition to handle signals such as SIGINT,
> SIGHUP, and
> SIGTERM.
> This JEP 260 motivated alternative to sun.misc.Signal supports the use
> case for
> interactive applications that need to handle Control-C and other
> signals.
>
> The new java.util.Signal class provides a settable primary signal
> handler and a
> default
> signal handler.  The primary signal handler can be unregistered and
> handling is
> restored
> to the default signal handler.  System initialization registers
> default signal
> handlers
> to terminate on SIGINT, SIGHUP, and SIGTERM.  Use of the Signal API
> requires
> a permission if a SecurityManager is set.
>
> The sun.misc.Signal implementation is modified to be layered on a
> common
> thread and dispatch mechanism. The VM handling of native signals is
> not affected.
> The command option to reduce signal use by the runtime with -Xrs is
> unmodified.
>
> The changes to hotspot are minimal to rename the hardcoded callback to
> the Java
> Signal dispatcher.
>
> Please review and comment on the API and implementation.
>
> javadoc:
> http://cr.openjdk.java.net/~rriggs/signal-doc/
>
> Webrev:
> jdk: http://cr.openjdk.java.net/~rriggs/webrev-signal-8087286/
> hotspot: http://cr.openjdk.java.net/~rriggs/webrev-hs-signal-8087286/
>
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8087286
>
> JEP 260:
> https://bugs.openjdk.java.net/browse/JDK-8132928
>
> Thanks, Roger
>
>
>
>>