hg: jdk8/tl/jdk: 8023528: Rename Comparator combinators to disambiguate overloading methods

[henry . jen]

Changeset: be2d25a277a7
Author:henryjen
Date:  2013-08-21 20:41 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/be2d25a277a7

8023528: Rename Comparator combinators to disambiguate overloading methods
Reviewed-by: mduigou, smarks

! src/share/classes/java/util/Comparator.java
! test/java/util/Comparator/BasicTest.java
! test/java/util/Map/EntryComparators.java
! test/java/util/function/BinaryOperator/BasicTest.java

Re: Java 8 RFR 8010430: Math.round has surprising behavior for odd values of ulp 1

[Dmitry Nadezhin]

Is it reasonable to make specification clearer ?

Either to return JLS 1 specification:
<<<
The result is rounded to an integer by adding , taking the floor of the
result, and casting the result to type long.
>>>
or to replace "rounding up" with "rounding to positive infinity":
<<<
Returns the closest {@code int} to the argument, with ties rounding to
positive infinity.
>>>


On Tue, Aug 27, 2013 at 3:19 AM, Brian Burkhalter <
brian.burkhal...@oracle.com> wrote:

> On Aug 26, 2013, at 7:52 AM, Guy Steele wrote:
>
> On Aug 24, 2013, at 3:02 PM, Jeff Hain  wrote:
>
>
> Dmitry Nadezhin wrote:
>
> Nevertheless, I send this variant now in hope that it may be useful.
>
>
> Great! It's much faster than what I proposed, cleaner (only integers),
>
> and according to my tests it behaves the same.
>
>
> Excellent!  Nice piece of work---thanks, Dmitry.
>
>
> Indeed - thanks, Dmitry! That is much better than what I proposed.
>
> I did a quick and dirty JMH test with this code and it looks to be more
> than 6% faster for double and more than 24% faster for float versus the
> current codebase. The speed up is probably more than these values which
> should be considered lower bounds on speed improvement.
>
> I have updated the webrev accordingly:
>
> http://cr.openjdk.java.net/~bpb/8010430/
>
> Approval by a Reviewer of this patch is still needed.
>
> Thanks,
>
> Brian
>

RFR: 7057785 : (xs) Add note to hashCode() that support for self referential is optional

[Mike Duigou]

Hello all;

Fairly frequently it is reported that various Collection/Map implementations of 
hashCode() fail when the instance directly or indirectly contains itself. For a 
variety of reasons, mostly performance and resource related, most 
implementations choose not to support calculation of hash codes for 
self-referential collections. This is not likely to change. So to reduce 
confusion and "bug" reports I am proposing a non-normative @apiNote be added to 
Collection and HashMap. The text of the proposed note is:

> Support for calculation of hash code by self referential {Collection|Map}s 
> (they either directly or indirectly contain themselves) is optional. Few 
> Collection implementations support calculation of hash code for self 
> referential instances.


http://cr.openjdk.java.net/~mduigou/JDK-7057785/0/webrev/

Cheers,

Mike

Re: RFR: 8015068 : (m) Use jtreg -exclude for problemlist.txt processing

[Kelly O'Hair]

Nice.

I think deleting code should be like hazard pay, double the normal hourly wage, 
but deleting Makefile lines should be triple wage. :)  Looks great to me.  Not 
that I use this makefile much anymore.

-kto

On Aug 27, 2013, at 4:17 PM, Mike Duigou wrote:

> Hello all;
> 
> I have updated the changeset for this issue based upon feedback from the 
> earlier version. As a result of intervening work this version contains even 
> more cleanup.
> 
> http://cr.openjdk.java.net/~mduigou/JDK-8015068/1/webrev/
> 
> Since the last revision:
> 
> - One open issue remains--handling of shared_library_permissions. The 
> proposed patch uses a pattern rule, "jdk_%", rather than explicit make 
> targets. This means the knowledge of which targets required 
> shared_library_permissions has been lost. The shared_library_permissions 
> target could be run unconditionally as part of prep but I am uncertain if 
> this is reasonable. Alternatives?
> 
> - The previously proposed behaviour of writing testoutput to the current 
> directory if not ALT_OUTPUTDIR is provided has been changed in response to 
> feedback. Output will now be written to jdk/testoutput and this directory has 
> been added to the .hgignore. Since most people are expected to use only the 
> root repo make which sets ALT_OUTPUTDIR it is not expected that this change 
> will make any difference for most users.
> 
> As before, testing of this patch requires using a source build of JTReg as it 
> requires one fix that is not in the promoted builds. This changeset will not 
> be integrated until after the next JTReg promotion.
> 
> Mike
> 
> On Aug 7 2013, at 14:44 , Mike Duigou wrote:
> 
>> Hello all;
>> 
>> This changesest simplifies how the the jdk/test/Makefile processes excluded 
>> tests. Previously the test exclusions were pre-processed by scripts in the 
>> Makefile before being passed to JTReg. JTReg will now the all the 
>> processing. The change depends upon improvements in JTReg since the test 
>> exclude mechanism was originally defined.
>> 
>> There are some changes in the reporting. Most obvious is that the 
>> excludelist.txt output file is no longer produced. The 'excluded' count in 
>> the summary report may be removed in a future reporting depending on whether 
>> the value can be calculated some other way.
>> 
>> Additional to the exclusion list processing changes some other changes are 
>> possible as a result of the changes to exclusion processing. In particular 
>> the obsolete (and incorrect) logic for determining the build path included 
>> in the test/Makefile is now removed. If no output directory is provided via 
>> ALT_OUTPUTDIR then a default location in the current directory is used, 
>> "testoutput". At some point in the future this could be improved to get the 
>> CONF from the build configuration but that is not currently possible.
>> 
>> Testing of this patch requires using a source build of JTReg as it requires 
>> one fix that is not in the promoted builds. 
>> 
>> http://cr.openjdk.java.net/~mduigou/JDK-8015068/0/webrev/
>> 
>> Mike
> 

RFR: JDK-4792059 -- test/java/io/pathNames/GeneralSolaris.java fails on symbolic links

[Dan Xu]

Hi,

When GeneralSolaris testcase follows symbolic link to pick up an 
existing file or directory for testing, it will fail the assertion in 
check()method because the file path canonicalization process will result 
in the real path not a path containing symbolic link. I enforce this 
test not to use any symbolic link as a testing file ordirectory to avoid 
this problem. Please help review my fix. Thanks!


bug: https://bugs.openjdk.java.net/browse/JDK-4792059
webrev: http://cr.openjdk.java.net/~dxu/4792059/webrev/

-Dan

Re: RFR: 8015068 : (m) Use jtreg -exclude for problemlist.txt processing

[Mike Duigou]

Hello all;

I have updated the changeset for this issue based upon feedback from the 
earlier version. As a result of intervening work this version contains even 
more cleanup.

http://cr.openjdk.java.net/~mduigou/JDK-8015068/1/webrev/

Since the last revision:

- One open issue remains--handling of shared_library_permissions. The proposed 
patch uses a pattern rule, "jdk_%", rather than explicit make targets. This 
means the knowledge of which targets required shared_library_permissions has 
been lost. The shared_library_permissions target could be run unconditionally 
as part of prep but I am uncertain if this is reasonable. Alternatives?

- The previously proposed behaviour of writing testoutput to the current 
directory if not ALT_OUTPUTDIR is provided has been changed in response to 
feedback. Output will now be written to jdk/testoutput and this directory has 
been added to the .hgignore. Since most people are expected to use only the 
root repo make which sets ALT_OUTPUTDIR it is not expected that this change 
will make any difference for most users.

As before, testing of this patch requires using a source build of JTReg as it 
requires one fix that is not in the promoted builds. This changeset will not be 
integrated until after the next JTReg promotion.

Mike

On Aug 7 2013, at 14:44 , Mike Duigou wrote:

> Hello all;
> 
> This changesest simplifies how the the jdk/test/Makefile processes excluded 
> tests. Previously the test exclusions were pre-processed by scripts in the 
> Makefile before being passed to JTReg. JTReg will now the all the processing. 
> The change depends upon improvements in JTReg since the test exclude 
> mechanism was originally defined.
> 
> There are some changes in the reporting. Most obvious is that the 
> excludelist.txt output file is no longer produced. The 'excluded' count in 
> the summary report may be removed in a future reporting depending on whether 
> the value can be calculated some other way.
> 
> Additional to the exclusion list processing changes some other changes are 
> possible as a result of the changes to exclusion processing. In particular 
> the obsolete (and incorrect) logic for determining the build path included in 
> the test/Makefile is now removed. If no output directory is provided via 
> ALT_OUTPUTDIR then a default location in the current directory is used, 
> "testoutput". At some point in the future this could be improved to get the 
> CONF from the build configuration but that is not currently possible.
> 
> Testing of this patch requires using a source build of JTReg as it requires 
> one fix that is not in the promoted builds. 
> 
> http://cr.openjdk.java.net/~mduigou/JDK-8015068/0/webrev/
> 
> Mike

Re: RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.

[Xueming Shen]

On 08/27/2013 03:07 PM, Martin Buchholz wrote:

It does seem vaguely reasonable to support any extra data.

Don't you want to also handle arbitrary byte arrays, if e.g. one the 16-bit 
size fields overflows the extra data?
It looks to me like getExtraLen could return a negative number.



probably I should. The webrev has been updated to simply
copy the rest of the extra data, if the "sz" is either < 0 or
out of the range.

http://cr.openjdk.java.net/~sherman/8023713/webrev/



And put a SPACE after "if".


updated.

Thanks!
-Sherman




On Tue, Aug 27, 2013 at 2:42 PM, Xueming Shen mailto:xueming.s...@oracle.com>> wrote:

Hi,

Please help review the change for #8023713

http://cr.openjdk.java.net/~sherman/8023713/webrev 


The root cause is that the newly introduced ZOS.writeExtra() (for
#8015666) fails to handle "irregular" extra data field. The zip spec
requires the the extra data stars with 4 bytes of "tag + size" pair
and then followed by the actual "extra data". The "offending" zip
file actually has the "irregular" extra data field with 1 single byte
as the extra data. That said, the implementation (ZOS) should still
be able handle this kind of zip entry correctly and appropriately.

The proposed solution is to simply copy the specified extra data
into the output stream.

Thanks!
-Sherman

Re: RFR: 8021591 : (s) Additional explicit null checks

[Mike Duigou]

Hello all;

Here's an updated version of the patch which incorporates feedback and improves 
the tests (the reason for delay):

http://cr.openjdk.java.net/~mduigou/JDK-8021591/1/webrev/

The substance of the patch is largely to add missing checks that the collection 
provided to removeAll()/retainAll() is not null. The specification of these 
methods in the Collection interface has always indicated that an NPE should be 
thrown if the passed collection was null. Historically various implementations 
were inconsistent about whether they threw the NPE if a null collection was 
passed. Some collections would throw the NPE, some would not. The intent of 
this patch is to improve consistency and since there were examples of the NPE 
being correctly thrown the most prudent approach seems to have all 
implementations throw the NPE. If there had been no examples of the NPE being 
thrown then it would have been more prudent to amend the interface spec to 
remove the NPE notice.

A few other inconsistencies around null handling were also resolved. Some unit 
tests issues were also cleaned up.

Mike

On Jul 26 2013, at 16:31 , Mike Duigou wrote:

> Hello all;
> 
> This patch adds some missing checks for null that, according to interface 
> contract, should be throwing NPE. It also improves the existing tests to 
> check for these cases. 
> 
> http://cr.openjdk.java.net/~mduigou/JDK-8021591/0/webrev/
> 
> The changes to src/share/classes/java/util/concurrent/ConcurrentHashMap.java 
> will be synchronized separately with the jsr166 workspace. They are part of 
> this review to avoid test failures.
> 
> Mike

Re: Take 2 Re: RFR 8023155: Ensure functional consistency across Random, ThreadLocalRandom, SplittableRandom

[Mike Duigou]

Looks good.

Random::

- Seems fine.


ThreadLocalRandom::

- I don't understand the point of having a writeObject() if the readResolve() 
ignores the result. My expectation for a serialized TLR might be that upon 
de-serialization the seeding state is restored. If that isn't provided, why 
offer any serialization at all? Alternately we should be more explicit that the 
seeding state is not part of the serialization.

- There's no test for the serialization behaviour.

SplittableRandomTest::

- executeAndCatch -> assertThrows perhaps? There are a few implementations of 
assertThrows around in other tests (which haven't been collected into a library 
yet).


Mike

On Aug 27 2013, at 12:06 , Paul Sandoz wrote:

> Updated:
> 
>  http://cr.openjdk.java.net/~psandoz/tl/JDK-8023155-Random-TLR-SR-sync/webrev/
> 
> - we reverted the addition of the new next* methods on Random. The 
> stream-based methods remain unchanged. Decided to be extra conservative, 
> since there may be sub-classes that define such methods (not unusual e.g. 
> just like TLR) and the contract might be different to what we specify 
> (probably most likely around error handling but there could be other subtle 
> issues).
> 
> - the stream-based int origin/bound support is defined using nextInt methods 
> rather than next, which should be better when used with existing sub-classes.
> 
> - some additional tests were added for double-related origin/bounds methods 
> of Random, ThreadLocalRandom, and SplittableRandom.
> 
> Note for TLR default constructors the seed is not yet based on the same 
> algorithm as SplittableRandom. As discussed in a previous email we cannot do 
> that until the hash seed functionality is removed from WeakHashMap and 
> Hashtable.

Soon I hope.



> Paul.
> 
> 

Re: RFR 8023463 Update HashMap and LinkedHashMap to use bins/buckets or trees (red/black)

[Remi Forax]

On 08/27/2013 03:51 PM, Martin Buchholz wrote:

On Tue, Aug 27, 2013 at 3:00 AM, Paul Sandoz  wrote:


[...]


Note that j.u. classes are quite inconsistent in this respect to using
diamonds and @Overrides. My preference is to do a sweeping change to all
such code.


For jsr166, we continue to maintain some support for jdk6 and use javac6 in
places, and so we've been known to de-<> for portability.   We're also not
huge fans of using @Override pervasively.


From a reader perspective, @Override is great to figure out which part 
of the code

is part of the supertypes contract and which part is specific to the class.

Rémi

RFR JDK-8023713: ZipFileSystem has compatiable issue to handle old zip file.

[Xueming Shen]

Hi,

Please help review the change for #8023713

http://cr.openjdk.java.net/~sherman/8023713/webrev

The root cause is that the newly introduced ZOS.writeExtra() (for
#8015666) fails to handle "irregular" extra data field. The zip spec
requires the the extra data stars with 4 bytes of "tag + size" pair
and then followed by the actual "extra data". The "offending" zip
file actually has the "irregular" extra data field with 1 single byte
as the extra data. That said, the implementation (ZOS) should still
be able handle this kind of zip entry correctly and appropriately.

The proposed solution is to simply copy the specified extra data
into the output stream.

Thanks!
-Sherman

Re: RFR 8023463 Update HashMap and LinkedHashMap to use bins/buckets or trees (red/black)

[Remi Forax]

On 08/27/2013 12:00 PM, Paul Sandoz wrote:

On Aug 26, 2013, at 11:13 PM, Remi Forax  wrote:


On 08/26/2013 10:10 PM, Paul Sandoz wrote:

On Aug 25, 2013, at 8:04 PM, Remi Forax  wrote:


On 08/21/2013 02:25 PM, Paul Sandoz wrote:

Hi,

Here are Doug's Linked/HashMap changes, discussed in a previous thread, as a 
webrev:

   
http://cr.openjdk.java.net/~psandoz/tl/JDK-8023463-Linked-HashMap-bin-and-tree/webrev/

I also added some tests related to characteristics associated with fixing 
another bug.

Looking at the diffs will be tricky given there are so many changes.

The code can be simplified a little bit by using the diamond syntax,
HashMap (lines: 919, 963, 1026, 1497, 1569) and
LinkedHashMap (lines: 255, 264, 270, 278)

There are a bunch of @Override that are missing making the code harder than it 
should to read.


Yes, i think this is because it sticks to the 166 style i suspect. Easy to 
change.


Note that j.u. classes are quite inconsistent in this respect to using diamonds 
and @Overrides. My preference is to do a sweeping change to all such code. I 
wonder if an IDE can automate that?


Eclipse has a 'cleanup tool' for that. So it is able to add @Override 
but unfortunately, not to add diamonds.


[...]


Thanks.

Here is the latest patch that also includes updates for the above 2 comments 
and Mike's comments:

   
http://cr.openjdk.java.net/~psandoz/tl/JDK-8023463-Linked-HashMap-bin-and-tree/webrev/

Paul.


It looks good for me.

Rémi

hg: jdk8/tl/jdk: 8023647: "abc1c".matches("(\\w)+1\\1")) returns false

[xueming . shen]

Changeset: 3f6777cbfe69
Author:sherman
Date:  2013-08-27 12:54 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3f6777cbfe69

8023647: "abc1c".matches("(\\w)+1\\1")) returns false
Summary: to correct the wrong GroupCurly group index backoff code
Reviewed-by: alanb

! src/share/classes/java/util/regex/Pattern.java
! test/java/util/regex/RegExTest.java

hg: jdk8/tl/langtools: 8023826: Typo in warning about obsolete source / target values

[joe . darcy]

Changeset: 662a5188bded
Author:darcy
Date:  2013-08-27 11:58 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/662a5188bded

8023826: Typo in warning about obsolete source / target values
Reviewed-by: jjg, wmdietl

! src/share/classes/com/sun/tools/javac/main/JavaCompiler.java

hg: jdk8/tl/jdk: 8023275: Wrapping collections should override default methods

[henry . jen]

Changeset: ade440668f94
Author:henryjen
Date:  2013-08-26 22:32 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ade440668f94

8023275: Wrapping collections should override default methods
Reviewed-by: mduigou, psandoz

! src/share/classes/java/util/Collections.java
+ test/java/util/Collections/Wrappers.java

Take 2 Re: RFR 8023155: Ensure functional consistency across Random, ThreadLocalRandom, SplittableRandom

[Paul Sandoz]

Updated:

  http://cr.openjdk.java.net/~psandoz/tl/JDK-8023155-Random-TLR-SR-sync/webrev/

- we reverted the addition of the new next* methods on Random. The stream-based 
methods remain unchanged. Decided to be extra conservative, since there may be 
sub-classes that define such methods (not unusual e.g. just like TLR) and the 
contract might be different to what we specify (probably most likely around 
error handling but there could be other subtle issues).

- the stream-based int origin/bound support is defined using nextInt methods 
rather than next, which should be better when used with existing sub-classes.

- some additional tests were added for double-related origin/bounds methods of 
Random, ThreadLocalRandom, and SplittableRandom.

Note for TLR default constructors the seed is not yet based on the same 
algorithm as SplittableRandom. As discussed in a previous email we cannot do 
that until the hash seed functionality is removed from WeakHashMap and 
Hashtable.

Paul.

hg: jdk8/tl/jdk: 8023827: Fix doclint issues in javax.net.ssl

[joe . darcy]

Changeset: 51151b440e95
Author:darcy
Date:  2013-08-27 11:46 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/51151b440e95

8023827: Fix doclint issues in javax.net.ssl
Reviewed-by: wetmore, xuelei

! src/share/classes/javax/net/ssl/SNIHostName.java
! src/share/classes/javax/net/ssl/X509KeyManager.java

hg: jdk8/tl/langtools: 7052170: javadoc -charset option generates wrong meta tag

[bhavesh . x . patel]

Changeset: 7fb27bc201cc
Author:bpatel
Date:  2013-08-27 11:41 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/langtools/rev/7fb27bc201cc

7052170: javadoc -charset option generates wrong meta tag
Reviewed-by: jjg

! src/share/classes/com/sun/tools/doclets/formats/html/HtmlDocletWriter.java
! src/share/classes/com/sun/tools/doclets/formats/html/markup/HtmlAttr.java
! src/share/classes/com/sun/tools/doclets/formats/html/markup/HtmlDocWriter.java
! src/share/classes/com/sun/tools/doclets/formats/html/markup/HtmlTree.java
+ test/com/sun/javadoc/testCharset/TestCharset.java
+ test/com/sun/javadoc/testCharset/pkg/Foo.java

hg: jdk8/tl/jdk: 2 new changesets

[sean . mullan]

Changeset: 6a1bfcde4d4d
Author:mullan
Date:  2013-08-27 12:04 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/6a1bfcde4d4d

8019830: Add com.sun.media.sound to the list of restricted package
Reviewed-by: vinnie

! src/share/lib/security/java.security-linux
! src/share/lib/security/java.security-macosx
! src/share/lib/security/java.security-solaris
! src/share/lib/security/java.security-windows
! test/java/lang/SecurityManager/CheckPackageAccess.java

Changeset: 3575263eefab
Author:mullan
Date:  2013-08-27 12:27 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/3575263eefab

Merge

- src/share/classes/com/sun/security/auth/PolicyParser.java
- src/share/classes/com/sun/security/auth/SubjectCodeSource.java
- src/share/classes/sun/misc/Compare.java
- src/share/classes/sun/misc/Sort.java

hg: jdk8/tl/jdk: 8023769: JDK-8016850 broke the old build

[sean . mullan]

Changeset: 134283a88499
Author:mullan
Date:  2013-08-27 10:46 -0400
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/134283a88499

8023769: JDK-8016850 broke the old build
Summary: remove files that were moved/removed from 
com/sun/security/auth/FILES_java.gmk
Reviewed-by: chegar, xuelei

! make/com/sun/security/auth/FILES_java.gmk

hg: jdk8/tl/nashorn: 10 new changesets

[sundararajan . athijegannathan]

Changeset: badc919cd621
Author:lagergren
Date:  2013-08-23 14:16 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/badc919cd621

8023550: -d option was broken for any dir but '.'. Fixed Java warnings.
Reviewed-by: jlaskey, sundar

! 
buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ConstructorGenerator.java
! 
buildtools/nasgen/src/jdk/nashorn/internal/tools/nasgen/ScriptClassInstrumentor.java
! src/jdk/internal/dynalink/ChainedCallSite.java
! src/jdk/internal/dynalink/DefaultBootstrapper.java
! src/jdk/internal/dynalink/beans/AbstractJavaLinker.java
! src/jdk/internal/dynalink/beans/OverloadedDynamicMethod.java
! src/jdk/nashorn/api/scripting/NashornScriptEngine.java
! src/jdk/nashorn/internal/codegen/CompilationPhase.java
! src/jdk/nashorn/internal/objects/NativeArray.java
! src/jdk/nashorn/internal/objects/NativeRegExp.java
! src/jdk/nashorn/internal/runtime/Context.java
! src/jdk/nashorn/internal/runtime/DebuggerSupport.java
! src/jdk/nashorn/internal/runtime/ScriptObject.java
! src/jdk/nashorn/internal/runtime/regexp/joni/ArrayCompiler.java
! src/jdk/nashorn/internal/runtime/regexp/joni/BitSet.java
! src/jdk/nashorn/internal/runtime/regexp/joni/ByteCodeMachine.java
! src/jdk/nashorn/internal/runtime/regexp/joni/CodeRangeBuffer.java
! src/jdk/nashorn/internal/runtime/regexp/joni/Lexer.java
! src/jdk/nashorn/internal/runtime/regexp/joni/Parser.java
! src/jdk/nashorn/internal/runtime/regexp/joni/Region.java
! src/jdk/nashorn/internal/runtime/regexp/joni/ScannerSupport.java
! src/jdk/nashorn/internal/runtime/regexp/joni/SearchAlgorithm.java
! src/jdk/nashorn/internal/runtime/regexp/joni/StackMachine.java
! src/jdk/nashorn/internal/runtime/regexp/joni/WarnCallback.java
! src/jdk/nashorn/internal/runtime/regexp/joni/ast/EncloseNode.java
! src/jdk/nashorn/internal/runtime/regexp/joni/ast/Node.java
! src/jdk/nashorn/internal/runtime/regexp/joni/ast/QuantifierNode.java
! src/jdk/nashorn/internal/runtime/regexp/joni/exception/ErrorMessages.java
! tools/fxshell/jdk/nashorn/tools/FXShell.java

Changeset: e2d94d032760
Author:jlaskey
Date:  2013-08-23 09:56 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/e2d94d032760

8020946: TokenType#toString returned null
Reviewed-by: hannesw, lagergren
Contributed-by: james.las...@oracle.com

! src/jdk/nashorn/internal/parser/TokenType.java

Changeset: eb7b8340ce3a
Author:lagergren
Date:  2013-08-23 15:46 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/eb7b8340ce3a

8023454: Updated DEVELOPER_README and command line flags, ensuring that 
undocumented flags that aren't guaranteed to work (disabled by default) and 
that are work in progress show up with an EXPERIMENTAL tag.
Reviewed-by: attila, jlaskey

! docs/DEVELOPER_README
! src/jdk/nashorn/internal/runtime/resources/Options.properties

Changeset: 12820c8d0a5d
Author:jlaskey
Date:  2013-08-23 12:20 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/12820c8d0a5d

8019987: String trimRight and trimLeft could be defined
Reviewed-by: sundar
Contributed-by: james.las...@oracle.com

! src/jdk/nashorn/internal/objects/NativeString.java
+ test/script/basic/JDK-8019987.js

Changeset: c19c66e661a9
Author:hannesw
Date:  2013-08-26 15:59 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/c19c66e661a9

8023650: Regexp m flag does not recognize CRNL or CR
Reviewed-by: jlaskey, lagergren

! src/jdk/nashorn/internal/runtime/regexp/joni/ByteCodeMachine.java
! src/jdk/nashorn/internal/runtime/regexp/joni/Config.java
! src/jdk/nashorn/internal/runtime/regexp/joni/EncodingHelper.java
! src/jdk/nashorn/internal/runtime/regexp/joni/Lexer.java
! src/jdk/nashorn/internal/runtime/regexp/joni/Matcher.java
! src/jdk/nashorn/tools/Shell.java
+ test/script/basic/JDK-8023650.js

Changeset: 99e48c76d11f
Author:jlaskey
Date:  2013-08-26 15:33 -0300
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/99e48c76d11f

8023721: Simplify eval in DebuggerSupport.
Reviewed-by: sundar, lagergren, hannesw
Contributed-by: james.las...@oracle.com

! src/jdk/nashorn/internal/runtime/DebuggerSupport.java

Changeset: 3bd077423a08
Author:sundar
Date:  2013-08-27 15:54 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/3bd077423a08

8022773: ScriptEngineTest.printManyTest fails
Reviewed-by: lagergren, attila

! test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java

Changeset: 47f0a4c4b729
Author:attila
Date:  2013-08-27 13:17 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/47f0a4c4b729

8023780: Gracefully handle @CS methods while binding bean properties
Reviewed-by: jlaskey, lagergren, sundar

! src/jdk/nashorn/internal/objects/NativeObject.java
+ test/script/basic/JDK-8023780.js
+ test/script/basic/JDK-8023780.js.EXPECTED

Changeset: bda0e89f88ae
Author:sundar
Date:  2013-08-27 18:57 +0530
URL:   http://hg.openjdk.java.net/jdk8/tl/nashorn/rev/bda0e89f88ae

8023784: Object.prototype.

Re: RFR: JDK-8023765 -- Improve MaxPathLength.java testcase and reduce its test load

[Dan Xu]

On 08/27/2013 12:12 AM, Alan Bateman wrote:

On 27/08/2013 01:18, Dan Xu wrote:

Hi All,

MaxPathLength.javais a troublesome testcase, and fails intermittently 
in the nightly test. And it also runs for a long time, especially on 
Windows platforms. Inorder to improve the test stability, I remove 
its unnecessary test iterations, and use NIOdelete method todo the 
clean-up to make the potential failureseasier for diagnosis. Please 
review thechanges. Thanks!


bug: https://bugs.openjdk.java.net/browse/JDK-8023765
webrev: http://cr.openjdk.java.net/~dxu/8023765/webrev/

-Dan
The double to quickly skip over the names to MAX_LENGTH/2 looks 
reasonable.


I guess fileCreated should really be fileExists as it may be deleted 
and then deleted. An alternative here would be to use 
Files.deleteIfExists as that would avoid the need to introduce flags 
to track whether the directory and exist exists.


-Alan

Hi Alan,

Those flag names are a little misleading. Sorry about that.

fileCreated and dirCreated are actually tracking the existence of new 
file and directories. If the new file gets deleted, I marked the flag to 
false in the code. And at the end, I also change the recorded file path 
after the rename operation.


I agree that using deleteIfExists is a good alternative. In my original 
thought, I plan to monitor every step and make sure all file operations 
happen as expected. Thanks!


-Dan

Re: Additional source to guess the local timezone ID on linux

[Omair Majid]

On 08/27/2013 03:00 AM, Masayoshi Okutsu wrote:
> /etc/sysconfig/clock used to be supported, but it was removed in JDK 7.
> The problem is discussed in bug #6456628.
> 
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6456628

Thanks for that link. I did not know that this support was present in
JDK7 but then removed. I did not expect various distributions and
versions to have significant differences in that file either.

> Have you tested your fix on all Red Hat-like distros, including some
> older releases, with all the time zone IDs?

No, I tested this with Red Hat Enterprise Linux 6. I did not try it on
older versions. I know newer versions of Fedora don't ship with this
file any more.

Given that this support was removed because there were significant
differences in that file making maintenance harder in OpenJDK, I am
going to withdraw this patch.

Thanks to everyone who reviewed and commented on it.

Omair

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681

Re: RFR JDK-8011940 : java.lang.Class.getAnnotations() always enters synchronized method

[Peter Levart]

Hi Joel and others,

Here's a 3rd revision of this proposed patch:

http://cr.openjdk.java.net/~plevart/jdk8-tl/AnnotationData/webrev.03/

I used LinkedHashMap for annotations in this one. It means that now even 
.getAnnotations() are reported in "declaration order": 1st inherited 
(includes overridden) then declared (that are not overriding). For 
example, using @Inherited annotations A1, A2, A3:


@A1("C")
@A2("C")
class C {}

@A1("D")
@A3("D")
class D extends C {}

D.class.getAnnotations() now returns: { @A1("D"), @A2("C"), @A3("D") } ...

The LHM constructor uses the following expression to estimate the 
initial capacity of the LHM:


3326 annotations = new LinkedHashMap<>((Math.max(
3327 declaredAnnotations.size(),
3328 Math.min(12, declaredAnnotations.size() + 
superAnnotations.size())
3329 ) * 4 + 2) / 3
3330 );


I think this strikes some balance between effectivity and accuracy of 
estimation. I could pre-scan the superclass annotations and calculate 
the exact final size of the annotations Map before constructing it 
though. Tell me if this is worth the effort.


I also created a test that tests 3 things:
- class annotation inheritance
- order of class annotations reported by .getAnnotations() and 
.getDeclaredAnnotations() methods (although not specified, the order is 
now stable and resembles declaration order)

- behaviour of class annotation caching when class is redefined


Regards, Peter

On 08/13/2013 09:52 AM, Joel Borggrén-Franck wrote:

Hi,

Comments inline,

On 12 aug 2013, at 14:40, Peter Levart  wrote:

On 08/12/2013 12:54 PM, Joel Borggren-Franck wrote:
- annotation (@interface)  declarations can themselves be redefined (for 
example, defaults changed). Such redefinitions don't affect already initialized 
annotations. Default values are cached in AnnotationType which is not 
invalidated as a result of class redefinition. Maybe it should be. And even if 
AnnotationType was invalidated as a result of class redefinition, the defaults 
are looked-up when particular annotations are initialized and then combined 
with parsed values in a single values map inside each annotation instance 
(proxy), so invalidating AnnotationType would have no effect on those 
annotations.


3326 if (annotations == null) { // lazy construction
3327 annotations = new HashMap<>();
3328 }

I think this should be a LinkedHashMap, so that iteration is predictable
(I know it isn't in the current code). Also the size of the map is known
so you can use a constructor with an explicit initial capacity.

Right, AnnotationParser does return LinkedHashMap, so at least 
decalredAnnotations are in parse-order. I will change the code to use 
LinkedHashMap for inherited/combined case too.

Thanks.


The number of annotations that end-up in inherited/combined Map is not known in 
advance. I could make a separate pre-scan for superclass annotations that are 
@Inheritable and don't have the same key as any of declared annotations and 
then sum this count with declared annotations count, but I don't think this 
will be very effective. I could allocate a large-enough Map to always fit (the 
count of superclass annotations + the count of declared annotations), but that 
could sometimes lead to much over-allocated Maps. I could take the 
min(DEFAULT_CAPACITY, superclass annotations count + declared annotations 
count) as the initial capacity for the Map. What do you think which of those 3 
alternatives is the best?

My bad. I was thinking of the case where every inherited annotation was 
overridden, in that case annotations.size() == declaredAnnotations.size(). That 
is of course not generally true. I'm fine with handling this as a follow up 
since the situation is no worse than today and the surrounding code is better. 
However,

1) We should really measure this.
2) My gut feeling is that the ratio of not overridden inherited annotations to 
declared annotations is small IE the expected nr of annotations is much closer 
to declare annotations than to declared + superclass annotations.
3) The default initial capacity is 16 and load factor is 0.75. I do think the 
mean nr of annotations is closer to 3 than to 12. We are probably wasting some 
space here.

Perhaps use min(default cap, (total annotations/0.75 (+ 1?))) for now as that 
will make the situation no worse than today and often better?


Since this is a fairly significant rewrite I think it might be good to
make sure our tests exercise the new code. Can you run some kind of
coverage report on this?

I successfully ran the jdk_lang jtreg tests which contain several tests for 
annotations.

I'm a bit worried these tests doesn't cover annotations + class redefine. But I 
might be persuaded to have more extensive testing as a follow up as well.

cheers
/Joel

RE: core-libs-dev Digest, Vol 76, Issue 112

[Salter, Thomas A]

I vote for pedantry, though not necessarily for every NPE.

I've had to port the JDK to a non-standard operating system.  The docs are 
frequently inadequate to answer detailed coding questions, so I end up reading 
code to figure out what is supposed to happen.  Even then, we sometimes don't 
sort things out until we run the conformance tests.

The root cause, of course, is that the same spec serves as both user 
documentation and implementation specification.  It's inevitable that there 
will be too much information for users and too little for implementers.

Tom Salter

--

Message: 3
Date: Mon, 26 Aug 2013 20:16:00 -0700
From: Mike Duigou 
Subject: Re: RFR: 8021591 : (s) Additional explicit null checks
To: "core-libs-dev@openjdk.java.net Libs"

Message-ID: 
Content-Type: text/plain; charset=iso-8859-1


On Aug 26 2013, at 18:37 , Mike Duigou wrote:

> 
> On Aug 19 2013, at 15:35 , Martin Buchholz wrote:
> 
>> My feeling is that the JDK specs have been creeping in the direction of 
>> excessive pedantry and doc lawyerism.  I think it's overall a benefit of 
>> Java that its documentation is more readable (at the cost of being a little 
>> less precise) than the typical ISO spec.
> 
> There is a definite tension here. We would like to keep the documentation and 
> specification as readable as possible while still being sufficiently exacting 
> so that behaviour of an API can be correctly predicted by a reader. It goes 
> further than that though because Oracle employs an entire group of engineers 
> who examine the JDK API javadocs looking for normative statements and then 
> write tests to confirm that implementations conform to the API 
> documentation/specification.  The number and quality of tests they provide to 
> ensure conformance has been steadily increasing (and accelerating). Is this a 
> good thing? To me it seems so. When I hear that people encounter problems 
> (other than performance) when switching among 
> Vector<->ArrayList<->LinkedList<->CopyOnWriteArrayList or 
> HashMap<->ConcurrentHashMap or TreeSet<->ConcurrentSkipListSet because of 
> arbitrary corner case differences between implementations I become smy

Sorry, replying to my own message to finish an incomplete sentence.

...sympathetic to concerns that the JDK docs/specs are not specific enough.

Mike

Re: RFR 8023463 Update HashMap and LinkedHashMap to use bins/buckets or trees (red/black)

[Paul Sandoz]

On Aug 26, 2013, at 11:13 PM, Remi Forax  wrote:

> On 08/26/2013 10:10 PM, Paul Sandoz wrote:
>> On Aug 25, 2013, at 8:04 PM, Remi Forax  wrote:
>> 
>>> On 08/21/2013 02:25 PM, Paul Sandoz wrote:
 Hi,
 
 Here are Doug's Linked/HashMap changes, discussed in a previous thread, as 
 a webrev:
 
   
 http://cr.openjdk.java.net/~psandoz/tl/JDK-8023463-Linked-HashMap-bin-and-tree/webrev/
 
 I also added some tests related to characteristics associated with fixing 
 another bug.
 
 Looking at the diffs will be tricky given there are so many changes.
>>> 
>>> The code can be simplified a little bit by using the diamond syntax,
>>> HashMap (lines: 919, 963, 1026, 1497, 1569) and
>>> LinkedHashMap (lines: 255, 264, 270, 278)
>>> 
>>> There are a bunch of @Override that are missing making the code harder than 
>>> it should to read.
>>> 
>> Yes, i think this is because it sticks to the 166 style i suspect. Easy to 
>> change.
>> 

Note that j.u. classes are quite inconsistent in this respect to using diamonds 
and @Overrides. My preference is to do a sweeping change to all such code. I 
wonder if an IDE can automate that?

 
>>> In putVal, line 654, the null check (e != null) makes the method hard to 
>>> read,
>>> at least I think a comment saying that it means that an existing node need 
>>> to be altered is needed.
>>> 
>> e.g.:
>> 
>> if (e != null) { // existing mapping for key
> 
> Yes, perfect.
> 
>> 
>> 
>>> And I still don't like the way the method clone() is implemented (the older 
>>> implementation has the same issue),
>>> result should not be initialized to null and the catch clause should thow 
>>> an InternalError or an AssertionError,
>>> 
>> You mean like this:
>> 
>> public Object clone() {
>> HashMap result = null;
>> try {
>> result = (HashMap)super.clone();
>> } catch (CloneNotSupportedException e) {
>> // this shouldn't happen, since we are Cloneable
>> throw new InternalError(e);
>> }
>> result.reinitialize();
>> result.putMapEntries(this, false);
>> return result;
>> }
> 
> Yes, and in that case, you don't need to initialize result to null (the first 
> line), because when result is used
> (result.reinitialize()) result is already initialized by the return value of 
> super.clone().
> 

Thanks.

Here is the latest patch that also includes updates for the above 2 comments 
and Mike's comments:

  
http://cr.openjdk.java.net/~psandoz/tl/JDK-8023463-Linked-HashMap-bin-and-tree/webrev/

Paul.

hg: jdk8/tl/jdk: 2 new changesets

[weijun . wang]

Changeset: ca53110f1c74
Author:weijun
Date:  2013-08-27 17:50 +0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ca53110f1c74

8015669: KerberosPrincipal::equals should ignore name-type
Reviewed-by: mullan

! src/share/classes/javax/security/auth/kerberos/KerberosPrincipal.java
+ test/sun/security/krb5/auto/KPEquals.java

Changeset: 4bddc344848e
Author:weijun
Date:  2013-08-27 17:50 +0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4bddc344848e

8022761: regression: SecurityException is NOT thrown while trying to pack a 
wrongly signed Indexed Jar file
Reviewed-by: sherman

! src/share/classes/java/util/jar/JarVerifier.java
+ test/sun/security/tools/jarsigner/jvindex.sh

Re: 8020292: j.u.SplittableRandom

[Paul Sandoz]

On Aug 27, 2013, at 8:54 AM, Peter Levart  wrote:

> Hi,
> 
> The system property name "java.util.secureRandomSeed" suggests that it is not 
> for SplittableRandom only. Now that ThreadLocalRandom is algorithmically 
> aligned with SplittableRandom, should initial seed for TLR instances also use 
> the same algorithm?
> 

Well spotted, yes that was the intention. 

We did sync this with TLR but ran into an issue due to it's current use in 
WeakHashMap and Hashtable e.g. in WeakHashMap:

final void initHashSeed() {
if (sun.misc.VM.isBooted() && Holder.USE_HASHSEED) {
// Do not set hashSeed more than once!
// assert hashSeed == 0;
int seed = ThreadLocalRandom.current().nextInt();
hashSeed = (seed != 0) ? seed : 1;
}
}

The first call to TLR.current() will initialize the TLR class and uses 
InetAddress or SecureRandom, which in turn results in use of WeakHashMap and 
the second call to TLR.current() that throws a NPE that causes an 
java.lang.ExceptionInInitializerError.

My understanding is the hash seed functionality will be removed and when done 
we can add such functionality back to TLR.

It would be good to remove this functionality soonish so we can get this 
feature in.

Paul.

Re: RFR: JDK-8023765 -- Improve MaxPathLength.java testcase and reduce its test load

[Alan Bateman]

On 27/08/2013 01:18, Dan Xu wrote:

Hi All,

MaxPathLength.javais a troublesome testcase, and fails intermittently 
in the nightly test. And it also runs for a long time, especially on 
Windows platforms. Inorder to improve the test stability, I remove its 
unnecessary test iterations, and use NIOdelete method todo the 
clean-up to make the potential failureseasier for diagnosis. Please 
review thechanges. Thanks!


bug: https://bugs.openjdk.java.net/browse/JDK-8023765
webrev: http://cr.openjdk.java.net/~dxu/8023765/webrev/

-Dan

The double to quickly skip over the names to MAX_LENGTH/2 looks reasonable.

I guess fileCreated should really be fileExists as it may be deleted and 
then deleted. An alternative here would be to use Files.deleteIfExists 
as that would avoid the need to introduce flags to track whether the 
directory and exist exists.


-Alan

Re: RFR: JDK-8023647, , " abc1c " .matches( " (\\w)+1\\1 " )) returns false

[Alan Bateman]

On 26/08/2013 21:54, Xueming Shen wrote:

Hi,

Please help review the proposed change for 8023647:

http://cr.openjdk.java.net/~sherman/8023647/webrev

It appears group index updating in situation of GroupCurly backing off
is incorrect from day one (the bug is reproducible back to jdk5). The
current implementation does not back off the index to a previous match,
but the one is being back off.

The ln#4459 is updating a local variable before return, which is not
necessary, I removed it with this change.
This looks okay to me, I just wonder if there we need other tests here 
to be confident that there aren't other issues.


-Alan

Re: Additional source to guess the local timezone ID on linux

[Masayoshi Okutsu]

Hi Omair,

/etc/sysconfig/clock used to be supported, but it was removed in JDK 7. 
The problem is discussed in bug #6456628.


http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6456628

Have you tested your fix on all Red Hat-like distros, including some 
older releases, with all the time zone IDs?


Thanks,
Masayoshi

On 8/26/2013 5:40 PM, Alan Bateman wrote:


Including i18n-dev on this discussion because that is where this code 
is maintained.



On 23/08/2013 22:09, Omair Majid wrote:

Hi,

The algorithm that OpenJDK uses to guess the local timezone ID on Linux
goes like this:

1. If TZ environment variable is set, use that
2. If /etc/timezone is readable, read the time zone from there
3. If /etc/localtime is a symlink, resolve that, and use the name to
guess the time zone.
4. Scan /usr/share/zoneinfo for a file whose contents match the contents
of /etc/localtime.

Step 4 (if it is ever reached) is probably going to lead to incorrect
results since there are a number of timezones that have the same
zoneinfo data (such as Europe/London and Europe/Belfast). So it seems
sensible to me to try and use additional sources to guess the timezone
ID before resorting to file content comparisons.

The webrev adds a step between 2 and 3 that reads and parses
/etc/sysconfig/clock to extract the timezone:

http://cr.openjdk.java.net/~omajid/webrevs/timezone-read-sysconfig-clock/00/ 



This file exists on some Red Hat Enterprise Linux (and derivative)
distributions and contains contents that look this:

# The time zone of the system is defined by the contents of 
/etc/localtime.
# This file is only for evaluation by system-config-date, do not 
rely on its

# contents elsewhere.
ZONE="Europe/Zurich"

With this, we should be able to identify the exact timezone ID.

Thanks,
Omair