Re: RFR: 8046443 : A few typos in JAXP JavaDoc

[huizhe wang]

On 6/16/2014 5:35 PM, Stuart Marks wrote:

This is somewhat moot at this point, but

I'd recommend against using paragraph end tags . Paragraph end 
tags really are optional in HTML. I've heard some advocates of end 
tags, such as commenters on the linked web page, say that end tags are 
somehow "more correct" (wrong) or that end tags should be used 
"because XHTML" (javadoc is HTML4, not XHTML).


It's not xhmtl, but I would think closing tags is a good practice. Being 
explicit is always a good thing to do. It also provides a nice structure 
(NetBeans would auto-close it with an indentation), e.g.:


   this is paragraph 1


although, the JAXP javadoc wasn't written with any good structure.



The problem with using the  end tag is that it's easy for 
additional textual content to slip in afterward. This text would end 
up as part of the parent element. This might result in its being 
styled incorrectly or otherwise treated inconsistently with the rest 
of the text.


That seems to be an argument for a closing tag. When a style is 
specified for , closing tag makes it clear where exactly it would be 
applied -- it's content inside paragraphs, not the whole . If 
there's anything "slipped" outside of the P tags, it would be an error.


For example, I happened to notice the following in one of the files in 
the patch, jaxp/src/javax/xml/namespace/QName.java:


In this example, I think it was intentional by the author to add the 
closing tag to separate the paragraphs, otherwise he would have to add 
another  before .




 * To workaround this issue, serialVersionUID is set with either
 * a default value or a compatibility value.  To use the
 * compatibility value, set the system property:
 *
 * 
com.sun.xml.namespace.QName.useCompatibleSerialVersionUID=1.0

 *
 * This workaround was inspired by classes in the javax.management
 * package, e.g. ObjectName, etc.

Note that the text enclosed in the  element is outside of any 
paragraph. If the style sheet were to have a distinct style for code 
appearing within a paragraph, that style wouldn't apply to the text in 
this example.


with css,  would have its own style.



It turns out that this text is from a private field in the QName class 
(serialVersionUID) so it's usually not rendered anyway, but I'd guess 
that there are other places where text has accidentally ended up 
outside of paragraph elements because a paragraph end tag was used and 
a paragraph start tag (or block start tag) was missing.




s'marks

P.S. Note that the start tag for the  element is optional 
and is implied by context.


haha,  can be put to good use in our writings :-)

-Joe






On 6/11/14 10:49 AM, huizhe wang wrote:

And, here is a good discussion on closing tags:

http://webdesign.about.com/od/htmltags/qt/optional-html-end-tags-when-to-include-them.htm 




-Joe

On 6/11/2014 10:24 AM, Lance Andersen wrote:

Hi Joe,

  is what I am talking about.

On the myriad of clean-ups that were needed to be done in JDBC, 
getting rid of

these type of blah blocks was needed and just use 

Best
Lance
On Jun 11, 2014, at 1:20 PM, huizhe wang mailto:huizhe.w...@oracle.com>> wrote:



On 6/11/2014 9:48 AM, Lance Andersen wrote:

Hi Joe,

please change

dont

to

 don't


Oops, I committed too quickly.  But this is a dev comment, so we 
can fix that

in the next patch.



I would get rid of the 


Guide[1] for JavaDoc Tool suggests using  to separate paragraphs:
If you have more than one paragraph in the doc comment, separate the
paragraphs with a ||paragraph tag, as shown.

[1]
http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html 



-Joe



otherwise it is OK

Best
Lance
On Jun 11, 2014, at 11:54 AM, huizhe wang mailto:huizhe.w...@oracle.com>> wrote:


Fix some typos in the JAXP documentation.  Please review.

http://cr.openjdk.java.net/~joehw/jdk9/8046443/webrev/


Thanks,
Joe








Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 













Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR: 8046443 : A few typos in JAXP JavaDoc

[Stuart Marks]

This is somewhat moot at this point, but

I'd recommend against using paragraph end tags . Paragraph end tags really 
are optional in HTML. I've heard some advocates of end tags, such as commenters 
on the linked web page, say that end tags are somehow "more correct" (wrong) or 
that end tags should be used "because XHTML" (javadoc is HTML4, not XHTML).


The problem with using the  end tag is that it's easy for additional textual 
content to slip in afterward. This text would end up as part of the parent 
element. This might result in its being styled incorrectly or otherwise treated 
inconsistently with the rest of the text.


For example, I happened to notice the following in one of the files in the 
patch, jaxp/src/javax/xml/namespace/QName.java:


 * To workaround this issue, serialVersionUID is set with either
 * a default value or a compatibility value.  To use the
 * compatibility value, set the system property:
 *
 * 
com.sun.xml.namespace.QName.useCompatibleSerialVersionUID=1.0
 *
 * This workaround was inspired by classes in the javax.management
 * package, e.g. ObjectName, etc.

Note that the text enclosed in the  element is outside of any paragraph. 
If the style sheet were to have a distinct style for code appearing within a 
paragraph, that style wouldn't apply to the text in this example.


It turns out that this text is from a private field in the QName class 
(serialVersionUID) so it's usually not rendered anyway, but I'd guess that there 
are other places where text has accidentally ended up outside of paragraph 
elements because a paragraph end tag was used and a paragraph start tag (or 
block start tag) was missing.




s'marks

P.S. Note that the start tag for the  element is optional and is 
implied by context.





On 6/11/14 10:49 AM, huizhe wang wrote:

And, here is a good discussion on closing tags:

http://webdesign.about.com/od/htmltags/qt/optional-html-end-tags-when-to-include-them.htm


-Joe

On 6/11/2014 10:24 AM, Lance Andersen wrote:

Hi Joe,

  is what I am talking about.

On the myriad of clean-ups that were needed to be done in JDBC, getting rid of
these type of blah blocks was needed and just use 

Best
Lance
On Jun 11, 2014, at 1:20 PM, huizhe wang mailto:huizhe.w...@oracle.com>> wrote:



On 6/11/2014 9:48 AM, Lance Andersen wrote:

Hi Joe,

please change

dont

to

 don't


Oops, I committed too quickly.  But this is a dev comment, so we can fix that
in the next patch.



I would get rid of the 


Guide[1] for JavaDoc Tool suggests using  to separate paragraphs:
If you have more than one paragraph in the doc comment, separate the
paragraphs with a ||paragraph tag, as shown.

[1]
http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html

-Joe



otherwise it is OK

Best
Lance
On Jun 11, 2014, at 11:54 AM, huizhe wang mailto:huizhe.w...@oracle.com>> wrote:


Fix some typos in the JAXP documentation.  Please review.

http://cr.openjdk.java.net/~joehw/jdk9/8046443/webrev/


Thanks,
Joe








Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com











Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com

Re: [9] RFR (XS): 8044730: small errors in ConcurrentHashMap and LongAdder docs

[Martin Buchholz]

Looks good!


On Mon, Jun 16, 2014 at 4:52 PM, Stuart Marks 
wrote:

> Hi Martin,
>
> Thanks for augmenting my patch and pushing it into the JSR166 repo. Here's
> the patch rebased against jdk9-dev. Please review.
>
> s'marks
>
>
>
> # HG changeset patch
> # User smarks
> # Date 1402960663 25200
> #  Mon Jun 16 16:17:43 2014 -0700
> # Node ID c8c2d902f3f1338b8074607367e7af8e294ae1c6
> # Parent  ade4491b571e65f28a6295b8be9679830a5e9a85
> 8044730: small errors in ConcurrentHashMap and LongAdder docs
> Reviewed-by: XXX
>
> diff -r ade4491b571e -r c8c2d902f3f1 src/share/classes/java/util/
> concurrent/ConcurrentHashMap.java
> --- a/src/share/classes/java/util/concurrent/ConcurrentHashMap.java
> Mon Jun 16 13:48:58 2014 -0400
> +++ b/src/share/classes/java/util/concurrent/ConcurrentHashMap.java
> Mon Jun 16 16:17:43 2014 -0700
> @@ -133,12 +133,12 @@
>   * mapped values are (perhaps transiently) not used or all take the
>   * same mapping value.
>   *
> - * A ConcurrentHashMap can be used as scalable frequency map (a
> + * A ConcurrentHashMap can be used as a scalable frequency map (a
>   * form of histogram or multiset) by using {@link
>   * java.util.concurrent.atomic.LongAdder} values and initializing via
>   * {@link #computeIfAbsent computeIfAbsent}. For example, to add a count
>   * to a {@code ConcurrentHashMap freqs}, you can use
> - * {@code freqs.computeIfAbsent(k -> new LongAdder()).increment();}
> + * {@code freqs.computeIfAbsent(key, k -> new LongAdder()).increment();}
>   *
>   * This class and its views and iterators implement all of the
>   * optional methods of the {@link Map} and {@link Iterator}
> diff -r ade4491b571e -r c8c2d902f3f1 src/share/classes/java/util/
> concurrent/atomic/LongAdder.java
> --- a/src/share/classes/java/util/concurrent/atomic/LongAdder.java
>  Mon Jun 16 13:48:58 2014 -0400
> +++ b/src/share/classes/java/util/concurrent/atomic/LongAdder.java
>  Mon Jun 16 16:17:43 2014 -0700
> @@ -57,7 +57,7 @@
>   * frequency map (a form of histogram or multiset). For example, to
>   * add a count to a {@code ConcurrentHashMap freqs},
>   * initializing if not already present, you can use {@code
> - * freqs.computeIfAbsent(k -> new LongAdder()).increment();}
> + * freqs.computeIfAbsent(key, k -> new LongAdder()).increment();}
>   *
>   * This class extends {@link Number}, but does not define
>   * methods such as {@code equals}, {@code hashCode} and {@code
>

[9] RFR (XS): 8044730: small errors in ConcurrentHashMap and LongAdder docs

[Stuart Marks]

Hi Martin,

Thanks for augmenting my patch and pushing it into the JSR166 repo. Here's the 
patch rebased against jdk9-dev. Please review.


s'marks



# HG changeset patch
# User smarks
# Date 1402960663 25200
#  Mon Jun 16 16:17:43 2014 -0700
# Node ID c8c2d902f3f1338b8074607367e7af8e294ae1c6
# Parent  ade4491b571e65f28a6295b8be9679830a5e9a85
8044730: small errors in ConcurrentHashMap and LongAdder docs
Reviewed-by: XXX

diff -r ade4491b571e -r c8c2d902f3f1 
src/share/classes/java/util/concurrent/ConcurrentHashMap.java
--- a/src/share/classes/java/util/concurrent/ConcurrentHashMap.java	Mon Jun 16 
13:48:58 2014 -0400
+++ b/src/share/classes/java/util/concurrent/ConcurrentHashMap.java	Mon Jun 16 
16:17:43 2014 -0700

@@ -133,12 +133,12 @@
  * mapped values are (perhaps transiently) not used or all take the
  * same mapping value.
  *
- * A ConcurrentHashMap can be used as scalable frequency map (a
+ * A ConcurrentHashMap can be used as a scalable frequency map (a
  * form of histogram or multiset) by using {@link
  * java.util.concurrent.atomic.LongAdder} values and initializing via
  * {@link #computeIfAbsent computeIfAbsent}. For example, to add a count
  * to a {@code ConcurrentHashMap freqs}, you can use
- * {@code freqs.computeIfAbsent(k -> new LongAdder()).increment();}
+ * {@code freqs.computeIfAbsent(key, k -> new LongAdder()).increment();}
  *
  * This class and its views and iterators implement all of the
  * optional methods of the {@link Map} and {@link Iterator}
diff -r ade4491b571e -r c8c2d902f3f1 
src/share/classes/java/util/concurrent/atomic/LongAdder.java
--- a/src/share/classes/java/util/concurrent/atomic/LongAdder.java	Mon Jun 16 
13:48:58 2014 -0400
+++ b/src/share/classes/java/util/concurrent/atomic/LongAdder.java	Mon Jun 16 
16:17:43 2014 -0700

@@ -57,7 +57,7 @@
  * frequency map (a form of histogram or multiset). For example, to
  * add a count to a {@code ConcurrentHashMap freqs},
  * initializing if not already present, you can use {@code
- * freqs.computeIfAbsent(k -> new LongAdder()).increment();}
+ * freqs.computeIfAbsent(key, k -> new LongAdder()).increment();}
  *
  * This class extends {@link Number}, but does not define
  * methods such as {@code equals}, {@code hashCode} and {@code

Re: RFR [9] 8041972: Add improved parse/format methods for Long/Integer

[Joe Darcy]

Hello Claes,

Instead of saying

* Extend upon parseFoo(String, int)

in the javadoc of the new methods paired with an

* @see parseFoo(String, int)

please use an {@link parseFoo} instead.

Thanks,

-Joe

On 06/15/2014 03:22 PM, Claes Redestad wrote:
I've updated the patch to use CharSequence in favor of String for all 
new methods, as well as ensuring all new methods are package private 
(for now):


http://cr.openjdk.java.net/~redestad/8041972/webrev.2/

Reviews appreciated!

/Claes

On 2014-06-15 14:29, Claes Redestad wrote:


On 2014-06-15 13:48, Remi Forax wrote:


On 06/15/2014 12:36 AM, Claes Redestad wrote:

Hi,

please review this patch to add offset based variants of 
Integer.parseInt/parseUnsignedInt, Long.parseLong/parseUnsignedLong 
and expose them through JavaLangAccess along with 
formatUnsignedInt/-Long. This is proposed to enable a number of 
minor optimizations, starting with 
https://bugs.openjdk.java.net/browse/JDK-8006627


Webrev: http://cr.openjdk.java.net/~redestad/8041972/webrev.0/

Thanks!

/Claes


In my opinion, and as suggested in the description of 8041972, these 
methods should be public.
Ultimately, yes. The ulterior motive here is to be able to backport 
these to 8u40 (maybe even 7) for internal JDK use, while updating to 
public (and dropping the SharedSecrets) should be done in a later, 
9-only update. Adding public methods would require CCC approval, more 
detailed javadocs and possibly be part of a JEP, so pushing a 
back-portable internal patch as a first step seems reasonable.
Having written my share of scanners/parsers in Java, these methods 
are really helpful but
given that they only rely on charAt/length of String, I think they 
should take a CharSequence and not a String as first parameter,
in order to support other storages of characters like by example a 
java.nio.CharBuffer.


So for me, instead of adding a bunch of method in shared secrets, 
those method should be public and take a CharSequence.
I wouldn't mind switching over to CharSequence for the newly added 
methods. I wasn't around last time[1] this was proposed for the 
existing methods, but I know there were arguments that changing to a 
(possibly) mutable input type might have problematic side effects 
that would form a valid argument against this switch, while I 
personally share the opinion that it's up to the caller to enforce 
immutability when necessary and that text parsing methods should all 
use CharSequence when possible.


Miinor nits, Integer.parseInt(String,int,int) don't need to test if 
s is null given it delegated to parseInt(String, int, int, int),
and Integer.parseInt(String) should directly call parseInt(s, 10, 0, 
s.length()) instead of calling parseInt(s, 10) that calls

parseInt(s, 10, 0) that calls parseInt(s, 10, 0, s.length()).

cheers,
Rémi
Not checking for null in parseInt(String, int, int) would mean we 
would get a NPE calling s.length() in the call to parseInt(String, 
int, int, int), so this is needed for compatibility. Microbenchmarks 
suggests the extra check does not have any performance impact since 
the JIT can easily prove that the inner null checks can be removed 
when an outer method.


Calling parseInt(s, 10, 0, s.length()) directly in place of 
parseInt(s, 10, 0) would likewise require an earlier null check 
(slightly more code duplication) while being performance neutral 
either way.


/Claes

[1] 
https://www.mail-archive.com/core-libs-dev@openjdk.java.net/msg09694.html

Re: RFR [9] 8041972: Add improved parse/format methods for Long/Integer

[Claes Redestad]

On 2014-06-16 21:50, roger riggs wrote:

Hi Claes,

The descriptions of the new methods should take the same form as
the coresponding existing methods.  The rationalization about 
intermediary
objects is not useful in describing the behavior of the method and 
should be omitted.

Point taken!


/**
 * Parses the string argument starting at fromIndex as a signed 
integer in the radix

 * specified by the second argument.
...

static int parseInt(CharSequence s, int radix, int fromIndex)

The terminology used in java.lang.String for offsets and indexes into 
strings would

be provide a consistent base for talking about substrings.
If we're taking cues from String.substring, I guess int beginIndex[, int 
fromIndex] would be more appropriate. How about:


/**
 * Parses the character sequence argument in the specified {@code 
radix},

 * beginning at the specified {@code beginIndex} and extending to the
 * character at index {@code endIndex - 1}.
 *
 * @see java.lang.Integer#parseInt(String, int)
 * @param  s   the {@code CharSequence} containing the integer
 *  representation to be parsed
 * @param  radix   the radix to be used while parsing {@code s}.
 * @param  beginIndex   the beginning index, inclusive.
 * @param  endIndex the ending index, exclusive.
 * @return the integer represented by the subsequence in the
 * specified radix.
 */
static int parseInt(CharSequence s, int radix, int beginIndex, int 
endIndex)


?

Thanks!

/Claes


Thanks, Roger


For example,


On 6/15/2014 6:22 PM, Claes Redestad wrote:
I've updated the patch to use CharSequence in favor of String for all 
new methods, as well as ensuring all new methods are package private 
(for now):


http://cr.openjdk.java.net/~redestad/8041972/webrev.2/

Reviews appreciated!

/Claes

On 2014-06-15 14:29, Claes Redestad wrote:


On 2014-06-15 13:48, Remi Forax wrote:


On 06/15/2014 12:36 AM, Claes Redestad wrote:

Hi,

please review this patch to add offset based variants of 
Integer.parseInt/parseUnsignedInt, 
Long.parseLong/parseUnsignedLong and expose them through 
JavaLangAccess along with formatUnsignedInt/-Long. This is 
proposed to enable a number of minor optimizations, starting with 
https://bugs.openjdk.java.net/browse/JDK-8006627


Webrev: http://cr.openjdk.java.net/~redestad/8041972/webrev.0/

Thanks!

/Claes


In my opinion, and as suggested in the description of 8041972, 
these methods should be public.
Ultimately, yes. The ulterior motive here is to be able to backport 
these to 8u40 (maybe even 7) for internal JDK use, while updating to 
public (and dropping the SharedSecrets) should be done in a later, 
9-only update. Adding public methods would require CCC approval, 
more detailed javadocs and possibly be part of a JEP, so pushing a 
back-portable internal patch as a first step seems reasonable.
Having written my share of scanners/parsers in Java, these methods 
are really helpful but
given that they only rely on charAt/length of String, I think they 
should take a CharSequence and not a String as first parameter,
in order to support other storages of characters like by example a 
java.nio.CharBuffer.


So for me, instead of adding a bunch of method in shared secrets, 
those method should be public and take a CharSequence.
I wouldn't mind switching over to CharSequence for the newly added 
methods. I wasn't around last time[1] this was proposed for the 
existing methods, but I know there were arguments that changing to a 
(possibly) mutable input type might have problematic side effects 
that would form a valid argument against this switch, while I 
personally share the opinion that it's up to the caller to enforce 
immutability when necessary and that text parsing methods should all 
use CharSequence when possible.


Miinor nits, Integer.parseInt(String,int,int) don't need to test if 
s is null given it delegated to parseInt(String, int, int, int),
and Integer.parseInt(String) should directly call parseInt(s, 10, 
0, s.length()) instead of calling parseInt(s, 10) that calls

parseInt(s, 10, 0) that calls parseInt(s, 10, 0, s.length()).

cheers,
Rémi
Not checking for null in parseInt(String, int, int) would mean we 
would get a NPE calling s.length() in the call to parseInt(String, 
int, int, int), so this is needed for compatibility. Microbenchmarks 
suggests the extra check does not have any performance impact since 
the JIT can easily prove that the inner null checks can be removed 
when an outer method.


Calling parseInt(s, 10, 0, s.length()) directly in place of 
parseInt(s, 10, 0) would likewise require an earlier null check 
(slightly more code duplication) while being performance neutral 
either way.


/Claes

[1] 
https://www.mail-archive.com/core-libs-dev@openjdk.java.net/msg09694.html

Re: RFR [9] 8041972: Add improved parse/format methods for Long/Integer

[roger riggs]

Hi  Claes,

The descriptions of the new methods should take the same form as
the coresponding existing methods.  The rationalization about intermediary
objects is not useful in describing the behavior of the method and 
should be omitted.


/**
 * Parses the string argument starting at fromIndex as a signed integer 
in the radix

 * specified by the second argument.
...

static int parseInt(CharSequence s, int radix, int fromIndex)

The terminology used in java.lang.String for offsets and indexes into 
strings would

be provide a consistent base for talking about substrings.

Thanks, Roger


For example,


On 6/15/2014 6:22 PM, Claes Redestad wrote:
I've updated the patch to use CharSequence in favor of String for all 
new methods, as well as ensuring all new methods are package private 
(for now):


http://cr.openjdk.java.net/~redestad/8041972/webrev.2/

Reviews appreciated!

/Claes

On 2014-06-15 14:29, Claes Redestad wrote:


On 2014-06-15 13:48, Remi Forax wrote:


On 06/15/2014 12:36 AM, Claes Redestad wrote:

Hi,

please review this patch to add offset based variants of 
Integer.parseInt/parseUnsignedInt, Long.parseLong/parseUnsignedLong 
and expose them through JavaLangAccess along with 
formatUnsignedInt/-Long. This is proposed to enable a number of 
minor optimizations, starting with 
https://bugs.openjdk.java.net/browse/JDK-8006627


Webrev: http://cr.openjdk.java.net/~redestad/8041972/webrev.0/

Thanks!

/Claes


In my opinion, and as suggested in the description of 8041972, these 
methods should be public.
Ultimately, yes. The ulterior motive here is to be able to backport 
these to 8u40 (maybe even 7) for internal JDK use, while updating to 
public (and dropping the SharedSecrets) should be done in a later, 
9-only update. Adding public methods would require CCC approval, more 
detailed javadocs and possibly be part of a JEP, so pushing a 
back-portable internal patch as a first step seems reasonable.
Having written my share of scanners/parsers in Java, these methods 
are really helpful but
given that they only rely on charAt/length of String, I think they 
should take a CharSequence and not a String as first parameter,
in order to support other storages of characters like by example a 
java.nio.CharBuffer.


So for me, instead of adding a bunch of method in shared secrets, 
those method should be public and take a CharSequence.
I wouldn't mind switching over to CharSequence for the newly added 
methods. I wasn't around last time[1] this was proposed for the 
existing methods, but I know there were arguments that changing to a 
(possibly) mutable input type might have problematic side effects 
that would form a valid argument against this switch, while I 
personally share the opinion that it's up to the caller to enforce 
immutability when necessary and that text parsing methods should all 
use CharSequence when possible.


Miinor nits, Integer.parseInt(String,int,int) don't need to test if 
s is null given it delegated to parseInt(String, int, int, int),
and Integer.parseInt(String) should directly call parseInt(s, 10, 0, 
s.length()) instead of calling parseInt(s, 10) that calls

parseInt(s, 10, 0) that calls parseInt(s, 10, 0, s.length()).

cheers,
Rémi
Not checking for null in parseInt(String, int, int) would mean we 
would get a NPE calling s.length() in the call to parseInt(String, 
int, int, int), so this is needed for compatibility. Microbenchmarks 
suggests the extra check does not have any performance impact since 
the JIT can easily prove that the inner null checks can be removed 
when an outer method.


Calling parseInt(s, 10, 0, s.length()) directly in place of 
parseInt(s, 10, 0) would likewise require an earlier null check 
(slightly more code duplication) while being performance neutral 
either way.


/Claes

[1] 
https://www.mail-archive.com/core-libs-dev@openjdk.java.net/msg09694.html

Re: [9] RFR (XS): 8046903: VM anonymous class members can't be statically invocable

[John Rose]

Reviewed.  — John

On Jun 16, 2014, at 9:50 AM, Vladimir Ivanov  
wrote:

> http://cr.openjdk.java.net/~vlivanov/8046903/webrev.00/
> https://bugs.openjdk.java.net/browse/JDK-8046903
> 
> j.l.i.InvokerBytecodeGenerator::isStaticallyInvocable doesn't distinguish 
> between VM anonymous classes and ordinary classes. In some very specific 
> circumstances (VM anonymous class declared in one of predefined core 
> packages), it can consider a member of VM anonymous class as statically 
> invocable and symbolically reference it from generated bytecode. It's wrong 
> because such class can't be looked up by name and the attempt to run such 
> code ends up with NoClassDefFoundError.
> 
> The fix is to disable static invocation for members of VM anonymous classes.
> 
> Testing: regression test.
> 
> Thanks!
> 
> Best regards,
> Vladimir Ivanov

Re: RFR 8u20: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class

[Joe Darcy]

Hi Joel,

The 8u version looks fine; thanks,

-Joe

On 06/16/2014 03:57 AM, Joel Borggrén-Franck wrote:

Hi,

Can I get a review for this almost trivial 8u20 back port. The 9 fix reviewed 
here: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-June/027173.html  
applies cleanly except one hunk fails due to a non-backported change updating a 
for to a for each:

$ cat src/share/classes/java/lang/Class.java.rej
--- Class.java
+++ Class.java
@@ -2819,7 +2894,7 @@
  // the end.
  MethodArray inheritedMethods = new MethodArray();
  for (Class i : getInterfaces()) {
-inheritedMethods.addAllNonStatic(i.privateGetPublicMethods());
+inheritedMethods.addInterfaceMethods(i.privateGetPublicMethods());
  }
  if (!isInterface()) {
  Class c = getSuperclass();

In 8u the hunk needs to be:

@@ -2819,9 +2894,8 @@
  // out concrete implementations inherited from superclasses at
  // the end.
  MethodArray inheritedMethods = new MethodArray();
-Class[] interfaces = getInterfaces();
-for (int i = 0; i < interfaces.length; i++) {
-
inheritedMethods.addAllNonStatic(interfaces[i].privateGetPublicMethods());
+for (Class i : getInterfaces()) {
+inheritedMethods.addInterfaceMethods(i.privateGetPublicMethods());
  }
  if (!isInterface()) {
  Class c = getSuperclass();

8u webrev here: http://cr.openjdk.java.net/~jfranck/8029674/jdk8u/webrev.8u.00/
9 Bug here: http://bugs.openjdk.java.net/browse/JDK-8029674

cheers
/Joel

RE: RFR [9] 8046902: Remove sun.misc.Timer

[Iris Clark]

Hi, Chris.

Looks great!  Long overdue since java.util.Timer was added around Spring 1999 
during JDK 1.3 development.

Thanks,
iris

-Original Message-
From: Chris Hegarty 
Sent: Monday, June 16, 2014 8:06 AM
To: Core-Libs-Dev
Subject: RFR [9] 8046902: Remove sun.misc.Timer

The type sun.misc.Timer has long since been replaced by the standard 
java.util.Timer. As an unneeded part of sun.misc, sun.misc.Timer should be 
removed from the platform. This is part of the larger cleanup of legacy unused 
classes from sun.misc, see JDK-6852936 [1].

sun.misc.Timeable will also be removed as part of this issue.

http://cr.openjdk.java.net/~chegar/8046902/webrev.00/webrev/

-Chris.

[0] https://bugs.openjdk.java.net/browse/JDK-8046902
[1] https://bugs.openjdk.java.net/browse/JDK-6852936

Re: RFR: JDK-8042369 Remove duplicated java.time classes in build.tools.tzdb

[Xueming Shen]

OK, let's go the non-controversial approach. The webrev has been updated to 
simply remove
those redundant/duplicated class files from the builder and use the updated 
version of the tzdb
builder.

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

Thanks!
-Sherman

On 5/20/2014 1:52 PM, roger riggs wrote:

Hi Sherman,

Even though it has well defined content, checking in the tar.gz seems not quite 
right.
Can the tests reconstruct the tar file from the contents or directly use
the IANA data on demand from the IANA site?

From a maintenance point of view,  functions added to the JDK should be
well used and maintained and be well supported.

TzdbZoneRulesCompiler.java: +130 The error for srcFile == null still mentions 
"-srcdir".

 :+153; the commented out message should be removed


Roger


On 5/19/2014 2:12 PM, Xueming Shen wrote:

Hi,

I've not got any feedback so far, so I assume it's good:-)

Anyway, I'm going a little further to throw in a TarInputStream so now we just 
build the
tzdb data directly on  the tzdata201xy.tar.gz from iana site. I'm not sure if 
it is OK to
check in the original .tar.gz file into the jdk repository though.

There are also more improvements in this version, it is much faster now. It 
might
not matter as it is only used during build time, though:-)

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

-Sherman

Btw, I'm still trying to sell the idea of checking in a second tzdb provider 
implementation
into the jdk, which directly loads in the tzdb data by using the iana original 
source data,
with the assumption that this might be useful at least in certain circumstance 
(such as
one gov pushes a tz change, and some of your big jdk customers need their apps 
run with
the new data in next 24 hrs...) in the future.

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


On 05/04/2014 11:16 PM, Xueming Shen wrote:

Hi

Please help review the change for #8042369

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

In jdk8 we had to duplicate dozen java.time classes in build.tools to build the 
timezone data
for the new JSR310 timezone data compiler, which uses the new jdk8 java.time 
classes (these
classes don't exit in the < 8 boot jdk). JDK9 has switched to use the jdk8 as 
the boot jdk,  so
most of these duplicated classes are no longer needed, with ZoneRules as the 
only exception.
The ZoneRules is still needed to help the tzdb compiler to output the tzdb data 
in the
serialization forms of those transitions and rules. The proposed change here is 
to remove
those unneeded duplicated classes.

I also took the opportunity to re-organize/re-wrote the "builder" classes to 
have a faster, simpler
and and straightforward implementation, with the goal of migrating it into a 
second default
ZoneRulesProvider later to plug it into jdk, so jdk/jre can uses the tzdb 
source data from the
IANA directly. One of the benefits of such a provider is that the end user may 
just drop the latest
timezone data file into the jdk/jre and go, without waiting for the latest 
timezone binary bits from
Oracle.

Here is the webrev for the idea
http://cr.openjdk.java.net/~sherman/tzdbProvider/webrev/

The only disadvantage appears to be the possible "slowdown" of startup time 
because of
the reading and compiling of the 200k tzdb source data...(we need another new 
"bridge" for
 j.u.TimeZone, if we go with this direction)

Thanks!
-Sherman

Re: [OpenJDK 2D-Dev] JDK-8041679 Replace uses of StringBuffer with StringBuilder within the JDK

[Paul Sandoz]

After some further reflection i have decided to not separate this out and some 
bits going to client and some bits going to dev, and have pushed all changes to 
dev. I realise this might cause some friction but believe it the right thing to 
do.

I remain unconvinced that the separate process for patches such as this scales 
and is a valuable use of our development time and resources. A merging process 
is surely fundamentally more efficient (and anyway has to be performed) over N 
developers having to use a slightly different process [*].

Call me human and flawed in my management of time :-( but higher priority 
things keep getting pushed to the top of my stack before getting around to 
following this separate process and i really don't want the code to bit rot. I 
suspect i am not unique in this regard.

Paul.

[*] Is it documented? I have never managed to obtain a definitive answer, nor 
an explicit white-list of Java package names (external yes, internal no).

On May 20, 2014, at 7:45 AM, Paul Sandoz  wrote:

> 
> On May 19, 2014, at 6:18 PM, Phil Race  wrote:
> 
>> On 5/19/2014 12:50 AM, Alan Bateman wrote:
>>> On 19/05/2014 07:53, Paul Sandoz wrote:
 If i don't have permission to push to the client repo (which might be 
 likely) i will need to hand over the patch to yourself or Sergey to 
 commit. And i presume this will have to be a separate issue.
>>> If you do decide to split this then it will require creating a second issue 
>>> JIRA to avoid running foul of jcheck when jdk/client eventually pushes to 
>>> jdk/dev. I don't know how often jdk9/client integrates into jdk9/dev but it 
>>> doesn't seem to be very frequent (hg incoming suggests there is currently 
>>> about a month of changes backed up in jdk9/client but there may be issues 
>>> to explain this).
>>> 
>>> For this specific case then it doesn't seem worth it, it would be much less 
>>> effort to just push the lot to jdk9/dev. Clearly if there were substantive 
>>> changes then it would be important to push the changes to the forest where 
>>> they are most likely to get tested but it hardly seems worth it here. From 
>>> what I can tell then Phil or others sync up jdk9/client regularly and that 
>>> might be the most efficient way to get these changes into jdk9/client.
>>> 
>> The changes should go through client for the reasons I already gave.
> 
> IIUC these were the reasons you gave in a previous email on this thread:
> 
> "I would not push hotspot changes to client either. Also lots of files
> are being updated in client and doing it this way will minimise merges ..."
> 
> I don't find either very convincing.
> 
> 
>> No new permissions are needed but it will need a unique bug id.
>> 
> 
> Ok.
> 
> 
>> FWIW integrations are intended to be bi-weekly but holidays interfered this 
>> time.
>> 
> 
> Why does it take so long?
> 
> Paul.

[9] RFR (XS): 8046903: VM anonymous class members can't be statically invocable

[Vladimir Ivanov]

http://cr.openjdk.java.net/~vlivanov/8046903/webrev.00/
https://bugs.openjdk.java.net/browse/JDK-8046903

j.l.i.InvokerBytecodeGenerator::isStaticallyInvocable doesn't 
distinguish between VM anonymous classes and ordinary classes. In some 
very specific circumstances (VM anonymous class declared in one of 
predefined core packages), it can consider a member of VM anonymous 
class as statically invocable and symbolically reference it from 
generated bytecode. It's wrong because such class can't be looked up by 
name and the attempt to run such code ends up with NoClassDefFoundError.


The fix is to disable static invocation for members of VM anonymous classes.

Testing: regression test.

Thanks!

Best regards,
Vladimir Ivanov

Re: RFR [9] 8046902: Remove sun.misc.Timer

[Mike Duigou]

Remove it before someone starts to use it!

Mike

On Jun 16 2014, at 08:05 , Chris Hegarty  wrote:

> The type sun.misc.Timer has long since been replaced by the standard 
> java.util.Timer. As an unneeded part of sun.misc, sun.misc.Timer should be 
> removed from the platform. This is part of the larger cleanup of legacy 
> unused classes from sun.misc, see JDK-6852936 [1].
> 
> sun.misc.Timeable will also be removed as part of this issue.
> 
> http://cr.openjdk.java.net/~chegar/8046902/webrev.00/webrev/
> 
> -Chris.
> 
> [0] https://bugs.openjdk.java.net/browse/JDK-8046902
> [1] https://bugs.openjdk.java.net/browse/JDK-6852936

Re: RFR [9] 8046902: Remove sun.misc.Timer

[roger riggs]

Hi Chris,

Looks good,  especially since there are no current uses.

Roger

On 6/16/2014 11:05 AM, Chris Hegarty wrote:
The type sun.misc.Timer has long since been replaced by the standard 
java.util.Timer. As an unneeded part of sun.misc, sun.misc.Timer 
should be removed from the platform. This is part of the larger 
cleanup of legacy unused classes from sun.misc, see JDK-6852936 [1].


sun.misc.Timeable will also be removed as part of this issue.

http://cr.openjdk.java.net/~chegar/8046902/webrev.00/webrev/

-Chris.

[0] https://bugs.openjdk.java.net/browse/JDK-8046902
[1] https://bugs.openjdk.java.net/browse/JDK-6852936

Re: RFR: 8046389: Add missing @since tag under javax.sql.**

[Henry Jen]

Thanks for the review, Lance.

Cheers,
Henry

On 06/16/2014 04:33 AM, Lance @ Oracle wrote:

This is fine Henry

Best
Lance


Lance Andersen|
Principal Member of Technical Staff | +1.781.442.2037 
Oracle Java Engineering
1 Network Drive 
Burlington, MA 01803 
lance.ander...@oracle.com 
Sent from my iPad

On Jun 9, 2014, at 5:39 PM, Henry Jen mailto:henry@oracle.com>> wrote:


Hi,

Please review a trivial webrev that provides missing @since tag for
elements under javax.sql,

javax.sql.CommonDataSource was extracted out since 1.6, methods have
@since 1.4 tag because those methods existed in origins since then.

http://cr.openjdk.java.net/~henryjen/jdk9/8046389/0/webrev/

Cheers,
Henry

RFR [9] 8046902: Remove sun.misc.Timer

[Chris Hegarty]

The type sun.misc.Timer has long since been replaced by the standard 
java.util.Timer. As an unneeded part of sun.misc, sun.misc.Timer should 
be removed from the platform. This is part of the larger cleanup of 
legacy unused classes from sun.misc, see JDK-6852936 [1].


sun.misc.Timeable will also be removed as part of this issue.

http://cr.openjdk.java.net/~chegar/8046902/webrev.00/webrev/

-Chris.

[0] https://bugs.openjdk.java.net/browse/JDK-8046902
[1] https://bugs.openjdk.java.net/browse/JDK-6852936

Re: RFR: 8046389: Add missing @since tag under javax.sql.**

[Lance @ Oracle]

This is fine Henry 

Best
Lance


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
Sent from my iPad

On Jun 9, 2014, at 5:39 PM, Henry Jen  wrote:

> Hi,
> 
> Please review a trivial webrev that provides missing @since tag for elements 
> under javax.sql,
> 
> javax.sql.CommonDataSource was extracted out since 1.6, methods have @since 
> 1.4 tag because those methods existed in origins since then.
> 
> http://cr.openjdk.java.net/~henryjen/jdk9/8046389/0/webrev/
> 
> Cheers,
> Henry

RFR 8u20: JDK-8029674: (reflect) getMethods returns default methods that are not members of the class

[Joel Borggrén-Franck]

Hi,

Can I get a review for this almost trivial 8u20 back port. The 9 fix reviewed 
here: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-June/027173.html  
applies cleanly except one hunk fails due to a non-backported change updating a 
for to a for each:

$ cat src/share/classes/java/lang/Class.java.rej
--- Class.java
+++ Class.java
@@ -2819,7 +2894,7 @@
 // the end.
 MethodArray inheritedMethods = new MethodArray();
 for (Class i : getInterfaces()) {
-inheritedMethods.addAllNonStatic(i.privateGetPublicMethods());
+inheritedMethods.addInterfaceMethods(i.privateGetPublicMethods());
 }
 if (!isInterface()) {
 Class c = getSuperclass();

In 8u the hunk needs to be:

@@ -2819,9 +2894,8 @@
 // out concrete implementations inherited from superclasses at
 // the end.
 MethodArray inheritedMethods = new MethodArray();
-Class[] interfaces = getInterfaces();
-for (int i = 0; i < interfaces.length; i++) {
-
inheritedMethods.addAllNonStatic(interfaces[i].privateGetPublicMethods());
+for (Class i : getInterfaces()) {
+inheritedMethods.addInterfaceMethods(i.privateGetPublicMethods());
 }
 if (!isInterface()) {
 Class c = getSuperclass();

8u webrev here: http://cr.openjdk.java.net/~jfranck/8029674/jdk8u/webrev.8u.00/
9 Bug here: http://bugs.openjdk.java.net/browse/JDK-8029674

cheers
/Joel