Re: [10] RFR 8134512 : provide Alpha-Numeric (logical) Comparator
Thank you Jason for valuable input!
On 9/28/17 7:22 AM, Jason Mehrens wrote:
Ivan,
Am I correct that subtraction of code points is safe from underflow due to the
range of Character.MIN_CODE_POINT and Character.MAX_CODE_POINT? That would
explain why you are not using Integer.compare.
Right. Since codepoints are all positive, it should be safe to subtract
them.
One alternative to calling CharSequence.subSequence is to use
CharBuffer.wrap(CharSequence csq, int start, int end). The sub-sequences are
short lived so that may be faster in the String case.
Yes, it's a good point, given how expensive String.subSequence() is.
The disadvantage of using CharBuffer.wrap() is that it wouldn't be
possible to use the standard String comparators as the custom
alphaComparator.
With the current implementation, it is possible (with additional casts,
of course) because we know that String.subSequence returns a String object.
To be honest, I see no easy way to optimize the String case with the use
of a custom comparator because we need to pass to it portions of the
input as String objects.
One possibility might be to recognize common cases, like using the
standard comparators (for example, String.CASE_INSENSITIVE_ORDER) in
place of the custom alphaComparator, and then execute special variant of
the routine, which doesn't create explicit subsequences, but works on
the entire sequence instead.
At this point, however, it's probably too early to do such optimizations.
Admittedly this is more of a Claes Redestad faster boot up type change but, the
use of the AlphaDecimalComparator constructor in Comparator might force the
bytecode verifier to force load AlphaDecimalComparator just to check the return
type.
Hm, interesting.
We can introduce another factory method in the Comparators class, so
that AlphaDecimalComparator won't be mentioned in the Comparator at all.
Please see the updated webrev:
http://cr.openjdk.java.net/~igerasim/8134512/05/webrev/
With kind regards,
Ivan
If you make factory methods in AlphaDecimalComparator you can usually craft the
return type to match and the verify of Comparator will not force load the
AlphaDecimalComparator.
Otherwise, the more ugly solution is to wrap 'new' with Comparator.class.cast.
Jason
From: core-libs-dev on behalf of Ivan
Gerasimov
Sent: Monday, September 25, 2017 12:49 PM
To: core-libs-dev
Subject: Re: [10] RFR 8134512 : provide Alpha-Numeric (logical) Comparator
Hello!
Could you please review at your convenience?
In the latest webrev I took all suggestions into account (unless I
missed something.)
http://cr.openjdk.java.net/~igerasim/8134512/04/webrev/
I think, if the suggested comparator is found useful by the users, then
it may make sense to create the String-oriented variant, which can be
implemented through the CharSequence-oriented one as:
class String {
...
@SuppressWarnings("unchecked")
public static Comparator
comparingAlphaDecimal(Comparator alphaComparator) {
return (Comparator) (Comparator)
new
Comparators.AlphaDecimalComparator<>(Objects.requireNonNull(
(Comparator) alphaComparator),
false);
}
}
This will be safe, since the specification guarantees that
String.subSequence() returns a String.
Then in the application code it would be possible to instantiate the
comparators as
String.comparingAlphaDecimal(String::compareTo);
String.comparingAlphaDecimal(String::compareToIgnoreCase);
or, alternatively,
String.comparingAlphaDecimal(Comparator.naturalOrder());
String.comparingAlphaDecimal(String.CASE_INSENSITIVE_ORDER);
But this could be deferred for later, of course.
With kind regards,
Ivan
On 8/27/17 1:38 PM, Ivan Gerasimov wrote:
Hello everyone!
Here's another iteration of the comparator with suggested improvements.
Now, there is the only input argument -- the alpha-comparator for
comparing the non-decimal-digit sub-sequences.
For the javadoc I used the text suggested by Peter with some
modifications, additional example and API/implementation notes.
Overall, the javadoc looks heavier than need to me, so I'd love to
hear comments about how to make it shorter and cleaner.
Also, I adopted the name AlphaDecimal, suggested by Peter. This name
is one of popular in the list of variants found in the wild. So, there
are higher chances the users can find the routine by its name.
For testing if a code point is a decimal digit, I used
(Character.getType(cp) == Character.DECIMAL_DIGIT_NUMBER), which seem
to be more appropriate than Character.isDigit(). (The later is true
for things like a digit in a circle, superscript, etc., which do not
seem to be a part of a decimal number composed of several digits.)
The updated webrev:
http://cr.openjdk.java.net/~igerasim/8134512/04/webrev/
Please review at your convenience.
With kind regards,
Ivan
On 8/9/17 4:59 PM, St
Re: [8u-dev][JAXB] Request for review and approval: 8159240: XSOM parser incorrectly processes type names with whitespaces
This looks fine Aleksei. Approved for jdk8u-dev. The JDK-8159240 record is currently closed as a duplicate. I'd suggest you reopen that master record for your 8u-dev port and mark it 9-na. regards, Sean. On 27/09/2017 22:28, Aleks Efimov wrote: Hi, Can I, please, ask for review and approval of JDK-8159240 backport to JDK8? This bug was resolved in JDK9 as part of JAXWS-RI sync changeset (JDK-8164479) therefore separate review for the partially backported changes is needed: http://cr.openjdk.java.net/~aefimov/8159240/8/00/ File changes partially backported to JDK8 from JDK9 sync changeset: http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/26c9b9c51052#l37.1 http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/26c9b9c51052#l38.1 http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/26c9b9c51052#l39.1 http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/26c9b9c51052#l40.1 http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/26c9b9c51052#l41.1 http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/26c9b9c51052#l42.1 http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/26c9b9c51052#l43.1 http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/26c9b9c51052#l44.1 http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/26c9b9c51052#l45.1 Testing: New regression test and all jaxb/ws related JCK/JTREG tests shows no failures. JBS bug: https://bugs.openjdk.java.net/browse/JDK-8159240 JAXWS-RI sync bug: https://bugs.openjdk.java.net/browse/JDK-8164479 JDK9 JAXWS-RI sync changeset: http://hg.openjdk.java.net/jdk9/dev/jaxws/rev/26c9b9c51052 JDK9 JAXWS-RI sync review threads: http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-September/043724.html http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-October/043920.html http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-November/044513.html JAXWS-RI changes for XSOM parser: https://github.com/javaee/jaxb-v2/commit/cc9c09a71d739989d1e250d9db8c1e011215abfd#diff-796464168dc0014203b00c9728d41de8 With Best Regards, Aleksei
Re: [10] RFR 8134512 : provide Alpha-Numeric (logical) Comparator
Ivan,
Am I correct that subtraction of code points is safe from underflow due to the
range of Character.MIN_CODE_POINT and Character.MAX_CODE_POINT? That would
explain why you are not using Integer.compare.
One alternative to calling CharSequence.subSequence is to use
CharBuffer.wrap(CharSequence csq, int start, int end). The sub-sequences are
short lived so that may be faster in the String case.
Admittedly this is more of a Claes Redestad faster boot up type change but, the
use of the AlphaDecimalComparator constructor in Comparator might force the
bytecode verifier to force load AlphaDecimalComparator just to check the return
type.
If you make factory methods in AlphaDecimalComparator you can usually craft the
return type to match and the verify of Comparator will not force load the
AlphaDecimalComparator.
Otherwise, the more ugly solution is to wrap 'new' with Comparator.class.cast.
Jason
From: core-libs-dev on behalf of Ivan
Gerasimov
Sent: Monday, September 25, 2017 12:49 PM
To: core-libs-dev
Subject: Re: [10] RFR 8134512 : provide Alpha-Numeric (logical) Comparator
Hello!
Could you please review at your convenience?
In the latest webrev I took all suggestions into account (unless I
missed something.)
http://cr.openjdk.java.net/~igerasim/8134512/04/webrev/
I think, if the suggested comparator is found useful by the users, then
it may make sense to create the String-oriented variant, which can be
implemented through the CharSequence-oriented one as:
class String {
...
@SuppressWarnings("unchecked")
public static Comparator
comparingAlphaDecimal(Comparator alphaComparator) {
return (Comparator) (Comparator)
new
Comparators.AlphaDecimalComparator<>(Objects.requireNonNull(
(Comparator) alphaComparator),
false);
}
}
This will be safe, since the specification guarantees that
String.subSequence() returns a String.
Then in the application code it would be possible to instantiate the
comparators as
String.comparingAlphaDecimal(String::compareTo);
String.comparingAlphaDecimal(String::compareToIgnoreCase);
or, alternatively,
String.comparingAlphaDecimal(Comparator.naturalOrder());
String.comparingAlphaDecimal(String.CASE_INSENSITIVE_ORDER);
But this could be deferred for later, of course.
With kind regards,
Ivan
On 8/27/17 1:38 PM, Ivan Gerasimov wrote:
> Hello everyone!
>
> Here's another iteration of the comparator with suggested improvements.
>
> Now, there is the only input argument -- the alpha-comparator for
> comparing the non-decimal-digit sub-sequences.
>
> For the javadoc I used the text suggested by Peter with some
> modifications, additional example and API/implementation notes.
> Overall, the javadoc looks heavier than need to me, so I'd love to
> hear comments about how to make it shorter and cleaner.
>
> Also, I adopted the name AlphaDecimal, suggested by Peter. This name
> is one of popular in the list of variants found in the wild. So, there
> are higher chances the users can find the routine by its name.
>
> For testing if a code point is a decimal digit, I used
> (Character.getType(cp) == Character.DECIMAL_DIGIT_NUMBER), which seem
> to be more appropriate than Character.isDigit(). (The later is true
> for things like a digit in a circle, superscript, etc., which do not
> seem to be a part of a decimal number composed of several digits.)
>
> The updated webrev:
> http://cr.openjdk.java.net/~igerasim/8134512/04/webrev/
>
> Please review at your convenience.
>
> With kind regards,
> Ivan
>
> On 8/9/17 4:59 PM, Stuart Marks wrote:
>> On 8/1/17 11:56 PM, Ivan Gerasimov wrote:
>>> I've tried to go one step further and created even more abstract
>>> comparator: It
>>> uses a supplied predicate to decompose the input sequences into
>>> odd/even
>>> subsequences (e.g. alpha/numeric) and then uses two separate
>>> comparator to
>>> compare them. Additionally, a comparator for comparing sequences,
>>> consisting
>>> only of digits is provided. For example, to build a case-insensitive
>>> AlphaDecimal comparator one could use: 1) Character::isDigit -- as
>>> the predicate
>>> for decomposing, 2) String::compareToIgnoreCase -- to compare alpha
>>> (i.e. odd
>>> parts); to work with CharSequences one would need to make it
>>> Comparator.comparing(CharSequence::toString,
>>> String::compareToIgnoreCase), 3)
>>> The special decimal-only comparator, which compares the decimal
>>> representation
>>> of the sequences. Here's the file with all the comparators and a
>>> simple test:
>>> http://cr.openjdk.java.net/~igerasim/8134512/test/Test.java
>>
>> Hi, a couple follow-up thoughts on this.
>>
>> 1) Supplementary characters
>>
>> The current code uses Character.isDigit(char), which works only for
>> char values in the BMP (basic multilingual plane, values <= U+).
>> It won't work for supplementary characters. There are sever
[10] [testbug] RFR of JDK-8187700 SetAuthenticator tests should handle the proxy port
Hi Daniel and all Would you like to review http://cr.openjdk.java.net/~fyuan/8187700/webrev.00/? Bug: https://bugs.openjdk.java.net/browse/JDK-8187700 I applied the fix for proxy port reusing issue, which was reviewed in bug JDK-8187507. And made a few code cleaning in HTTPSetAuthenticatorTest.java Thanks Frank
