Re: Moderator Responsibilities

2018-08-17 Thread Jonathan Bluett-Duncan
Hi Max (I only just realised that you gave your first name in your original
email!),

I'm still unclear on what you meant earlier by "no more kid games", and
also what you mean by "I am not here to adjust maturity in channel.  Agree
to get along here like adults is what". Did any earlier communications you
had on one of the OpenJDK mailing lists leave you disappointed or
something? 樂

Kind regards,
Jonathan

On Fri, 17 Aug 2018, 20:38 mr rupplin,  wrote:

> Great.  I'll check out the wiki.  I am not here to adjust maturity in
> channel.  Agree to get along here like adults is what.
>
> Ok.
>
>
>
> Get Outlook for Android <https://aka.ms/ghei36>
>
> ------
> *From:* Jonathan Bluett-Duncan 
> *Sent:* Friday, August 17, 2018 3:11:22 PM
> *To:* mea...@outlook.com
> *Cc:* core-libs-dev
> *Subject:* Re: Moderator Responsibilities
>
> Hi Mr Rupplin,
>
> I can't speak for anyone else, but I'm personally puzzled by your email.
> Could you clarify what you mean by "There will be no more kid games. If you
> are asked, please answer fully and promptly.  Debugging a long compile
> stack can be complicated."? I ask because it came across as antagonistic to
> me, but I'm sure didn't mean to!
>
> Do not the OpenJDK wiki <https://wiki.openjdk.java.net/> or the Adoption
> group's "New Contributor" page
> <https://wiki.openjdk.java.net/display/Adoption/New+Contributor> give you
> the information you need to work with the OpenJDK sources? If not, is there
> any additional info that you think someone with relevant permissions could
> add to the wiki to make working with the sources easier?
>
> Cheers,
> Jonathan
>
> On Fri, 17 Aug 2018 at 18:41, mr rupplin  wrote:
>
>> Hello.
>>
>>
>> I have been contacted by someone who has stated that there are complaints
>> that we ask the channel about debug questions.  I have 12+ years in
>> Software and a Masters Degree in Computer Science.
>>
>>
>> There will be no more kid games. If you are asked, please answer fully
>> and promptly.  Debugging a long compile stack can be complicated.
>>
>>
>> Also, if recurring questions become irksome make a JDK wiki on github.
>> This is the least we can expect.
>>
>>
>> That is all.
>>
>>
>> Thank you for your understanding.
>>
>>
>> Max R.
>>
>>
>> (Sr. Software Fellow)
>>
>


Re: Moderator Responsibilities

2018-08-17 Thread Jonathan Bluett-Duncan
Hi Mr Rupplin,

I can't speak for anyone else, but I'm personally puzzled by your email.
Could you clarify what you mean by "There will be no more kid games. If you
are asked, please answer fully and promptly.  Debugging a long compile
stack can be complicated."? I ask because it came across as antagonistic to
me, but I'm sure didn't mean to!

Do not the OpenJDK wiki  or the Adoption
group's "New Contributor" page
 give you
the information you need to work with the OpenJDK sources? If not, is there
any additional info that you think someone with relevant permissions could
add to the wiki to make working with the sources easier?

Cheers,
Jonathan

On Fri, 17 Aug 2018 at 18:41, mr rupplin  wrote:

> Hello.
>
>
> I have been contacted by someone who has stated that there are complaints
> that we ask the channel about debug questions.  I have 12+ years in
> Software and a Masters Degree in Computer Science.
>
>
> There will be no more kid games. If you are asked, please answer fully and
> promptly.  Debugging a long compile stack can be complicated.
>
>
> Also, if recurring questions become irksome make a JDK wiki on github.
> This is the least we can expect.
>
>
> That is all.
>
>
> Thank you for your understanding.
>
>
> Max R.
>
>
> (Sr. Software Fellow)
>


Re: RFR: JDK-8200436 - String::isBlank

2018-05-15 Thread Jonathan Bluett-Duncan
How about String.isWhitespaceOrEmpty? I think that accurately describes the
method.

Cheers,
Jonathan

On 15 May 2018 at 19:15, Jeremy Manson  wrote:

> Seems to me that consistency matters here: for Character to call it
> whitespace and for String to call it blank is a little weird.
>
> Is there a well-understood definition of "blank string", or did this just
> seem like a good name choice?
>
> Jeremy
>
> On Mon, May 14, 2018 at 11:14 PM Remi Forax  wrote:
>
> > Hi Louis,
> > I prefer isBlank to isWhitespace, there is a notion of all characters in
> > isBlank that you do not have with isWhitespace.
> >
> > Rémi
> >
> > - Mail original -
> > > De: "Louis Wasserman" 
> > > À: "Xueming Shen" 
> > > Cc: "core-libs-dev" 
> > > Envoyé: Lundi 14 Mai 2018 22:15:50
> > > Objet: Re: RFR: JDK-8200436 - String::isBlank
> >
> > > It's not clear to me that "isBlank" is a good name for this method.
> > > "isWhitespace" might be more appropriate, perhaps.
> > >
> > > On Mon, May 14, 2018 at 12:48 PM Xueming Shen  >
> > > wrote:
> > >
> > >> +1
> > >>
> > >> On 5/14/18, 8:25 AM, Jim Laskey wrote:
> > >> > New string instance method that returns true if the string is empty
> or
> > >> contains only white space, where white space is defined as any
> codepoint
> > >> returns true when passed to Character::isWhitespace.
> > >> >
> > >> > webrev: http://cr.openjdk.java.net/~jlaskey/8200436/webrev/index.
> html
> > >> > jbs: https://bugs.openjdk.java.net/browse/JDK-8200436
> > >> > csr: https://bugs.openjdk.java.net/browse/JDK-8200437
> > >> >
> > >>
> >
>


Re: RFR(JDK11/NIO) 8202285: (fs) Add a method to Files for comparing file contents

2018-04-27 Thread Jonathan Bluett-Duncan
Hi Joe,

I wonder if the method `isSameContent` should be named `haveSameContents`
so as to read more fluently in English.

Cheers,
Jonathan

On 27 April 2018 at 11:58, Daniel Fuchs  wrote:

> Hi Joe,
>
> On the specification side, I think I would reword the API
> documentation to first explain how the method checks the
> content of the two files.
>
> The fact that it doesn't check the actual content if
> the two files are 'the same' is kind of an optimization.
>
> So I would suggest to invert the order of the two paragraph
> in the documentation, and combine them into one - something like:
>
> 1536  * 
>   * This method first calls {@link #isSameFile(java.nio.file.Path,
> java.nio.file.Path) isSameFile(path, path2)} to determine whether the two
> files are the same.
> 1537  * If {@code isSameFile(path, path2)} returns false, this method
> will proceed
> 1538  * to read the files and compare them byte by byte to determine
> if they contain
> 1539  * the same contents.
>   * Otherwise, this method will return true without further
>   * processing.
>
>
> On the implementation side I don't think it's reasonable to call
> readAllBytes() and hold the content of the two files in memory
> for comparing their content, especially if it's to discover that
> the first byte differs.
>
> Some lock-step reading of the two files would seem more appropriate.
>
> best regards,
>
> -- daniel
>
>
>
>
>
> On 27/04/2018 05:51, Joe Wang wrote:
>
>> Hi,
>>
>> Considering extending isSameFile to add isSameContent to Files. Please
>> review.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8202285
>>
>> webrev: http://cr.openjdk.java.net/~joehw/jdk11/8202285/webrev/
>>
>> specdiff: http://cr.openjdk.java.net/~joehw/jdk11/8202285/specdiff/jav
>> a/nio/file/Files.html
>>
>> Thanks,
>> Joe
>>
>>
>


Re: Raw String Literal Library Support

2018-03-13 Thread Jonathan Bluett-Duncan
Sorry, I should really run things in an IDE before posting code examples
and results!

For examples ":".split(":", 0) and "".split(":", 0), they actually produce
[] and [""] respectively (which I still argue is inconsistent and undesired
for the proposed String.splits()).

For examples ":".split(":", -1) and "".split(":", -1), they actually
produce ["", ""] and [""] respectively, which I like better.

Cheers,
Jonathan

On 14 March 2018 at 00:12, Jonathan Bluett-Duncan <jbluettdun...@gmail.com>
wrote:

> Paul,
>
> AFAICT, one sort of behaviour which String.split() allows which
> Pattern.splitAsStream() and the proposed String.splits() don't is allowing
> a negative limit, e.g. String.split(string, -1).
>
> Over at http://errorprone.info/bugpattern/StringSplitter, they argue that
> a limit of -1 has less surprising behaviour than the default of 0, because
> e.g. "".split(":") produces [] (empty array), whereas ":".split(":")
> produces [""] (array with an empty string), which IMO is not consistent.
>
> This compares with ":".split(":", -1) and "".split(":", -1) which produce
> ["", ""] (array with two empty strings, each representing ends of `:`) and
> [] (empty array) respectively - more consistent IMO.
>
> Should String.splits(`\n|\r\n?`) follow the behaviour of String.split(...,
> 0) or String.split(..., -1)?  I'd personally argue for the latter.
>
> Cheers,
> Jonathan
>
> On 13 March 2018 at 23:22, Paul Sandoz <paul.san...@oracle.com> wrote:
>
>>
>>
>> > On Mar 13, 2018, at 3:49 PM, John Rose <john.r.r...@oracle.com> wrote:
>> >
>> > On Mar 13, 2018, at 6:47 AM, Jim Laskey <james.las...@oracle.com>
>> wrote:
>> >>
>> >> …
>> >> A. Line support.
>> >>
>> >> public Stream lines()
>> >>
>> >
>> > Suggest factoring this as:
>> >
>> > public Stream splits(String regex) { }
>>
>> +1
>>
>> This is a natural companion to the existing array returning method (as it
>> was the case on Pattern when we added splitAsStream), where one can use a
>> limit() operation to achieve the same effect as the limit parameter on the
>> array returning method.
>>
>>
>> > public Stream lines() { return splits(`\n|\r\n?`); }
>> >
>>
>> See also Files/BufferedReader.lines. (Without going into details
>> Files.lines has some interesting optimizations.)
>>
>> Paul.
>
>
>


Re: Raw String Literal Library Support

2018-03-13 Thread Jonathan Bluett-Duncan
Paul,

AFAICT, one sort of behaviour which String.split() allows which
Pattern.splitAsStream() and the proposed String.splits() don't is allowing
a negative limit, e.g. String.split(string, -1).

Over at http://errorprone.info/bugpattern/StringSplitter, they argue that a
limit of -1 has less surprising behaviour than the default of 0, because
e.g. "".split(":") produces [] (empty array), whereas ":".split(":")
produces [""] (array with an empty string), which IMO is not consistent.

This compares with ":".split(":", -1) and "".split(":", -1) which produce
["", ""] (array with two empty strings, each representing ends of `:`) and
[] (empty array) respectively - more consistent IMO.

Should String.splits(`\n|\r\n?`) follow the behaviour of String.split(...,
0) or String.split(..., -1)?  I'd personally argue for the latter.

Cheers,
Jonathan

On 13 March 2018 at 23:22, Paul Sandoz  wrote:

>
>
> > On Mar 13, 2018, at 3:49 PM, John Rose  wrote:
> >
> > On Mar 13, 2018, at 6:47 AM, Jim Laskey  wrote:
> >>
> >> …
> >> A. Line support.
> >>
> >> public Stream lines()
> >>
> >
> > Suggest factoring this as:
> >
> > public Stream splits(String regex) { }
>
> +1
>
> This is a natural companion to the existing array returning method (as it
> was the case on Pattern when we added splitAsStream), where one can use a
> limit() operation to achieve the same effect as the limit parameter on the
> array returning method.
>
>
> > public Stream lines() { return splits(`\n|\r\n?`); }
> >
>
> See also Files/BufferedReader.lines. (Without going into details
> Files.lines has some interesting optimizations.)
>
> Paul.


Re: core-libs-dev Digest, Vol 131, Issue 19

2018-03-04 Thread Jonathan Bluett-Duncan
 Okay. Do you have any citations that we can refer to to confirm for
ourselves that this is indeed a recognised need?

Cheers,
Jonathan


On 4 March 2018 at 05:24, A Z  wrote:

> To who it may concern,
>
>
> 'There was a JSR to add a new mode'
>
>
> then I suppose I would be asking for that change on float and double, that
> being the case,
>
>
> as well as an extra mathematics class for BigDecimal.
>
>
> Despite all the time since 1.1, these are necessary changes,
>
>
> since having to use BigDecimal for all accurate arithmetic begins
>
>
> to waste memory, instructions, and the presently unavoidable
>
>
> translation betwee get/set (float or double) and compute (BigDecimal)
>
>
> and convert to (float or double).
>
>
> These are in fact not opinions, but very great needs that the Software
>
>
> community has needed from java for a very long time now,
>
>
> despite statement in the Java Language Specification.
>
> . . .


Re: [11] RFR: 4993841: (str) java.lang.Character should have a toString(int) method

2018-03-02 Thread Jonathan Bluett-Duncan
Hi Rémi,

I'm sure I've misunderstood something, but AFAIK the code example you gave
is already valid code and was valid as far back as Java 8 (I don't have an
IDE at hand to confirm this).

Did you perhaps mean to ask instead that the code snippet below would work
with the resolution of
https://bugs.openjdk.java.net/browse/JDK-4993841 (difference
being that it uses .codePoints() instead of .chars())? Or have I completely
lost the plot?

String s = "hello".codePoints()
.mapToObj(Character::toString)
.collect(Collectors.joining());

Cheers,
Jonathan

On 3 March 2018 at 00:42, Remi Forax  wrote:

> Just to be sure, it now means that a code like this will work
>
>   String s = "hello".chars()
> .mapToObj(Character::toString)
> .collect(Collectors.joining());
>
> Rémi
>
> - Mail original -
> > De: "naoto sato" 
> > À: "Stuart Marks" , "Xueming Shen" <
> xueming.s...@gmail.com>, "core-libs-dev"
> > 
> > Envoyé: Vendredi 2 Mars 2018 03:47:51
> > Objet: [11] RFR: 4993841: (str) java.lang.Character should have a
> toString(int) method
>
> > Hi,
> >
> > Please review the fix to the following issue:
> >
> > https://bugs.openjdk.java.net/browse/JDK-4993841
> >
> > The proposed changeset is located at:
> >
> > http://cr.openjdk.java.net/~naoto/4993841/webrev.03/
> >
> > This stems from the recent discussion regarding String.repeat().[1] The
> > corresponding CSR has already been approved.
> >
> > Naoto
> >
> > --
> > [1]
> > http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-
> February/051568.html
>


Re: [10] RFR 8193856 takeWhile produces incorrect result with elements produced by flatMap

2017-12-20 Thread Jonathan Bluett-Duncan
Hi Paul,

It seems that some clever Googler managed to find a workaround for
aggressive `flatMap` operations in the form of a so-called
`MoreStreams.flatten` operation, implemented in a side project called
google/mug. I'm sharing the javadoc

and
GitHub project homepage  with you and the
rest of the mailing list in the hope that they prove to be useful.

Cheers,
Jonathan

On 20 December 2017 at 21:28, Paul Sandoz  wrote:

> Hi,
>
> Please review this fix for a bug in the stream takeWhile operation:
>
>   http://cr.openjdk.java.net/~psandoz/jdk10/JDK-8193856-
> takeWhile-incorrect-results/webrev/  psandoz/jdk10/JDK-8193856-takeWhile-incorrect-results/webrev/>
>
> The flatMap operation is currently aggressive and does not detect if a
> downstream operation may or has cancelled processing, and will push all of
> it’s elements downstream. Short-circuiting operations should be guarded
> against such behaviour but unfortunately takeWhile was not guarded.
>
> —
>
> Separately i plan to figure out a way to ensure flatMap operations become
> less aggressive if there are short-circuiting downstream operations. This
> may increase efficiency and also allow support for flat map results that
> are infinite.
>
> Paul.
>


Re: RFR 4358774: Add null InputStream and OutputStream

2017-12-08 Thread Jonathan Bluett-Duncan
Like David, I prefer the `null{$ClassName}` alternative to
`null{Source|Sink}`. (I assume from his wording that he prefers
`null{$ClassName}`.)

I prefer `null{$ClassName}` not only because it's less ambiguous, but also
because Guava has existing types `{Byte|Char}Source` and `{Byte|Char}Sink`,
so I'd find it a bit confusing to see `nullSource()` at first (especially
if it were statically-imported) as I would initially expect it to have a
return type of `*Source`.

Cheers,
Jonathan

On 8 December 2017 at 19:03, Brian Burkhalter 
wrote:

> On Dec 8, 2017, at 10:52 AM, David Lloyd  wrote:
>
> > On Fri, Dec 8, 2017 at 12:38 PM, Brian Burkhalter
> >  wrote:
> >> Patrick’s comment made us think again about the naming here as
> “nullStream()” would not fit for eventual equivalent methods on Reader and
> Writer. It might be better to go with something like
> >>
> >> InputStream InputStream.nullSource();
> >> OutputStream.nullSink();
> >>
> >> and later
> >>
> >> Reader.nullSource();
> >> Writer.nullSink();
> >>
> >> Another alternative would be simply to reflect the class names in the
> methods:
> >>
> >> InputStream InputStream.nullInputStream();
> >> OutputStream.nullOutputStream();
> >>
> >> and later
> >>
> >> Reader.nullReader();
> >> Writer.nullWriter();
> >
> > I for one prefer this alternative; it's very clear and unambiguous.
> The second one?
>
> Thanks,
>
> Brian
>
>


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

2017-12-07 Thread Jonathan Bluett-Duncan
Hi Stuart,

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

Cheers,
Jonathan

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

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


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

2017-12-06 Thread Jonathan Bluett-Duncan
Hi Claes,

Looking at
http://cr.openjdk.java.net/~redestad/8193128/open.00/src/java.base/share/classes/java/util/ImmutableCollections.java.cdiff.html,
there are sections labelled --- 646,657  and --- 834,845  where
lines like `Objects.requireNonNull(0 /* zero */);` are written. I believe
that they were supposed to be either removed or made to be written like
`Objects.requireNonNull(o /* lowercase o */)`. Is my belief/understanding
correct?

Cheers,
Jonathan

On 6 December 2017 at 20:21, Claes Redestad 
wrote:

> Hi,
>
> please help review this patch to consolidate the number of implementation
> classes returned by the static collection factories:
>
> http://cr.openjdk.java.net/~redestad/8193128/open.00/
>
> I set out to explore options for addressing small inefficiencies we've
> been running into, the latest after replacing use of @Stable arrays in
> java.lang.invoke with List.of() (JDK-8184777):
>
> - List.indexOf deferred to the iterator in AbstractList, which check for
> concurrent modification
> - Warmup takes a bit longer due to having to warm up four different
> classes and associated methods
> - Slowdowns in certain code paths attributable to certain calls becoming
> megamorphic
>
> Microbenchmark: http://cr.openjdk.java.net/~re
> destad/scratch/less_immutables/ListMorphism.java
> http://cr.openjdk.java.net/~redestad/scratch/less_immutables
> /benchmarks.jar
>
> The benchmark explores how call-sites behave when performing some naive
> operations on a few different Lists.
>
> For every benchmark using List.of() there's a variant using ArrayList for
> comparison:
>
> Baseline:
>
> Benchmark Mode  CntScore Error   Units
> ListMorphism.finalGetFromArrayList   thrpt   25   92.659 ±  3.058 ops/us
> ListMorphism.finalGetFromListthrpt   25  335.245 ±  0.244 ops/us
> # 3.6x
> ListMorphism.finalSumSizesArrayList  thrpt   25  245.020 ±  0.106 ops/us
> ListMorphism.finalSumSizesList   thrpt   25  335.107 ±  0.439 ops/us
> # 1.4x
> ListMorphism.getFromArrayListthrpt   25   70.343 ±  0.972 ops/us
> ListMorphism.getFromList thrpt   25   37.121 ±  0.135 ops/us
> # 0.53x
> ListMorphism.getFromArrayList12  thrpt   25 109.890 ±  0.286  ops/us
> ListMorphism.getFromList12   thrpt   25  109.552 ± 6.972  ops/us
> # 1.0x
> ListMorphism.sumSizesArrayList   thrpt   25  131.467 ±  4.672 ops/us
> ListMorphism.sumSizesListthrpt   25   57.877 ±  0.102 ops/us
> # 0.45x
> ListMorphism.sumSizesArrayList12 thrpt   25 208.652 ± 11.294  ops/us
> ListMorphism.sumSizesList12  thrpt   25  227.269 ± 0.961  ops/us
> # 1.1x
>
> The good: When dealing with List literals (the final* benchmarks),
> List.of() allows really nice speed-ups compared to ArrayList.
>
> The bad: When not used as a literal, List.of() implementations at
> call-sites can cause a substantial slowdown (megamorphic dispatch)
>
> The ugly:
>
> After some explorations[1], I narrowed in on the following experiment:
> http://cr.openjdk.java.net/~redestad/scratch/less_immutables/webrev/
>
> Basically: Merge List1 and List2 into a single class, List12, merge List0
> into ListN (List0 has a singleton instance, so might as well be an instance
> of ListN). Same for Set0,1,2,N. Map0 is merged into MapN.
>
> This strikes a balance between throughput, footprint and slightly better
> startup/warmup behavior.
>
> According to jol estimates[2] the size of List12 is the same as both List1
> and List2 in the current JDK implementation. Set12 is footprint neutral
> compared to Set2 on all platforms but lose a few bytes on 64-bit VMs
> compared to Set1.
>
> Benchmark Mode  CntScore   Error Units
> ListMorphism.finalGetFromArrayList   thrpt   25   93.046 ± 0.569 ops/us
> ListMorphism.finalGetFromListthrpt   25  335.280 ± 0.154 ops/us #
> 3.6x
> ListMorphism.finalSumSizesArrayList  thrpt   25  244.595 ± 1.085 ops/us
> ListMorphism.finalSumSizesList   thrpt   25  303.351 ± 0.182 ops/us #
> 1.2x
> ListMorphism.getFromArrayListthrpt   25   70.631 ± 0.070 ops/us
> ListMorphism.getFromList thrpt   25   73.258 ± 2.955 ops/us #
> 1.04x
> ListMorphism.getFromArrayList12  thrpt   25 109.921 ± 0.096  ops/us
> ListMorphism.getFromList12   thrpt   25  127.392 ± 0.088  ops/us
> # 1.16x
> ListMorphism.sumSizesArrayList   thrpt   25  131.393 ± 4.882 ops/us
> ListMorphism.sumSizesListthrpt   25  107.686 ± 5.286 ops/us #
> 0.82x
> ListMorphism.sumSizesArrayList12 thrpt   25  212.350 ± 0.134  ops/us
> ListMorphism.sumSizesList12  thrpt   25  198.778 ± 0.479  ops/us
> # 0.94x
>
> The experiment has a flag to change number of specialized List/Set/Map
> classes (-Djdk.ImmutableCollections.specializations=0|1|2, default=2).
>
> 1 specialization (List1 + ListN, Set1 + SetN) is more or less the same as
> 2, some ~1-2% improvements, mainly in sumSizes micros.
>
> 0 

Re: RFR: JDK-8133680 add Stream.foldLeft() terminal operation

2017-08-21 Thread Jonathan Bluett-Duncan
> OK, so I think an indexed stream returning pairs of (long,T) is
the answer.

...which I suspect might be worth waiting on Project Valhalla for, if I've
understood you correctly, since an IndexedElement would essentially be a
`long` and object pair, and having the `long` element stored or retrievable
as a `Long` object would be a bit costly compared to having
stored/retrievable as a generic `long` stored on the stack, like what
Project Valhalla is exploring AFAICT.

Alternatively, perhaps having it implement a theoretical interface like
`LongObjectEntry` might be an option, at the expense of cluttering things a
bit?

Cheers,
Jonathan

On 21 August 2017 at 22:00, John Rose  wrote:

> On Aug 21, 2017, at 9:57 AM, Paul Sandoz  wrote:
> >
> > Hi,
> >
> > My preference, naming-wise, is to stick with the current naming scheme
> and refer to such operations as reduceLeft and reduceRight. We have used
> reduce both for the seed accepting, and optional returning methods, and we
> don’t use different names to distinguish those.
>
> +1
>
> > I don’t think we can rely on the sequential() trick, since as Tagir
> points out it is global.
>
> Yes, that's unfortunate for the present purpose.
>
> The closest thing we have at present is forEachOrdered (as you imply).
> The transform Stream.sorted looks like something close, but it can eagerly
> discard the current ordering.  What I think we want, as an input to
> sequential reduce,
> is something that (a) preserves the encounter order and (b) optionally
> transforms
> it in a predictable way (no-op for reduceLeft, reverse for reduceRight).
>
> So maybe there is something akin to Stream.sorted that wants to exist:
> Stream.ordered(p), where p is some token that expresses at least the
> null permutation (keep encounter order, but allow previous parallelism)
> and the reverse permutation.
>
> A complete, somewhat crazy generalization would be something like
> Stream.ordered(LongBinaryOperator comparator), where the comparator
> would decide where to place the stream elements based on an ordinal
> assigned to encounter order.  I don't know if there are middle grounds
> for less powerful distinctions that are useful, and are more powerful
> than just "forward/reverse".
>
> > My inclination would be to focus on reduceLeft, and spin off reduceRight
> into a separate issue and in addition ponder support for a reverse
> operation, since reduceRight can be built upon that.
>
> Thanks.
>
> > It’s possible to develop a reduceRight or reserve operation that does
> something very similar to forEachOrdered when operating in parallel (the
> last FJ task has to complete before given the ok for it’s direct sibling to
> the left to proceed when ready). For good splitters like ArrayList this
> might work well, for bad splitters like Iterator it will be terrible. It
> may be possible in some cases to fuse reverse().toArray().
>
> Yes, I think so.  Maybe the concept of "ordered()" is too weak; the thing
> we are considering is really turning a stream into a data source with
> two characteristics:  (a) it is ordered, and (b) it is random access.
> If you have a random access data sink (toArray) then, if you can
> determine data source indexes, you can then perform the actual
> reordering when you store the result, instead of earlier.
>
> So, here's a stream transform that gives those characteristics:
>
> Stream Stream.indexed().
>
> (Where IndexedElement implements Map.Entry.
> And can be pattern-matched, etc.)
>
> The semantics is that the elements are associated with a compact
> zero-based set of indexes (which may be either easy or hard to
> compute).  The indexes are compatible with forEachOrdered
> (encounter order).  Those indexes can be used to store the
> original stream's elements into any desired random-access pattern.
>
> From there, we can get buffering into an array (or a consumer
> function), which can redirect the values to a reducing collector
> that takes the values in whatever order the user wants.
>
> (The indexed-element guy is also good for numeric array processing.
> It should eventually be a value type.)
>
> OK, so I think an indexed stream returning pairs of (long,T) is
> the answer.
>
> — John


Re: RFR: 8186334: JarFile throws ArrayIndexOutOfBoundsException when the manifest contains certain characters

2017-08-21 Thread Jonathan Bluett-Duncan
Would it not be viable to merge this into Java 9 in the foreseeable future
for one of its inevitable bug fix releases? I admit I'm unfamiliar with the
whole process, so I realise my question may be naive or that the answer is
considered "common knowledge".

Cheers,
Jonathan

On 21 August 2017 at 18:41, Claes Redestad 
wrote:

>
>
> On 2017-08-21 19:15, Paul Sandoz wrote:
>
>> On 21 Aug 2017, at 02:53, Claes Redestad 
>>> wrote:
>>>
>>> Hi,
>>>
>>> this patch addresses an unfortunate regression where backtick characters
>>> in a manifest can cause an AIOOBE.
>>>
>>> Webrev: http://cr.openjdk.java.net/~redestad/8186334/jdk.00/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8186334
>>>
>>> Basically an off-by-one issue during certain steps in the search
>>> algorithm,
>>> meaning it is context dependent whether a backtick will trip on this
>>> issue
>>> or not.
>>>
>>> Ooops, looks good.
>>
>
> Thanks for reviewing!
>
>
>> I see a workaround has been pushed to Jackson, which reduced my urge to
>> suggest a nine respin. But… being slightly paranoid testing JarFile on a
>> local maven central mirror would give us a better sense of the impact.
>>
>
> I think the 9 train has left the station even for issues of this severity.
> And seeing as there are somewhat straightforward workarounds I guess we'll
> have to live with my mistakes. Sorry!
>
> /Claes
>


Re: [10] RFR 8134512 : provide Alpha-Numeric (logical) Comparator

2017-07-28 Thread Jonathan Bluett-Duncan
Hi Ivan,

It looks like the MyComparator code example which you gave in your last
email lost its formatting along the way, so I'm finding it difficult to
read. Would you mind resubmitting it?

Cheers,
Jonathan

On 28 July 2017 at 17:25, Ivan Gerasimov  wrote:

> Hi Peter!
>
> Thank a lot for looking into this!
>
> On 7/28/17 7:32 AM, Peter Levart wrote:
>
>> Hi Ivan,
>>
>> In the light of what Stuart Marks wrote, then what do you think about a
>> parameterized comparator (would be sub-optimal, I know) where one would
>> supply
>> 2 Comparator(s) which would be used to compare Ax and Nx sub-sequences
>> respectively as described below...
>>
>> Yes.  Inspired by what Stuart suggested I made a draft of such a
> comparator (see below).
> It works, but as you've said it's not that efficient (mostly due to
> expensive substrings) and a bit harder to use in a simple case.
> Now I need to think about how to combine two approaches.
>
> For Nx sub-sequences, one would need a comparator comparing the reversed
>> sequence lexicographically,
>>
> I'm not sure I understand why they need to be reversed.
>
>> but for Ax sub-sequences, one could choose from a plethora of
>> case-(in)sensitive comparator(s) and collators already available on the
>> platform.
>>
>> Yes. In the example below I used compareToIgnoreCase to compare alpha
> subsequences.
>
> ---
>
> class MyComparator implements Comparator {Comparator
> alphaCmp;Comparator numCmp;MyComparator(Comparator
> alphaCmp,Comparator numCmp) {this.alphaCmp = alphaCmp;this.numCmp =
> numCmp;}boolean skipLeadingZeroes(String s, int len) {for (int i = 0; i <
> len ; ++i) {if (Character.digit(s.charAt(i), 10) != 0)return false;}return
> true;}@Override public int compare(String o1, String o2)
> {Supplier s1 = new NumberSlicer(o1);Supplier s2 = new
> NumberSlicer(o2);while (true) {// alpha part String ss1 = s1.get();String
> ss2 = s2.get();int cmp = alphaCmp.compare(ss1, ss2);if (cmp != 0) return
> cmp;if (ss1.length() == 0) return 0;// numeric part ss1 = s1.get();ss2 =
> s2.get();int len1 = ss1.length();int len2 = ss2.length();int dlen = len1 -
> len2;if (dlen > 0) {if (!skipLeadingZeroes(ss1, dlen))return 1;ss1 =
> ss1.substring(dlen, len1);} else if (dlen < 0) {if (!skipLeadingZeroes(ss2,
> -dlen))return -1;ss2 = ss2.substring(-dlen, len2);}cmp =
> numCmp.compare(ss1, ss2);if (cmp != 0) return cmp;if (dlen != 0) return
> dlen;}}static class NumberSlicer implements Supplier {private
> String sequence;private boolean expectNumber = false;private int index =
> 0;NumberSlicer(String s) {sequence = s;}@Override public String get()
> {int start = index, end = start, len = sequence.length() - start;for (; len
> > 0; ++end, --len) {if (Character.isDigit(sequence.charAt(end)) ^
> expectNumber)break;}expectNumber = !expectNumber;return
> sequence.substring(start, index = end);}}}Here how it is
> invoked with case-insensitive comparator:Arrays.sort(str,new
> MyComparator(Comparator.comparing(String::toString,String::
> compareToIgnoreCase),Comparator.comparing(String::toString,
> String::compareTo)));
>
> simple test results for case insensitive sorting:java 1java 1 javajava 1
> JAVAJava 2JAVA 5jaVA 6.1java 10java 10 v 13Java 10 v 013Java 10 v
> 13java 10 v 113Java 2017Java 2017Java 20017Java 200017Java 217Java
> 2017Java 2017Java 20017With kind regards, Ivan
>


Re: [10] RFR 8134512 : provide Alpha-Numeric (logical) Comparator

2017-07-24 Thread Jonathan Bluett-Duncan
You're welcome Ivan!

I'm just proofreading the new webrev, and a few more things have occurred
to me:

1. What happens if the comparators are given the elements {"1abc", "2abc",
"10abc"} to compare? Would they sort the elements as {"1abc", "2abc",
"10abc"} or as {"1abc", "10abc", "2abc"}?
What about the array {"x1yz", "x2yz", "x10yz"}?
I wonder if these two cases should be added to the test too and given as
additional examples in the javadocs.

2. The example arrays which you kindly added to the comparators' javadoc
have no whitespace at the start but one space between the last element and
the }, so I think there either should be no space at the end or an extra
space at the start.

3. Very sorry, error on my part: on the javadoc lines which now say "then the
corresponding numerical values are compared; otherwise", I suggested to add
a "then" at the start; re-reading it though, I personally think it reads
better without, but I would understand and not press it further if you
disagree and think that the "then" is a useful addition.

Best regards,
Jonathan


On 24 Jul 2017 06:21, "Ivan Gerasimov" <ivan.gerasi...@oracle.com> wrote:

Thank you Jonathan for looking into that!

I agree with all your suggestions.

Here's the updated webrev with suggested modifications:
WEBREV: http://cr.openjdk.java.net/~igerasim/8134512/02/webrev/

With kind regards,
Ivan


On 7/23/17 3:56 AM, Jonathan Bluett-Duncan wrote:

Meant to reply to core-libs-dev as well; doing so now.

Jonathan

On 23 July 2017 at 11:50, Jonathan Bluett-Duncan <jbluettdun...@gmail.com>
wrote:

> Hi Ivan,
>
> I'm not a reviewer per se, but I thought I'd chime in.
>
> There's a few things with Comparator.java which I think could be improved:
>
>1. `comparingNumericallyLeadingZerosAhead()` is a confusing name for
>me, as the "ahead" has no meaning in my mind; IMO the name
>`comparingNumericallyLeadingZerosFirst()` better implies that "0001" <
>"001" < "01".
>2. It wasn't clear to me from the javadoc that the comparators compare
>strings like "abc9" and "abc10" as "abc9" < "abc10", so I think they should
>include more examples.
>3. There's a typo in the javadocs for both methods: "represets" -->
>"represents".
>4. Where the javadocs say "follows the procedure", I think it'd make
>more grammatical sense if they said "does the following".
>5. The lines which say "at the boundary of a subsequence, consisting
>of decimal digits, the" would flow better if they said "at the boundary of
>a subsequence *consisting solely of decimal digits*, then the". Note
>the removal of the comma between "subsequence" and "consisting".
>
> Hope this helps!
>
> Jonathan
>
> On 23 July 2017 at 05:36, Ivan Gerasimov <ivan.gerasi...@oracle.com>
> wrote:
>
>> Hello!
>>
>> This is a gentle reminder.
>>
>> The proposed comparator implementation would be particularly useful when
>> one will need to compare version strings.
>> Some popular file managers also use similar comparing algorithm, as the
>> results often look more natural to the human eyes (e.g. File Explorer on
>> Windows, Files on Ubuntu).
>>
>> Now, as Java 10 is been worked on, to sort the list of Java names
>> correctly, this kind of comparator is needed:
>>
>> Look: a list { ... "Java 8", "Java 9", "Java 10" } definitely looks nicer
>> than { "Java 1", "Java 10", "Java 2", ... }  :-)
>>
>> Would you please help review the proposal?
>>
>> With kind regards,
>> Ivan
>>
>>
>>
>> On 7/19/17 1:41 AM, Ivan Gerasimov wrote:
>>
>>> Hello!
>>>
>>> It is a proposal to provide a String comparator, which will pay
>>> attention to the numbers embedded into the strings (should they present).
>>>
>>> This proposal was initially discussed back in 2014 and seemed to bring
>>> some interest from the community:
>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-De
>>> cember/030343.html
>>>
>>> In the latest webrev two methods are added to the public API:
>>> j.u.Comparator.comparingNumerically() and
>>> j.u.Comparator.comparingNumericallyLeadingZerosAhead().
>>>
>>> The regression test is extended to exercise this new comparator.
>>>
>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8134512
>>> WEBREV: http://cr.openjdk.java.net/~igerasim/8134512/01/webrev/
>>>
>>> Comments, suggestions are very welcome!
>>>
>>>
>> --
>> With kind regards,
>> Ivan Gerasimov
>>
>>
>

-- 
With kind regards,
Ivan Gerasimov


Re: [10] RFR 8134512 : provide Alpha-Numeric (logical) Comparator

2017-07-23 Thread Jonathan Bluett-Duncan
Meant to reply to core-libs-dev as well; doing so now.

Jonathan

On 23 July 2017 at 11:50, Jonathan Bluett-Duncan <jbluettdun...@gmail.com>
wrote:

> Hi Ivan,
>
> I'm not a reviewer per se, but I thought I'd chime in.
>
> There's a few things with Comparator.java which I think could be improved:
>
>1. `comparingNumericallyLeadingZerosAhead()` is a confusing name for
>me, as the "ahead" has no meaning in my mind; IMO the name `
>comparingNumericallyLeadingZerosFirst()` better implies that "0001" <
>"001" < "01".
>2. It wasn't clear to me from the javadoc that the comparators compare
>strings like "abc9" and "abc10" as "abc9" < "abc10", so I think they should
>include more examples.
>3. There's a typo in the javadocs for both methods: "represets" -->
>"represents".
>4. Where the javadocs say "follows the procedure", I think it'd make
>more grammatical sense if they said "does the following".
>5. The lines which say "at the boundary of a subsequence, consisting
>of decimal digits, the" would flow better if they said "at the boundary of
>a subsequence *consisting solely of decimal digits*, then the". Note
>the removal of the comma between "subsequence" and "consisting".
>
> Hope this helps!
>
> Jonathan
>
> On 23 July 2017 at 05:36, Ivan Gerasimov <ivan.gerasi...@oracle.com>
> wrote:
>
>> Hello!
>>
>> This is a gentle reminder.
>>
>> The proposed comparator implementation would be particularly useful when
>> one will need to compare version strings.
>> Some popular file managers also use similar comparing algorithm, as the
>> results often look more natural to the human eyes (e.g. File Explorer on
>> Windows, Files on Ubuntu).
>>
>> Now, as Java 10 is been worked on, to sort the list of Java names
>> correctly, this kind of comparator is needed:
>>
>> Look: a list { ... "Java 8", "Java 9", "Java 10" } definitely looks nicer
>> than { "Java 1", "Java 10", "Java 2", ... }  :-)
>>
>> Would you please help review the proposal?
>>
>> With kind regards,
>> Ivan
>>
>>
>>
>> On 7/19/17 1:41 AM, Ivan Gerasimov wrote:
>>
>>> Hello!
>>>
>>> It is a proposal to provide a String comparator, which will pay
>>> attention to the numbers embedded into the strings (should they present).
>>>
>>> This proposal was initially discussed back in 2014 and seemed to bring
>>> some interest from the community:
>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-De
>>> cember/030343.html
>>>
>>> In the latest webrev two methods are added to the public API:
>>> j.u.Comparator.comparingNumerically() and
>>> j.u.Comparator.comparingNumericallyLeadingZerosAhead().
>>>
>>> The regression test is extended to exercise this new comparator.
>>>
>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8134512
>>> WEBREV: http://cr.openjdk.java.net/~igerasim/8134512/01/webrev/
>>>
>>> Comments, suggestions are very welcome!
>>>
>>>
>> --
>> With kind regards,
>> Ivan Gerasimov
>>
>>
>


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Jonathan Bluett-Duncan
Hi Cédric,

I don't know if it's been considered, but has anyone from the Gradle team
asked the Bazel team about this problem?

They may have useful insight about this problem with `System.getenv`,
considering that Bazel is apparently very fast and (partially) written in
Java.

I hope this helps.

Best regards,
Jonathan


Re: Optional.isEmpty()

2017-04-22 Thread Jonathan Bluett-Duncan
Hi Peter,

Your reasoning has personally convinced me that a method like `isEmpty()`
would pull its weight. However, at the risk of bikeshedding, I think it
should be named differently, as `isEmpty()` immediately makes me think that
`findModule()` returns a List, which I'd easily find confusing.

How about `isNotPresent()` instead?

Best regards,
Jonathan

On 22 April 2017 at 10:40, Peter Levart  wrote:

> Hi,
>
> Seeing the following line in some JDK test that was up for review:
>
> return cf.findModule(target).orElse(null) == null;
>
> I immediately jumped to suggest it would look better if written as:
>
> return !cf.findModule(target).isPresent();
>
>
> But then I leaned back and asked myself: "Would it really?"
>
> The boolean negation in front of a chain of method calls tends to visually
> disappear from the view when we finally reach the offending method and
> might get missed when quickly browsing the code. In this respect,
> ".orElse(null) == null" is actually better. But the best would of course be
> something like that:
>
> return cf.findModule(target).isEmpty();
>
> What do you think? Would this pull its weight?
>
> Regards, Peter
>
>


Re: [PATCH] 8178117: Add public state constructors for Int/Long/DoubleSummaryStatistics

2017-04-06 Thread Jonathan Bluett-Duncan
Hi Chris,

Unfortunately the patch you sent (in what I presume was an attachment) is
missing. I believe the OpenJDK mailing list servers intentionally strip out
attachments in all emails, which seems to be at odds with the advice given
in http://openjdk.java.net/contribute/. (Either the contribution advice or
the servers should be changed, ideally!)

I understand that one can host patches somewhere official, but I've
forgotten the details of the process.

Can anyone help?

Jonathan


On 6 April 2017 at 15:08, Chris Dennis  wrote:

> Hi Paul (et al)
>
> Like all things API there are wrinkles here when it comes to implementing.
>
> This patch isn’t final, there appears to be no existing test coverage for
> these classes beyond testing the compensating summation used in the double
> implementation, and I left off adding any until it was decided how much
> parameter validation we want (since that’s the only testing that can really
> occur here).
>
> The wrinkles relate to the issues around instances that have suffered
> overflow in count and/or sum which the existing implementation doesn’t
> defend against:
>
>  * I chose to ignore all parameters if 'count == 0’ and just leave the
> instance empty. I held off from validating min, max and count however. This
> obviously 'breaks things’ (beyond how broken they would already be) if
> count == 0 only due to overflow.
>  * I chose to fail if count is negative.
>  * I chose to enforce that max and min are logically consistent with count
> and sum, this can break the moment either one of the overflows as well (I’m
> also pretty sure that the FPU logic is going to suffer here too - there
> might be some magic I can do with a pessimistic bit of rounding on the FPU
> multiplication result).
>
> I intentionally went with the most aggressive parameter validation here to
> establish one end of what could be implemented.  There are arguments for
> this and also arguments for the other extreme (no validation at all).
> Personally I’m in favor of the current version where the creation will fail
> if the inputs are only possible through overflow (modulo fixing the FPU
> precision issues above) even though it’s at odds with approach of the
> existing implementation.
>
> Would appreciate thoughts/feedback.  Thanks.
>
> Chris
>
>
> P.S. I presume tests would be required for the parameter checking (once it
> is finalized)?
>
>


Re: [NEW BUG]: Avoid allocations in String.replace(CharSequence, CharSequence) in case no replacement happened

2017-03-02 Thread Jonathan Bluett-Duncan
Hi Vitaly,


> If there's no early bail out, there's an implicit null check when
> replacement.toString() is called...


Oh apologies! I had missed in the code that NPE does get thrown if
`replacement` is null, no matter if the `if (j < 0) { ... }` block is
entered or not, because `replacement.toString()` gets called immediately
after the if block, which of course executes an implicit null-check.

In that case, the patch LGTM. :)

Kind regards,
Jonathan


Re: [NEW BUG]: Avoid allocations in String.replace(CharSequence, CharSequence) in case no replacement happened

2017-03-02 Thread Jonathan Bluett-Duncan
Hi Christoph and Vitaly,

I am struggling to understand why the null-check for `replacement` has been
moved within the `if (j < 0) { ... }` block. Could one of you explain to me
the reasoning behind this change?

I ask because, AFAICT, making it so that it only throws an NPE if
`replacement` is null *and* `j < 0`, rather than throwing NPE at the very
start of the method, seems to be going against the class-level doc which
says, "Unless otherwise noted, passing a null argument to a constructor or
method in this class will cause a NullPointerException to be thrown."

Other than that, I am equally happy with `replacement` itself having an
explicit-nullness-related comment instead of target having an
implicit-nullness comment - it seems to deliver the same sort of message. :)

Kind regards,
Jonathan

On 2 March 2017 at 07:26, Christoph Dreis 
wrote:

> Hey Vitaly and Jonathan,
>
> > As mentioned offline, I would move the null check right before "return
> this".
>
> @Vitaly: Thanks again. Adjusted.
> @Jonathan: Thanks. Thought about that as well, but I'd probably rather go
> with explaining the explicit nullcheck.
>
> See the adjusted patch below. What do you think?
>
> = PATCH ==
> diff --git a/src/java.base/share/classes/java/lang/String.java
> b/src/java.base/share/classes/java/lang/String.java
> --- a/src/java.base/share/classes/java/lang/String.java
> +++ b/src/java.base/share/classes/java/lang/String.java
> @@ -2177,11 +2177,13 @@
>   */
>  public String replace(CharSequence target, CharSequence replacement) {
>  String tgtStr = target.toString();
> -String replStr = replacement.toString();
>  int j = indexOf(tgtStr);
>  if (j < 0) {
> +// Explicit nullcheck of replacement to fulfill NPE contract
> in early out
> +Objects.requireNonNull(replacement);
>  return this;
>  }
> +String replStr = replacement.toString();
>  int tgtLen = tgtStr.length();
>  int tgtLen1 = Math.max(tgtLen, 1);
>  int thisLen = length();
>
>


Re: [NEW BUG]: Avoid allocations in String.replace(CharSequence, CharSequence) in case no replacement happened

2017-03-01 Thread Jonathan Bluett-Duncan
Hi Christoph,

I think it's worth adding a small comment to the end of `String tgtStr =
target.toString();`, saying that the null check is implicit, in case people
read the code in the future and get confused as to why only `replacement`
is apparently null-checked. What do you think?

Kind regards,
Jonathan

On 1 March 2017 at 23:47, Christoph Dreis 
wrote:

> Hey Vitaly,
>
> > Seems like a good idea.  You probably want to null check 'replacement'
> before the bail out as the method is specified as throwing NPE in that case.
>
> Thanks for your feedback. See the adjusted patch below.
>
> = PATCH ==
>
> diff --git a/src/java.base/share/classes/java/lang/String.java
> b/src/java.base/share/classes/java/lang/String.java
> --- a/src/java.base/share/classes/java/lang/String.java
> +++ b/src/java.base/share/classes/java/lang/String.java
> @@ -2176,12 +2176,13 @@
>   * @since 1.5
>   */
>  public String replace(CharSequence target, CharSequence replacement) {
> +Objects.requireNonNull(replacement);
>  String tgtStr = target.toString();
> -String replStr = replacement.toString();
>  int j = indexOf(tgtStr);
>  if (j < 0) {
>  return this;
>  }
> +String replStr = replacement.toString();
>  int tgtLen = tgtStr.length();
>  int tgtLen1 = Math.max(tgtLen, 1);
>  int thisLen = length();
>
>


Re: Stream based API for tree like structures

2017-01-20 Thread Jonathan Bluett-Duncan
Hi Kasper,

If what you're looking for is a more immediate solution, I can think of a
couple of libraries which may do the trick:

   1. Durian 's TreeDef and TreeStream.
   Difficult to understand and use at first, but I believe it ticks a good
   number of boxes for you.
   2. Guava 21 's
   `TreeTraverser` with the library's new Java 8 support. For example,

   
`Streams.stream(TreeTraverser.using(Node::getChildNodes).{preOrder|postOrder|breadthFirst}Traversal(rootNode));`

Both of these solutions allow depth- and breadth-first traversals (the
Guava solution allows both preOrder and postOrder depth traversals), but
it's not clear to me if either of them allows you to set a maximum depth to
traverse into.

Hope this helps.

Best regards,
Jonathan

On 20 January 2017 at 15:00, Remi Forax  wrote:

> https://gist.github.com/forax/bca6877e019d134f87c4cb1e8efae9cd
>
> Rémi
>
> - Mail original -
> > De: "Kasper Nielsen" 
> > À: "core-libs-dev" 
> > Envoyé: Vendredi 20 Janvier 2017 11:03:41
> > Objet: Stream based API for tree like structures
>
> > Hi,
> >
> > Sorry if this is a bit off-topic, but I thought but I thought it might
> have
> > some general interest if Java ever got some proper tree/graph collection
> > classes.
> >
> > Has anyone developed a stream based API that allows for tree based
> travels.
> > I'm mainly thinking about functionality for
> >
> > 1)
> > Whether or not to do recursive traversal of all child nodes, only 1 level
> > of child nodes, or just siblings
> >
> > 2)
> > Order of traversal: depth/breadth first.
> >
> > I'm trying to avoid an explosion of methods such as
> > streamSieblingsDepthOrderFirst.
> > One thought I had was taking a var arg of options to stream and
> > parallelStream such as:
> > enum TreeStreamOptions {
> >   SIEBLINGS_ONLY, RECURSIVELY, DEPTH_FIRST, BREATH_FIRST;
> > }
> > Stream stream(TreeStreamOptions... options)
> > Stream parallelStream(TreeStreamOptions... options)
> >
> > another one could be
> >
> > class TreeStreamOptions {
> >   TreeStreamOptions setDepthFirst();
> >   TreeStreamOptions setBreathFirst();
> >   TreeStreamOptions setDepth(); (0 sieblings only, Integer.MAX->infinity)
> > }
> > Stream stream(TreeStreamOptions options)
> > Stream parallelStream(TreeStreamOptions options)
> >
> > While a visitor pattern would make sense for many use cases. I really
> like
> > the simplicity of just working with streams.
> >
> > Maybe someone has some thoughts on this.
> >
> > Best
> >   Kasper
>


Re: RFR: 8054214: JapaneseEra.getDisplayName doesn't return names if it's an additional era

2016-12-08 Thread Jonathan Bluett-Duncan
Hi Masayoshi,

I'm not a reviewer, but there's something in JapaneseEra.java.cdiff.html

which
isn't terribly clear to me, so I wonder if you could clarify it for me.

In the method `getDisplayName(TextStyle style, Locale locale)`, it's not
clear to me why the call to `Objects.requireNonNull(locale, "locale");` is
within the if-statement rather than on the first line. Is this because
`Era.super::getDisplayName` already has a null check of its own?

Kind regards,
Jonathan

On 8 December 2016 at 08:55, Masayoshi Okutsu 
wrote:

> Hi,
>
> Please review the fix for JDK-8054214. It was necessary to override
> Era::getDisplayName to get era names from the specified property. This one
> fixes getAbbreviation() as well.
>
> Issue:
> https://bugs.openjdk.java.net/browse/JDK-8054214
>
> Webrev:
> http://cr.openjdk.java.net/~okutsu/9/8054214/webrev.00/
>
> Thanks,
> Masayoshi
>
>


Re: Enhancements to Java Collections API

2016-12-01 Thread Jonathan Bluett-Duncan
Hi John,

I'm not sure about most of the suggestions you've made, especially the
Bidi* ones, but I understand that something like your suggestion for an
enhanced `Optional getIfPresent(K key)` is (or was) being investigated
by Mr. Goetz himself and other people over at Project Valhalla
. However, I can't seem to find
the archived email where it was announced...

Kind regards,
Jonathan

On 29 November 2016 at 20:13, John Platts  wrote:

> There are many features that are missing from the Java Collections API
> that should be added to the Java Collections API, including the following:
>
>   *   Bidirectional iterators for collections other than lists
>  *   New interfaces
> *   BidiIterator - bidirectional iterator
>*   extended by ListIterator interface
>*   has hasPrevious() and previous() methods
> *   ReverseIterable - reverse iterable
>*   extends Iterable
>*   extended by List, Deque, NavigableSet, and BidiIterable
>*   has descendingIterator() and reverseForEach(Consumer super T>) methods
> *   BidiIterable - bidirectional iterable
>*   extends ReverseIterable
>*   bidiIterator() returns a BidiIterator
>*   bidiIteratorFromEnd() returns a BidiIterator that is
> positioned just past the last element in the collection
>*   extended by List, BidiIterableCollection, BidiIterableSet,
> BidiIterableNavigableSet, and BidiIterableDeque
> *   BidiIterableCollection - bidirectional iterable collection,
> extends BidiIterable and Collection
>*   extended by List, BidiIterableSet,
> BidiIterableNavigableSet, and BidiIterableDeque
>*   default implementation of bidiIterator() method added to
> the java.util.List interface that delegates to java.util.List.listIterator()
>*   default implementation of bidiIteratorFromEnd() method
> added to the java.util.List interface that delegates to
> java.util.List.listIterator(size())
> *   BidiIterableSet - bidirectional iterable set, extends
> BidiCollection and Set
> *   BidiIterableNavigableSet - bidirectional iterable navigable
> iterable set, extends BidiIterableSet and NavigableSet
>*   subSet(), headSet(), and tailSet() all return
> BidiIterableNavigableSet instances
> *   BidiIterableDeque - bidirectional iterable deque, extends
> BidiCollection and Deque
> *   BidiIterableMap - bidirectional iterable map
>*   extends Map
>*   keySet() and entrySet() return BidiIterableSet
>*   values() returns BidiCollection
>*   has reverseForEach(BiConsumer) method
> *   BidiIterableNavigableMap - bidirectional iterable navigable map
>*   extends BidiIterableMap and NavigableMap
>*   keySet(), navigableKeySet(), and descendingKeySet() all
> return BidiNavigableSet
>*   entrySet() returns BidiIterableSet
>*   values() returns BidiCollection
>*   subMap(), headMap() and tailMap() all return
> BidiIterableNavigableMap instances
>  *   BidiIterableDeque interface implemented on java.util.ArrayDeque,
> java.util.concurrent.ConcurrentLinkedDeque, 
> java.util.concurrent.LinkedBlockingDeque,
> and java.util.LinkedList
>  *   BidiIterableSet interface implemented on java.util.LinkedHashSet,
> java.util.TreeSet, and java.util.concurrent.ConcurrentSkipListSet
>  *   BidiIterableNavigableSet interface implemented on
> java.util.TreeSet and java.util.concurrent.ConcurrentSkipListSet
>  *   BidiIterableMap interface implemented on java.util.LinkedHashMap,
> java.util.TreeMap, and java.util.concurrent.ConcurrentSkipListMap
>  *   BidiIterableNavigableMap interface implemented on
> java.util.TreeMap and java.util.concurrent.ConcurrentSkipListMap
>   *   Pollable interface
>  *   Extended by the Queue, Deque, and NavigableSet interfaces
>  *   Also implemented by the java.lang.ref.ReferenceQueue class
>  *   Contains the poll() method, which is already defined by the Queue
> and Deque interfaces
>   *   BidiPollable interface
>  *   Extends the Pollable interface
>  *   Extended by the Deque and NavigableSet interfaces
>  *   Contains the pollFirst() and pollLast() methods, which are
> already defined in both the Deque and NavigableSet interfaces
>   *   Navigable versions of java.util.EnumMap and java.util.EnumSet
>  *   Option #1: Implement the NavigableMap interface on top of
> java.util.EnumMap and java.util.EnumSet
>  *   Option #2: Implement separate java.util.NavigableEnumMap and
> java.util.NavigableEnumSet classes that behave similarly to the existing
> java.util.EnumMap and java.util.EnumSet classes, except that the
> java.util.NavigableEnumMap class has the functionality of the
> java.util.NavigableMap interface and that the 

Re: RFR: JDK-8167648: java.io.PrintWriter should have PrintWriter((String|File), Charset) constructors

2016-11-23 Thread Jonathan Bluett-Duncan
Hi Patrick,

Have you considered making `withAutoFlush()` return the `PrintWriter`
itself, allowing fluent code snippets like the following?

```
PrintWriter writer = new PrintWriter(new File("path/to/file.txt"),
StandardCharsets.UTF_8).withAutoFlush();
```

Kind regards,
Jonathan

On 23 November 2016 at 13:09, Patrick Reinhart  wrote:

> Added those new public constructors:
>
> PrintWriter(OutputStream, Charset)
> PrintWriter(File, Charset)
> withAutoFlush()
>
> Also added a new private constructor:
>
> PrintWriter(OutputStream, Charset, boolean)
>
> and rewired the OutputStream constructor calls to this private constructor.
>
>
> Here's the webrev:
>
> http://cr.openjdk.java.net/~reinhapa/reviews/8167648/webrev.00
>
>
> -Patrick
>


Re: 8152515: (logging) LogManager.resetLogger should ignore LinkageError

2016-10-28 Thread Jonathan Bluett-Duncan
Oh, I see! Thanks for pointing out my misconception for me. :)

In that case, this fix looks fine to me as a non-reviewer.

Kind regards,
Jonathan

On 28 October 2016 at 17:15, Roger Riggs <roger.ri...@oracle.com> wrote:

> Hi Jonathan,
>
> There is no issue in this case.
> LinkageError does not extend Exception so they are disjoint at the catch
> clauses.
> And the compiler produces an error if a catch clause hides another
> exception to keep
> the mistake from being hidden.
>
> $.02, Roger
>
>
>
> On 10/28/2016 12:00 PM, Jonathan Bluett-Duncan wrote:
>
>> I've an awful suspicion that the `catch (LinkageError e)` block is
>> unreachable, as the `catch (Exception e)` block would run first, being
>> located above the other block in the source code.
>>
>> Is my suspicion correct?
>>
>> Kind regards,
>> Jonathan
>>
>> On 28 October 2016 at 16:36, Jason Mehrens <jason_mehr...@hotmail.com>
>> wrote:
>>
>> Daniel,
>>>
>>> Looks good to me.
>>>
>>> Thanks for fixing this!
>>>
>>> Jason
>>>
>>> 
>>> From: Daniel Fuchs <daniel.fu...@oracle.com>
>>> Sent: Friday, October 28, 2016 6:51 AM
>>> To: core-libs-dev
>>> Cc: Jason Mehrens
>>> Subject: RFR: 8152515: (logging) LogManager.resetLogger should ignore
>>> LinkageError
>>>
>>> Hi,
>>>
>>> Please find below a trivial  patch for:
>>>
>>> 8152515: (logging) LogManager.resetLogger should ignore LinkageError
>>> https://bugs.openjdk.java.net/browse/JDK-8152515
>>>
>>>
>>> Patch:
>>> http://cr.openjdk.java.net/~dfuchs/webrev_8152515/webrev.00/
>>>
>>> The issue might occur at shutdown, when a handler that makes uses
>>> of some APIs provided by an OSGI bundle which was already closed
>>> by the shutdown process is in turn closed by the LogManager.Cleaner
>>> thread. In that case some subclasses of LinkageError may be thrown,
>>> interrupting the reset process and preventing other handlers from
>>> being closed properly.
>>>
>>> The patch proposes to trivially ignore LinkageError at shutdown while
>>> the LogManager.Cleaner thread is running.
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>>
>


Re: 8152515: (logging) LogManager.resetLogger should ignore LinkageError

2016-10-28 Thread Jonathan Bluett-Duncan
I've an awful suspicion that the `catch (LinkageError e)` block is
unreachable, as the `catch (Exception e)` block would run first, being
located above the other block in the source code.

Is my suspicion correct?

Kind regards,
Jonathan

On 28 October 2016 at 16:36, Jason Mehrens 
wrote:

> Daniel,
>
> Looks good to me.
>
> Thanks for fixing this!
>
> Jason
>
> 
> From: Daniel Fuchs 
> Sent: Friday, October 28, 2016 6:51 AM
> To: core-libs-dev
> Cc: Jason Mehrens
> Subject: RFR: 8152515: (logging) LogManager.resetLogger should ignore
> LinkageError
>
> Hi,
>
> Please find below a trivial  patch for:
>
> 8152515: (logging) LogManager.resetLogger should ignore LinkageError
> https://bugs.openjdk.java.net/browse/JDK-8152515
>
>
> Patch:
> http://cr.openjdk.java.net/~dfuchs/webrev_8152515/webrev.00/
>
> The issue might occur at shutdown, when a handler that makes uses
> of some APIs provided by an OSGI bundle which was already closed
> by the shutdown process is in turn closed by the LogManager.Cleaner
> thread. In that case some subclasses of LinkageError may be thrown,
> interrupting the reset process and preventing other handlers from
> being closed properly.
>
> The patch proposes to trivially ignore LinkageError at shutdown while
> the LogManager.Cleaner thread is running.
>
> best regards,
>
> -- daniel
>


Re: RFR: jsr166 jdk9 integration wave 12

2016-10-20 Thread Jonathan Bluett-Duncan
Ah, I see. Thanks for answering my questions Martin.

Sounds like there's no real benefit then to using guava-testlib in this
changeset instead of CollectionTest. And even if the benefit of using it
was considered to be high enough in the future, it would probably need its
own JSR, since I presume it would be applicable to a number of places in
the jdk, making it a humongous change.

Overall, it seems sensible to wait until e.g. JUnit 5 develops a worthwhile
solution to the dynamic test generation problem, at which point we could
then consider it for writing exhaustive tests for the jdk collections.

Thanks again,
Jonathan


On 20 October 2016 at 04:45, Martin Buchholz <marti...@google.com> wrote:

>
>
> On Wed, Oct 19, 2016 at 4:40 PM, Jonathan Bluett-Duncan <
> jbluettdun...@gmail.com> wrote:
>
>> Hi Martin,
>>
>> By collections infrastructure, do you mean something like the collection
>> testers in guava-testlib?
>>
>> If so, I agree that JUnit 5's dynamic tests look promising for
>> implementing such an infrastructure. I admit I don't have all the context
>> here, but would using guava-testlib in the meantime be a viable medium- or
>> short-term solution? Or would its dependence on JUnit 3/4 make switching
>> impractical? Or, perhaps, has development into CollectionTest gone so far
>> that, for that reason instead, it would be impractical until switch, at
>> least until something superior using e.g. JUnit 5's dynamic tests is made?
>>
>
> I'm embarrassed to say I'm not familiar enough with guava's testlib.
> Guava does have generic collection oriented tests, and even runs them on
> jdk classes. (Someone on the jdk quality team should be running the guava
> tests using development jdks!).  I'm not familiar enough with the tools to
> work on this myself, but I encourage someone who is to do that.
>
> I see that guava testlib does have:
>
> TestsForQueuesInJavaUtil.java
> TestsForSetsInJavaUtil.java
> TestsForListsInJavaUtil.java
> TestsForMapsInJavaUtil.java
>
> and that the infrastructure there is vaguely similar to what I ended up
> doing.  JDK version skew is a problem, because openjdk development is
> focused on jdk9, while guava is still trying to digest jdk8.
>


Re: RFR: jsr166 jdk9 integration wave 12

2016-10-19 Thread Jonathan Bluett-Duncan
Hi Martin,

By collections infrastructure, do you mean something like the collection
testers in guava-testlib?

If so, I agree that JUnit 5's dynamic tests look promising for implementing
such an infrastructure. I admit I don't have all the context here, but
would using guava-testlib in the meantime be a viable medium- or short-term
solution? Or would its dependence on JUnit 3/4 make switching impractical?
Or, perhaps, has development into CollectionTest gone so far that, for that
reason instead, it would be impractical until switch, at least until
something superior using e.g. JUnit 5's dynamic tests is made?

Kind regards,
Jonathan

On 19 October 2016 at 23:21, Martin Buchholz  wrote:

> These tests are maintained outside of openjdk and have only recently been
> made available within openjdk.
>
> There may be a future where all the consumers of these tests are willing to
> accept a good test framework.  Junit 5 dynamic tests look promising
> http://junit.org/junit5/docs/current/user-guide/#writing-
> tests-dynamic-tests
> but it may be a decade before we can use it.
>
> Probably code bases that include interfaces (e.g. Collection) should
> provide infrastructure to test those interfaces for any implementation, but
> openjdk does not try to do so.  CollectionTest is an experimental step in
> that direction, but it looks we could do better with Junit 5 some day.  The
> openjdk and jtreg projects can try to help by supporting junit 5 earlier
> rather than later.
>
> On Wed, Oct 19, 2016 at 12:19 PM, Stuart Marks 
> wrote:
>
> >
> >
> > On 10/18/16 11:00 AM, Martin Buchholz wrote:
> >
> >> There's already a certain amount of mixing, e.g. stream tests sometimes
> >> test
> >> j.u.c. and Queue tests sometimes test all the Queue implementations.
> >> Unlike
> >> non-test code, some redundancy and divergence of testing frameworks and
> >> styles
> >> is fine.  I still haven't found a way of writing shared tests for
> >> implementations of an interface that I really like.
> >>
> >
> > It's probably the case that divergence of testing frameworks is
> > unavoidable, or at least it's impractical. I doubt that it's worthwhile
> to
> > rewrite all the straight jtreg tests into TestNG, or JUnit, or whatever.
> > But I'd draw the line before saying this is "fine." [1]
> >
> > Maintaining the tests' organization is pretty important. Most of the
> > collections tests are in jdk/test/java/util though they're spread around
> a
> > bit uncomfortably even here. But now it's, oh, ArrayDeque and
> > ArrayList.removeIf tests are over *here* instead. Having things spread
> > around increases the likelihood of cases being missed or being tested
> > redundantly.
> >
> > The test groups have to be maintained as well. I know I've been bitten by
> > problems (not in collections) where I *thought* I had run the right set
> of
> > tests, but it ended up that I hadn't, resulting in me breaking the build.
> > As it turns out, jdk_collections_core doesn't include any ArrayDeque
> tests.
> > Hmmm. Well, maybe jdk_collections_core isn't useful. It would have been
> > nice to know this when I added it last year. :-/ [2]
> >
> > As things stand, I'm basically OK with this changeset going in. But it
> > does seem to highlight some areas that need some future cleanup,
> > particularly in the tests and the test group declarations.
> >
> > s'marks
> >
> > [1] http://knowyourmeme.com/memes/this-is-fine
> >
> > [2] http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/7a0c06013ae6
> >
> >
>


Re: Review request: JDK-8167630 jdeps --generate-module-info forgets to close the resource after checking any unnamed package

2016-10-12 Thread Jonathan Bluett-Duncan
Not a reviewer, but looks good to me. :-)

Kind regards,
Jonathan

On 12 Oct 2016 23:54, "Lance Andersen"  wrote:

> +1
> > On Oct 12, 2016, at 6:52 PM, Mandy Chung  wrote:
> >
> > Simple patch close the ClassFileReader with try-with-resource.
> >
> >
> > diff --git a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
> b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
> > --- a/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
> > +++ b/src/jdk.jdeps/share/classes/com/sun/tools/jdeps/JdepsTask.java
> > @@ -680,9 +680,9 @@
> > private boolean genModuleInfo(JdepsConfiguration config) throws
> IOException {
> > // check if any JAR file contains unnamed package
> > for (String arg : inputArgs) {
> > +try (ClassFileReader reader = 
> > ClassFileReader.newInstance(Paths.get(arg)))
> {
> > Optional classInUnnamedPackage =
> > -ClassFileReader.newInstance(Paths.get(arg))
> > -.entries().stream()
> > +reader.entries().stream()
> > .filter(n -> n.endsWith(".class"))
> > .filter(cn -> toPackageName(cn).isEmpty())
> > .findFirst();
> > @@ -696,6 +696,7 @@
> > return false;
> > }
> > }
> > +}
> >
> > ModuleInfoBuilder builder
> > = new ModuleInfoBuilder(config, inputArgs,
> options.genModuleInfo);
> >
> > Thanks
> > Mandy
>
>  
>   <
> http://oracle.com/us/design/oracle-email-sig-198324.gif>
>  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: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-10-12 Thread Jonathan Bluett-Duncan
Yes, you're very welcome Stuart. :-)

Kind regards,
Jonathan

On 12 Oct 2016 21:49, "Patrick Reinhart" <patr...@reini.net> wrote:

You’re welcome :-)

-Patrick

Am 12.10.2016 um 22:41 schrieb Stuart Marks <stuart.ma...@oracle.com>:

Tests passed, spec change approved, changeset pushed:

http://hg.openjdk.java.net/jdk9/dev/jdk/rev/af71f6a36731

Jonathan, thanks for your contribution. And Patrick, thanks again for
hosting the webrev.

s'marks

On 10/12/16 6:53 AM, Jonathan Bluett-Duncan wrote:

Hi all,

Thank you very much for all reviewing my changeset over the last few days
and for finding the bits I forgot to transfer from webrev01 to webrev02.
I've been quiet as I'm still busy with university and will be for the
foreseeable future.

Stuart, many thanks again for sponsoring the change and going through the
whole procedure for me. I look forward to the rest of your results.

Kind regards,
Jonathan


Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-10-12 Thread Jonathan Bluett-Duncan
Hi all,

Thank you very much for all reviewing my changeset over the last few days
and for finding the bits I forgot to transfer from webrev01 to webrev02.
I've been quiet as I'm still busy with university and will be for the
foreseeable future.

Stuart, many thanks again for sponsoring the change and going through the
whole procedure for me. I look forward to the rest of your results.

Kind regards,
Jonathan


Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-10-10 Thread Jonathan Bluett-Duncan
Hi all,

Would you kindly review the latest webrev now?

http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.02

Thanks in advance.

Kind regards,
Jonathan

On 7 October 2016 at 21:59, Patrick Reinhart <patr...@reini.net> wrote:

> Here is the latest webrev:
>
> http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.02
>
> -Patrick
>
>
>
> > Am 07.10.2016 um 12:00 schrieb Jonathan Bluett-Duncan <
> jbluettdun...@gmail.com>:
> >
> > Hi all,
> >
> > Sorry for the delayed response, I've been busy lately with university
> and other things.
> >
> > Thank you all for your comments. I'll leave the DateTimeFormatter
> comment in, as you requested Stephen and Roger, and I'll work again with
> Patrick as soon as I'm ready.
> >
> > Kind regards,
> > Jonathan
> >
> > On 6 October 2016 at 09:38, Stephen Colebourne <scolebou...@joda.org>
> wrote:
> > On 6 October 2016 at 00:00, Stuart Marks <stuart.ma...@oracle.com>
> wrote:
> > >> I think you should perform no change to DateTimeFormatter (other than
> > >> a comment) as part of this changeset. The behaviour of that
> > >> DateTimeFormatter method is subtle, and I now suspect that what we
> > >> have there might be the best option.
> > >
> > > I had recommended removing the comment from DateTimeFormatter, but if
> > > Stephen wants the comment in, that's fine with me. In fact I'll defer
> to
> > > what Stephen (and Roger Riggs) want with this code, since they're the
> > > maintainers.
> >
> > I think it makes sense to leave the new comment in. All further change
> > should be under 8167222.
> >
> > Stephen
> >
>
>


Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-10-07 Thread Jonathan Bluett-Duncan
Hi all,

Sorry for the delayed response, I've been busy lately with university and
other things.

Thank you all for your comments. I'll leave the DateTimeFormatter comment
in, as you requested Stephen and Roger, and I'll work again with Patrick as
soon as I'm ready.

Kind regards,
Jonathan

On 6 October 2016 at 09:38, Stephen Colebourne  wrote:

> On 6 October 2016 at 00:00, Stuart Marks  wrote:
> >> I think you should perform no change to DateTimeFormatter (other than
> >> a comment) as part of this changeset. The behaviour of that
> >> DateTimeFormatter method is subtle, and I now suspect that what we
> >> have there might be the best option.
> >
> > I had recommended removing the comment from DateTimeFormatter, but if
> > Stephen wants the comment in, that's fine with me. In fact I'll defer to
> > what Stephen (and Roger Riggs) want with this code, since they're the
> > maintainers.
>
> I think it makes sense to leave the new comment in. All further change
> should be under 8167222.
>
> Stephen
>


Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-10-05 Thread Jonathan Bluett-Duncan
Stuart, thank you very much for your continued review of this changeset,
and for finding those usages of CookieManager::get in Grepcode for me. I've
applied the comment you suggested for ModuleFinder and I've also fixed the
NetscapeCookieStore typo.

Stephen, thank you for getting back about DateTimeFormatter. It's not clear
to me what should be done with
TCKDateTimeFormatter::test_resolverFields_listOfOneNull in this case. Do I
delete it; or do I change it to test that a null TemporalField param causes
a NullPointerException to be thrown; or do I do something else? May I have
your continued thoughts on this?

Kind regards,
Jonathan


Re: RFR: 8167005: Comment on the need for an empty constructor in ArrayList$Itr

2016-10-05 Thread Jonathan Bluett-Duncan
Okay, that makes sense to me! Thank you for your explanations Claes and
Stuart.

Kind regards,
Jonathan

On 5 October 2016 at 01:57, Stuart Marks <stuart.ma...@oracle.com> wrote:

> Right, the main point of the comment is to tell the reader the constructor
> isn't superfluous, to prevent it from being cleaned up and accidentally
> causing a regression. Full history can reside in the commit comment, the
> bug database, and in these email logs.
>
> I'd put in a link to a bug only when there's some action on this code
> associated with that bug, e.g., "don't remove this code until bug XXX
> has been fixed."
>
> s'marks
>
>
> On 10/4/16 5:00 PM, Claes Redestad wrote:
>
>> Hi Jonathan,
>>
>> the aim isn't to add an in-depth explanation to the code about exactly
>> the circumstance that led to this constructor and comment being added,
>> but to put a clear message that it was simply, in fact, deliberate, so
>> even the proposed comment might be going further than strictly necessary.
>>
>> I'm also not convinced of the value of putting explicit links to the
>> bug actually pushed, since there's an implicit link in the commit
>> itself anyhow.
>>
>> Thanks!
>>
>> /Claes
>>
>> On 2016-10-04 23:20, Jonathan Bluett-Duncan wrote:
>>
>>> The explanation which Stuart gives for this change in
>>> https://bugs.openjdk.java.net/browse/JDK-8167005 seems to describe why
>>> this constructor is needed much better than the comment itself does. So
>>> I wonder if it's worth adding the link to the bug report in the comment.
>>> E.g.
>>>
>>> // prevent generation of synthetic class required for access to private
>>> // constructor. See: https://bugs.openjdk.java.net/browse/JDK-8167005
>>>
>>> Kind regards,
>>> Jonathan
>>>
>>> On 4 October 2016 at 21:28, Stuart Marks <stuart.ma...@oracle.com
>>> <mailto:stuart.ma...@oracle.com>> wrote:
>>>
>>>
>>>
>>> On 10/4/16 3:55 AM, Claes Redestad wrote:
>>>
>>>
>>> On 2016-10-04 12:52, Aleksey Shipilev wrote:
>>>
>>> On 10/04/2016 12:50 PM, Claes Redestad wrote:
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~redestad/8167005/webrev.01/
>>> <http://cr.openjdk.java.net/~redestad/8167005/webrev.01/
>>> >
>>>
>>>
>>> OK.
>>>
>>> Thanks for the speedy review! :-)
>>>
>>>
>>> Thanks for looking at this. The shorter text in the bug report is ok
>>> too.
>>>
>>> s'marks
>>>
>>>
>>>


Re: RFR: 8167005: Comment on the need for an empty constructor in ArrayList$Itr

2016-10-04 Thread Jonathan Bluett-Duncan
The explanation which Stuart gives for this change in
https://bugs.openjdk.java.net/browse/JDK-8167005 seems to describe why this
constructor is needed much better than the comment itself does. So I wonder
if it's worth adding the link to the bug report in the comment. E.g.

// prevent generation of synthetic class required for access to private
// constructor. See: https://bugs.openjdk.java.net/browse/JDK-8167005

Kind regards,
Jonathan

On 4 October 2016 at 21:28, Stuart Marks  wrote:

>
>
> On 10/4/16 3:55 AM, Claes Redestad wrote:
>
>>
>> On 2016-10-04 12:52, Aleksey Shipilev wrote:
>>
>>> On 10/04/2016 12:50 PM, Claes Redestad wrote:
>>>
 Webrev: http://cr.openjdk.java.net/~redestad/8167005/webrev.01/

>>>
>>> OK.
>>>
>>> Thanks for the speedy review! :-)
>>
>
> Thanks for looking at this. The shorter text in the bug report is ok too.
>
> s'marks
>
>


Re: [9] Review Request: 8143077 Deprecate InputEvent._MASK in favor of InputEvent._DOWN_MASK

2016-09-30 Thread Jonathan Bluett-Duncan
Hi Sergey,

I'm not a reviewer, but after reading the deprecation messages in Event.java

* @deprecated It is recommended that {@code AWTEvent} class and its
> subclasses
> * be used instead.


I get the impression they would read better without the redundant "class"
in the middle, like so.

* @deprecated It is recommended that {@code AWTEvent} and its subclasses
> * be used instead.


Kind regards,
Jonathan


On 30 September 2016 at 16:45, Sergey Bylokhov 
wrote:

> Hello.
>
> Please review the fix for jdk9.
>
> This is request to deprecate the obsolete flags inside InputEvent. This
> constants were marked as obsolete in jdk1.4 and were replaced by the new
> one. In jdk1.4 the documentation were update with notion that the new
> constants should be used. And this bug is about official deprecation of
> them.
>
> We can replace old constants by the new one in the whole jdk, but I
> updated only the code in InputEvent to make change smaller and safer. So at
> least the new code in jdk and the code in applications will start to use
> the new constants.
>
> The changes in jconsole are necessary to fix deprecation warning.
>
> jprt build passed, no new issues were found by jck/jtreg tests.
>
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8143077
> Webrev can be found at: http://cr.openjdk.java.net/~serb/8143077/webrev.00
>
> --
> Best regards, Sergey.
>


Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-30 Thread Jonathan Bluett-Duncan
Hi Stuart,

Thanks for doing such a thorough review of the parts you've managed to look
at so far.

I see you had a number of questions in your numbered bullet points, so I'll
do my best to answer them below.

1) It actually didn't occur to me that there was a potential TOCTOU problem
in ModuleFinder.compose, despite the fact that I found a potential problem
in FileTreeIterator - it completely missed me, so thank you for finding it!
I'll see if I can put a similar comment there to what I wrote in
FileTreeIterator.

2) I seem to remember doing an at least semi-thorough search of the JDK
codebase, and coming to the conclusion that none of the code I touched was
being serialized by the JDK itself. I think I mentioned this is a previous
email, but I also remember saying that I wasn't sure if I took everything
into account, as I'm not that familiar with serialization. So I decided
just now to do a search on Grepcode for usages of CookieManager.get

and
CookieHandler.get
,
and curiously it looks like they're only used in sun classes in the JDK. So
this change seems safe to me, unless I've missed something?

Regarding code search engines, I know of sourcegraph.com, but I think it
requires an account to use their service, and I don't know which
repositories they index apart from GitHub.

3) In my local copy of jdk9, I've removed the TOCTOU comment in
FileTreeIterator and changed List.of back to Arrays.asList, as your
explanation regarding FileTreeWalker has convinced me that TOCTOU is not a
real problem here, and I don't know if future memory improvements to
List.of(T...) (like returning an ImmutableCollections.List1 when there's
only 1 element) are worth the extra time spent copying into a new list.

4) The 'resolverFields' problem/comment was regarding DateTimeFormatter
(see this part of latest webrev
),
where I realised I couldn't use Set.of instead of
Collections.unmodifiableSet(new HashSet<>(Arrays.asList(*))), because I
noticed that one of the java.time tests was testing whether
DateTimeFormatter.withResolverFields(TemporalField...) could accept null
parameters, which made using Set.of impossible since it's null-hostile (as
in it throws NullPointerException upon being constructed with null
element(s)).

So I never did submit a change with that bit of code replaced with Set.of.
Instead I wrote a comment in the method explaining why one can't use
Set.of. I don't really know if Stephen Colebourne saw that comment, though.

Stephen Colebourne, are you still fine with all the changes I've made to
the java.time classes? http://cr.openjdk.java.net/~reinhapa/reviews/8134373/
webrev.01/

Kind regards,
Jonathan

P.S. I'll wait until everything's reviewed before asking for a new webrev.


Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-15 Thread Jonathan Bluett-Duncan
Hi all,

Stuart, thank you very much for your thorough response.

Regarding serializability concerns, I've just checked my changes and all
non-test code in the JDK which calls it, and it doesn't seem to me that
they affect any fields in `Serializable` classes or the return values of
methods which either return instances of `Serializable` classes or whose
javadoc mention that the returned object is serializable. So I'm somewhat
certain that my changes are serialization-compatible, but only somewhat,
because I'm not that familiar with the intricacies of serialization...

Considering how busy Stuart is, would anyone else be happy to sponsor this
change?

Kind regards,
Jonathan

On 15 September 2016 at 18:20, Stuart Marks <stuart.ma...@oracle.com> wrote:

> Hi all,
>
> Unfortunately I don't have time to work on any of this at the moment,
> because of JavaOne preparation, and JavaOne next week.
>
> Jonathan, thanks for pushing forward with this. I'm glad that others have
> picked it up.
>
> Patrick, thanks for posting the changeset on Jonathan's behalf. This is
> very helpful.
>
> A few comments regarding issues raised up-thread.
>
> Regarding the (non)singleton-ness of the empty collections, this is
> covered by
>
> https://bugs.openjdk.java.net/browse/JDK-8156079
> consider making empty instances singletons
>
> It wasn't a design decision to make them not singletons. The spec
> requirement is only that the returned instance satisfy the requirements of
> the interfaces it implements (e.g., List) and nothing more. Certainly there
> is no spec requirement regarding object identity.
>
> Making the empty collections singletons is the "obvious" thing to do, but
> it's often the case that the "obvious" thing isn't the right thing. That
> said, it may still be the right thing to make them singletons. Given the
> proposed extension to the JDK 9 schedule, it might be possible to change
> this in JDK 9.
>
> Note that List.of() should be functionally equivalent to
> Collections.emptyList() -- and correspondingly for Set and Map -- but they
> do differ. In particular, they have different serialization formats.
>
> Also on this topic, please note comments that Daniel Fuchs and I have
> added to
>
> https://bugs.openjdk.java.net/browse/JDK-8134373
>
> regarding serialization compatibility. Reviewers should take care that
> updating code to use these new collection factories doesn't change any
> serialization formats. Unfortunately I am not confident that we have
> sufficient tests for serialization compatibility.
>
> s'marks
>
>
>
> On 9/15/16 7:02 AM, Jonathan Bluett-Duncan wrote:
>
>> Wow, lots of discussion went on since I was busy doing other stuff!
>>
>> Thanks Patrick for doing the work of creating a new webrev for me. Really
>> appreciated!
>>
>> Pavel already mentioned it, but I think List.of instead of
>> Collections.emptyList in ZoneOffsetTransition is the right thing to do for
>> visual and behavioural consistency. If it turns out that we need to revert
>> to Collections.empty* and Collections.unmodifiable* for e.g.
>> serializability or class loading concerns, then I'd be happy to revert
>> both
>> of the lines I touched. Otherwise I believe that List.of should be used
>> consistently.
>>
>> I think Stuart made List.of() non-singleton because there wasn't any
>> evidence that it made List.of() more memory- or time-intensive than
>> Collections.emptyList(), but I might be wrong on this. I'm sure he can
>> explain more or correct me in this case.
>>
>> Kind regards,
>> Jonathan
>>
>>
>> On 15 September 2016 at 13:33, Patrick Reinhart <patr...@reini.net>
>> wrote:
>>
>> Hello together,
>>>
>>> I tried to process all suggested change input into the following new
>>> webrev:
>>>
>>> http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.01
>>>
>>> Give me feedback if something is missing/wrong
>>>
>>> -Patrick
>>>
>>>
>>> On 2016-09-15 13:48, Pavel Rappo wrote:
>>>
>>> Daniel, Claes,
>>>>
>>>> List.of() and Collections.emptyList() are not the same. The behaviours
>>>> are
>>>> different. Moreover, immutable static factory methods return instances
>>>> which are
>>>> value-based. I believe it also means we are not tied with unconditional
>>>> instantiation, and in case of empty collections/maps could probably
>>>> return the
>>>> same object every time.
>>>>
>>>> We should ask Stuart why it has been done like that in the first place.
>>>> Maybe
>>>> out of concern people might synchronize of those objects? I don't know.
>>>> Let's
>>>> say for now it's an implementation-specific detail.
>>>>
>>>> On 15 Sep 2016, at 12:35, Claes Redestad <claes.redes...@oracle.com>
>>>>
>>>>> wrote:
>>>>>
>>>>> +1
>>>>>
>>>>> I don't mind List.of() aesthetically, but there are places where
>>>>> startup/footprint is important where Collections.emptyList()
>>>>> is simply superior, e.g., constituting permanent data structures
>>>>> such as the module graph during early bootstrap.
>>>>>
>>>>>
>>>>>


Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-15 Thread Jonathan Bluett-Duncan
Patrick, would you kindly line-wrap the TOCTOU comment in
http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.01/src/java.base/share/classes/java/nio/file/FileTreeIterator.java.cdiff.html
for me, so that each line has 80 characters or less (and maybe insert a `.`
at the end)?

Kind regards,
Jonathan

On 15 September 2016 at 15:06, Jonathan Bluett-Duncan <
jbluettdun...@gmail.com> wrote:

> Pavel,
>
>
>> 6. I couldn't find any evidence that `resolverFields` might contain
>> `null`.
>> Am I missing a javadoc or a line of code? Maybe we should talk to
>> java.time
>> folks to see if it's the case?
>>
>
> When I ran the tests for java.time, one of them failed because `null` was
> being passed to `resolverFields`, so I came the conclusion that it was
> unsafe to make a change there.
>
> Kind regards,
> Jonathan
>
>


Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-15 Thread Jonathan Bluett-Duncan
Pavel,


> 6. I couldn't find any evidence that `resolverFields` might contain `null`.
> Am I missing a javadoc or a line of code? Maybe we should talk to java.time
> folks to see if it's the case?
>

When I ran the tests for java.time, one of them failed because `null` was
being passed to `resolverFields`, so I came the conclusion that it was
unsafe to make a change there.

Kind regards,
Jonathan


Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-15 Thread Jonathan Bluett-Duncan
Wow, lots of discussion went on since I was busy doing other stuff!

Thanks Patrick for doing the work of creating a new webrev for me. Really
appreciated!

Pavel already mentioned it, but I think List.of instead of
Collections.emptyList in ZoneOffsetTransition is the right thing to do for
visual and behavioural consistency. If it turns out that we need to revert
to Collections.empty* and Collections.unmodifiable* for e.g.
serializability or class loading concerns, then I'd be happy to revert both
of the lines I touched. Otherwise I believe that List.of should be used
consistently.

I think Stuart made List.of() non-singleton because there wasn't any
evidence that it made List.of() more memory- or time-intensive than
Collections.emptyList(), but I might be wrong on this. I'm sure he can
explain more or correct me in this case.

Kind regards,
Jonathan


On 15 September 2016 at 13:33, Patrick Reinhart  wrote:

> Hello together,
>
> I tried to process all suggested change input into the following new
> webrev:
>
> http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.01
>
> Give me feedback if something is missing/wrong
>
> -Patrick
>
>
> On 2016-09-15 13:48, Pavel Rappo wrote:
>
>> Daniel, Claes,
>>
>> List.of() and Collections.emptyList() are not the same. The behaviours are
>> different. Moreover, immutable static factory methods return instances
>> which are
>> value-based. I believe it also means we are not tied with unconditional
>> instantiation, and in case of empty collections/maps could probably
>> return the
>> same object every time.
>>
>> We should ask Stuart why it has been done like that in the first place.
>> Maybe
>> out of concern people might synchronize of those objects? I don't know.
>> Let's
>> say for now it's an implementation-specific detail.
>>
>> On 15 Sep 2016, at 12:35, Claes Redestad 
>>> wrote:
>>>
>>> +1
>>>
>>> I don't mind List.of() aesthetically, but there are places where
>>> startup/footprint is important where Collections.emptyList()
>>> is simply superior, e.g., constituting permanent data structures
>>> such as the module graph during early bootstrap.
>>>
>>>


Re: RFR: JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-15 Thread Jonathan Bluett-Duncan
Thanks Patrick!

Stuart, would you be happy to sponsor this change?

If anyone else has any thoughts regarding this change, then I'm all ears.

Bug link: https://bugs.openjdk.java.net/browse/JDK-8134373
Rationale for changes:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-September/043484.html

Kind regards,
Jonathan


On 14 September 2016 at 21:55, Patrick Reinhart <patr...@reini.net> wrote:

> Hi Jonathan,
>
> Here are your changes in a webrev:
>
> http://cr.openjdk.java.net/~reinhapa/reviews/8134373/webrev.00
>
> -Patrick
>
> Am 13.09.2016 um 14:45 schrieb Patrick Reinhart <patr...@reini.net>:
>
> Hi  Jonathan,
>
> On 2016-09-13 02:13, Jonathan Bluett-Duncan wrote:
>
> Hi Patrick,
> Thank you very much for the offer to hold my patch for me!
>
>
> No problem.
>
> Is it common practice to send patches to others using PGP?
>
>
> No, this is my personal setting.
>
> Kind regards,
> Jonathan
> On 12 September 2016 at 21:08, Patrick Reinhart <patr...@reini.net> wrote:
>
> Hi Jonathan,
> As I just also wanted to help some more clean-up in the JDKs final phase, I
> could offer you to hold that patch. Just send it to me and I will prepare
> the webrev for you….
> -Patrick
>
>
> -Patrick
>
>
>


Re: [PATCH] JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-12 Thread Jonathan Bluett-Duncan
Hi Patrick,

Thank you very much for the offer to hold my patch for me!

Is it common practice to send patches to others using PGP?

Kind regards,
Jonathan

On 12 September 2016 at 21:08, Patrick Reinhart <patr...@reini.net> wrote:

> Hi Jonathan,
>
> As just also wanted to help some more clean-up in the JDKs final phase, I
> could offer you to hold that patch. Just send it to me and I will prepare
> the webrev for you….
>
> -Patrick
>
>
> > Am 12.09.2016 um 20:36 schrieb Jonathan Bluett-Duncan <
> jbluettdun...@gmail.com>:
> >
> > Hi David,
> >
> > Thanks for letting me know about the attachment stripping behaviour, and
> > reminding me about the current state of the JDK 9 release schedule.
> >
> > Stuart, would you be happy to host my patch on cr.openjdk.java.net? If
> not,
> > do you know who else might be happy to host it for me? Or alternatively,
> > would you prefer I wait until development of Java 9 Updates and/or Java
> 10
> > starts?
> >
> > Kind regards,
> > Jonathan
> >
> > On 12 September 2016 at 01:50, David Holmes <david.hol...@oracle.com>
> wrote:
> >
> >> Hi Jonathon,
> >>
> >> Attachments get stripped from most of the mailing lists so you will need
> >> to find someone to host these for you on cr.openjdk.java.net.
> >>
> >> That aside you may be hard pressed to find anyone who can look at this
> >> future work now, given where things are with the JDK 9 release schedule.
> >>
> >> Cheers,
> >> David
> >>
> >>
> >> On 11/09/2016 9:42 AM, Jonathan Bluett-Duncan wrote:
> >>
> >>> Hi all,
> >>>
> >>> Would you kindly review this patch to replace existing uses of
> >>> Collections.unmodifiable*(*Arrays.asList(*)) and plain
> Arrays.asList(*)
> >>> with (List|Set|Map).of(*). You may find the patch files in the
> >>> attachments.
> >>>
> >>> My rationale for replacing uses of Collections.unmodifiable*... with
> >>> (List|Set|Map).of is to make use of the memory savings allowed by the
> >>> newer
> >>> APIs.
> >>>
> >>> The general rationale for replacing the Arrays.asList calls I've
> touched
> >>> is
> >>> to again make use of memory savings, but this may be naive or misguided
> >>> reasoning on my part, as Arrays.asList may or may not be more
> >>> memory-efficient than List.of. However, where I've replaced
> Arrays.asList
> >>> for List.of in FileTreeIterator, my reasoning for doing so instead was
> to
> >>> help prevent TOCTOU attacks, but again this may be misguided on my
> part.
> >>>
> >>> It doesn't seem practical to me to include new unit tests, as these are
> >>> mainly performance improvements, but if it's believed that new unit
> tests
> >>> are needed, then I'd be happy to go back and try to include some.
> >>>
> >>> Kind regards,
> >>> Jonathan
> >>>
> >>>
>
>


Re: [PATCH] JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-12 Thread Jonathan Bluett-Duncan
Hi David,

Thanks for letting me know about the attachment stripping behaviour, and
reminding me about the current state of the JDK 9 release schedule.

Stuart, would you be happy to host my patch on cr.openjdk.java.net? If not,
do you know who else might be happy to host it for me? Or alternatively,
would you prefer I wait until development of Java 9 Updates and/or Java 10
starts?

Kind regards,
Jonathan

On 12 September 2016 at 01:50, David Holmes <david.hol...@oracle.com> wrote:

> Hi Jonathon,
>
> Attachments get stripped from most of the mailing lists so you will need
> to find someone to host these for you on cr.openjdk.java.net.
>
> That aside you may be hard pressed to find anyone who can look at this
> future work now, given where things are with the JDK 9 release schedule.
>
> Cheers,
> David
>
>
> On 11/09/2016 9:42 AM, Jonathan Bluett-Duncan wrote:
>
>> Hi all,
>>
>> Would you kindly review this patch to replace existing uses of
>> Collections.unmodifiable*(*Arrays.asList(*)) and plain Arrays.asList(*)
>> with (List|Set|Map).of(*). You may find the patch files in the
>> attachments.
>>
>> My rationale for replacing uses of Collections.unmodifiable*... with
>> (List|Set|Map).of is to make use of the memory savings allowed by the
>> newer
>> APIs.
>>
>> The general rationale for replacing the Arrays.asList calls I've touched
>> is
>> to again make use of memory savings, but this may be naive or misguided
>> reasoning on my part, as Arrays.asList may or may not be more
>> memory-efficient than List.of. However, where I've replaced Arrays.asList
>> for List.of in FileTreeIterator, my reasoning for doing so instead was to
>> help prevent TOCTOU attacks, but again this may be misguided on my part.
>>
>> It doesn't seem practical to me to include new unit tests, as these are
>> mainly performance improvements, but if it's believed that new unit tests
>> are needed, then I'd be happy to go back and try to include some.
>>
>> Kind regards,
>> Jonathan
>>
>>


Fwd: Make iterators cloneable?

2016-09-11 Thread Jonathan Bluett-Duncan
I think O(N^2)-like performance is unavoidable here Dave. However, although
Peter's algorithm is indeed O(N^2), it should be faster than Tagir's and
reasonable(-ish) in practice.

This is because the inner loop starts off not executing at all in the outer
loop's first iteration; and each time the outer loop iterates, the inner
loop's number of iterations increases by 1, eventually reaching N
iterations when the outer loop itself hits iteration N.

So although Peter's algorithm is O(N^2) in theory, in practice it shouldn't
or isn't as bad as most O(N^2) algos, since the number of steps it takes is
less than some ratio of N^2 for sufficiently large N.

On 11 September 2016 at 17:55, Dave Brosius 
wrote:

> Sure, but both of those algorithms are n^2, which is a bit painful,
> especially given all the pointer chasing that occurs with iterators.
>
>
>
> On 09/11/2016 08:20 AM, Peter Levart wrote:
>
>> Hi,
>>
>> Even if the elements are not comparable, you could rely on the fact that
>> Collection(s) usually create iterators with stable iteration order, so you
>> could do the following:
>>
>>
>> Set set = Set.of(1, 2, 3, 4);
>>
>> Iterator it1 = set.iterator();
>> for (int n1 = 0; it1.hasNext(); n1++) {
>> Integer e1 = it1.next();
>> Iterator it2 = set.iterator();
>> for (int n2 = 0; n2 < n1; n2++) {
>> Integer e2 = it2.next();
>> System.out.println(e1 + " <-> " + e2);
>> }
>> }
>>
>>
>> Regards, Peter
>>
>> On 09/11/2016 02:02 PM, Tagir F. Valeev wrote:
>>
>>> Hello!
>>>
>>> As your keys are comparable, you can create normal iterators and
>>> filter the results like this:
>>>
>>> for(String v1 : s) {
>>>for(String v2 : s) {
>>>  if(v1.compareTo(v2) < 0) {
>>>System.out.println(v1 + " <-->" + v2);
>>>  }
>>>}
>>> }
>>>
>>> Or using Stream API:
>>>
>>> s.stream().flatMap(v1 -> s.stream()
>>>  .filter(v2 -> v1.compareTo(v2) < 0).map(v2 -> v1 + " <-->" + v2))
>>>   .forEach(System.out::println);
>>>
>>> With best regards,
>>> Tagir Valeev.
>>>
>>>
>>> DB> It would be nice to be able to associate each element in a collection
>>> DB> with another element in the collection, which is something very
>>> easily
>>> DB> done with index based collections, but with sets, etc this isn't so
>>> DB> easy... unless i'm having a brainfart.
>>>
>>> DB> So i'd like to do this, but Iterator doesn't implement Cloneable...
>>> Any
>>> DB> reason not to? or is there another way that's missing me?
>>>
>>> DB> public class ItClone {
>>>
>>> DB>  public static void main(String[] args) {
>>> DB>  Set s = Collections.newSetFromMap(new
>>> DB> ConcurrentHashMap());
>>>
>>> DB>  s.add("Fee");
>>> DB>  s.add("Fi");
>>> DB>  s.add("Fo");
>>> DB>  s.add("Fum");
>>>
>>> DB>  Iterator it1 = s.iterator();
>>> DB>  while (it1.hasNext()) {
>>> DB>  String v1 = it1.next();
>>>
>>> DB>  Iterator it2 = (Iterator) it1.*clone*();
>>> DB>  while (it2.hasNext()) {
>>> DB>  String v2 = it2.next();
>>>
>>> DB>  System.out.println(v1 + " <-->" + v2);
>>> DB>  }
>>> DB>  }
>>> DB>  }
>>> DB> }
>>>
>>>
>>
>


Re: [PATCH] JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-10 Thread Jonathan Bluett-Duncan
Sorry, it seems Gmail is not cooperating with me...

If anyone does need to see the patches in-body, then I'd be happy to inline
them directly in a reply, but that wouldn't be ideal since there are 18 of
them in total... :/

On 11 September 2016 at 00:54, Jonathan Bluett-Duncan <
jbluettdun...@gmail.com> wrote:

> Oops, it didn't fully register for me that I actually needed to include
> attachments in the body! So here they are again, this time in the body
> (hopefully).
>


Re: [PATCH] JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-10 Thread Jonathan Bluett-Duncan
Oops, it didn't fully register for me that I actually needed to include
attachments in the body! So here they are again, this time in the body
(hopefully).


[PATCH] JDK-8134373: explore potential uses of convenience factories within the JDK

2016-09-10 Thread Jonathan Bluett-Duncan
Hi all,

Would you kindly review this patch to replace existing uses of
Collections.unmodifiable*(*Arrays.asList(*)) and plain Arrays.asList(*)
with (List|Set|Map).of(*). You may find the patch files in the attachments.

My rationale for replacing uses of Collections.unmodifiable*... with
(List|Set|Map).of is to make use of the memory savings allowed by the newer
APIs.

The general rationale for replacing the Arrays.asList calls I've touched is
to again make use of memory savings, but this may be naive or misguided
reasoning on my part, as Arrays.asList may or may not be more
memory-efficient than List.of. However, where I've replaced Arrays.asList
for List.of in FileTreeIterator, my reasoning for doing so instead was to
help prevent TOCTOU attacks, but again this may be misguided on my part.

It doesn't seem practical to me to include new unit tests, as these are
mainly performance improvements, but if it's believed that new unit tests
are needed, then I'd be happy to go back and try to include some.

Kind regards,
Jonathan


Re: Make iterators cloneable?

2016-09-10 Thread Jonathan Bluett-Duncan
Ah okay Louis, if that's the case then that certainly makes sense, and I'd
agree that there's no good way of doing so, as one would need to copy the
set into a list.

Dave, did Louis hit the mark? If not, would you kindly go into further
detail as to exactly what it is you're trying to do?

Best,
Jonathan

On 10 September 2016 at 23:36, Jonathan Bluett-Duncan <
jbluettdun...@gmail.com> wrote:

> Hi Dave,
>
> Rather than using Iterator.clone(), how about you just call
> collection.iterator() 2 times to return 2 unique, non-same iterators;
> something like the following:
>
> import java.util.Collections;
> import java.util.Iterator;
> import java.util.Set;
> import java.util.concurrent.ConcurrentHashMap;
>
> public class Example {
>   public static void main(String[] args) {
> Set s = Collections.newSetFromMap(new ConcurrentHashMap<String, 
> Boolean>());
>
> s.add("Fee");
> s.add("Fi");
> s.add("Fo");
> s.add("Fum");
>
> Iterator it1 = s.iterator();
> for (String v1 = null; it1.hasNext(); v1 =it1.next()) {
>   Iterator it2 = s.iterator(); // a completely separate iterator 
> to it1
>   for (String v2 = null; it2.hasNext(); v2 = it2.next()) {
> System.out.println(v1 + " <-->" + v2);
>   }
> }
>   }
> }
>
>
> Or, even better, if you're using Java 5+, you can skip using Iterators
> altogether and use for-loops directly:
>
> import java.util.Collections;
> import java.util.Set;
> import java.util.concurrent.ConcurrentHashMap;
>
> public class Example {
>   public static void main(String[] args) {
> Set s = Collections.newSetFromMap(new ConcurrentHashMap<String, 
> Boolean>());
>
> s.add("Fee");
> s.add("Fi");
> s.add("Fo");
> s.add("Fum");
>
> for (String v1 : s) {
>   for (String v2 : s) {
> System.out.println(v1 + "<-->" + v2);
>   }
> }
>   }
> }
>
>
> Kind regards,
> Jonathan
>
> On 10 September 2016 at 23:13, Dave Brosius <dbros...@mebigfatguy.com>
> wrote:
>
>> It would be nice to be able to associate each element in a collection
>> with another element in the collection, which is something very easily done
>> with index based collections, but with sets, etc this isn't so easy...
>> unless i'm having a brainfart.
>>
>> So i'd like to do this, but Iterator doesn't implement Cloneable... Any
>> reason not to? or is there another way that's missing me?
>>
>> public class ItClone {
>>
>> public static void main(String[] args) {
>> Set s = Collections.newSetFromMap(new
>> ConcurrentHashMap<String, Boolean>());
>>
>> s.add("Fee");
>> s.add("Fi");
>> s.add("Fo");
>> s.add("Fum");
>>
>> Iterator it1 = s.iterator();
>> while (it1.hasNext()) {
>> String v1 = it1.next();
>>
>> Iterator it2 = (Iterator) it1.*clone*();
>> while (it2.hasNext()) {
>> String v2 = it2.next();
>>
>> System.out.println(v1 + " <-->" + v2);
>> }
>> }
>> }
>> }
>>
>
>


Re: Make iterators cloneable?

2016-09-10 Thread Jonathan Bluett-Duncan
Hi Dave,

Rather than using Iterator.clone(), how about you just call
collection.iterator() 2 times to return 2 unique, non-same iterators;
something like the following:

import java.util.Collections;
import java.util.Iterator;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

public class Example {
  public static void main(String[] args) {
Set s = Collections.newSetFromMap(new
ConcurrentHashMap());

s.add("Fee");
s.add("Fi");
s.add("Fo");
s.add("Fum");

Iterator it1 = s.iterator();
for (String v1 = null; it1.hasNext(); v1 =it1.next()) {
  Iterator it2 = s.iterator(); // a completely separate
iterator to it1
  for (String v2 = null; it2.hasNext(); v2 = it2.next()) {
System.out.println(v1 + " <-->" + v2);
  }
}
  }
}


Or, even better, if you're using Java 5+, you can skip using Iterators
altogether and use for-loops directly:

import java.util.Collections;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

public class Example {
  public static void main(String[] args) {
Set s = Collections.newSetFromMap(new
ConcurrentHashMap());

s.add("Fee");
s.add("Fi");
s.add("Fo");
s.add("Fum");

for (String v1 : s) {
  for (String v2 : s) {
System.out.println(v1 + "<-->" + v2);
  }
}
  }
}


Kind regards,
Jonathan

On 10 September 2016 at 23:13, Dave Brosius 
wrote:

> It would be nice to be able to associate each element in a collection with
> another element in the collection, which is something very easily done with
> index based collections, but with sets, etc this isn't so easy... unless
> i'm having a brainfart.
>
> So i'd like to do this, but Iterator doesn't implement Cloneable... Any
> reason not to? or is there another way that's missing me?
>
> public class ItClone {
>
> public static void main(String[] args) {
> Set s = Collections.newSetFromMap(new
> ConcurrentHashMap());
>
> s.add("Fee");
> s.add("Fi");
> s.add("Fo");
> s.add("Fum");
>
> Iterator it1 = s.iterator();
> while (it1.hasNext()) {
> String v1 = it1.next();
>
> Iterator it2 = (Iterator) it1.*clone*();
> while (it2.hasNext()) {
> String v2 = it2.next();
>
> System.out.println(v1 + " <-->" + v2);
> }
> }
> }
> }
>


Re: Participating on https://bugs.openjdk.java.net/browse/JDK-8156070

2016-09-10 Thread Jonathan Bluett-Duncan
Hi Aleksey,

Sorry for the late reply. Thank for you pointing me to the contribute page;
it gave me all the information I needed.

Stuart, I'm now ready to make a patch review request, so submit one as soon
as I have the time.

Kind regards,
Jonathan

On 6 September 2016 at 09:46, Aleksey Shipilev <aship...@redhat.com> wrote:

> On 09/06/2016 01:49 AM, Jonathan Bluett-Duncan wrote:
> > I decided to have a crack at "JDK-8134373 explore potential uses of
> > convenience factories within the JDK" today. I recognise it's only a P4
> bug
> > and most likely won't be prioritised for Java 9, but I believe I found a
> > number of places where uses of
> > "Collections.unmodifiableList(Arrays.asList(...))" and
> "Arrays.asList(..)"
> > can be replaced with "List.of(...)". I've thus made some changes to my
> > local clone of the jdk9 codebase.
> >
> > I'm now at a point where I'm unfamiliar with what one does next when
> > contributing to OpenJDK, so I wonder if you'd kindly advise me on what my
> > next step(s) are.
>
> See: http://openjdk.java.net/contribute/
>
> In short, sign and send OCA, prepare and send the patch for review, work
> with your sponsor.
>
> Thanks,
> -Aleksey
>


Re: Participating on https://bugs.openjdk.java.net/browse/JDK-8156070

2016-09-05 Thread Jonathan Bluett-Duncan
Hi Stuart,

Thank you for the very detailed response.

I decided to have a crack at "JDK-8134373 explore potential uses of
convenience factories within the JDK" today. I recognise it's only a P4 bug
and most likely won't be prioritised for Java 9, but I believe I found a
number of places where uses of
"Collections.unmodifiableList(Arrays.asList(...))" and "Arrays.asList(..)"
can be replaced with "List.of(...)". I've thus made some changes to my
local clone of the jdk9 codebase.

I'm now at a point where I'm unfamiliar with what one does next when
contributing to OpenJDK, so I wonder if you'd kindly advise me on what my
next step(s) are.

Kind regards,
Jonathan

On 3 September 2016 at 01:41, Stuart Marks  wrote:

> Hi Jonathan,
>
> Welcome to OpenJDK, and thanks for your interest in JEP 269!
>
> I see you found a the subtasks of JDK-8156070, which is basically a
> container for a bunch of ideas for work on the JEP 269 collections. I took
> a quick look through them and this one seemed promising:
>
> JDK-8134373 explore potential uses of convenience factories within the JDK
>
> This isn't necessarily what you're looking for, as it's not working on the
> collections themselves, but instead it's attempting to put them to use
> within the JDK. I've added some notes to this bug about the dynamics of
> retrofitting usage into the JDK. Although it's not work directly on the
> collections, finding out where they can and cannot be applied would
> definitely be useful. It would also be useful, of course, if space savings
> could be realized by using them instead of conventional collections (where
> appropriate, of course).
>
> I'd also suggest looking beyond the JEP 269 tasks and at broader
> collections issues. To query for unresolved collections issues, go to
> bugs.openjdk.java.net; click Issues > Search for Issues; click Advanced;
> and paste the following query into the search box:
>
> project = jdk and component = core-libs and subcomponent =
> java.util:collections and resolution = unresolved
>
> (You should be able to run this query without having a JIRA account.)
>
> There's a wider variety of things here, which might turn up something more
> appropriate. (Then again, the choice might be overwhelming.)
>
> As Mark Reinhold mentioned, we're now in the "ramp-down phase 1" part of
> the JDK 9 schedule, so the proposal is to start restricting the kind of
> work that can be done. The low-priority bugs are probably off the table
> until JDK 10 (I'm not sure when that tree opens up), but some things like
> docs and tests are still fair game for the time being. (Also note that "the
> javadoc" is a mixture of specifications and informative documentation, and
> the differences between them aren't always apparent. Specification changes
> require a good deal more rigor, and they go through extra rounds of review.)
>
> Anyway, I hope you find something of interest. I'm going heads-down to
> prepare for JavaOne (Sep 18-22) but I'll try to keep an eye out for
> followup messages.
>
> s'marks
>
> [1] http://mail.openjdk.java.net/pipermail/jdk9-dev/2016-August/
> 004777.html


Re: Participating on https://bugs.openjdk.java.net/browse/JDK-8156070

2016-09-02 Thread Jonathan Bluett-Duncan
Hi Stuart Marks,

I've been told by Martijn Verburg on the discuss mailing list that you're
the lead person for the Immutable Collections enhancements piece.

Is there anything I can do to help you out with https://bugs.openjdk.
java.net/browse/JDK-8156070 or any other part of JEP 269
<http://openjdk.java.net/jeps/269>?

Kind regards,
Jonathan

On 28 August 2016 at 17:25, Jonathan Bluett-Duncan <jbluettdun...@gmail.com>
wrote:

> Hi all,
>
> My name is Jonathan, and it's my first time posting to this mailing list.
>
> I'm writing to you all as I'm rather interested in contributing to this
> area of the OpenJDK. Particularly, I'm thinking I'd quite like to help out
> with one of the subtasks for the Immutable Collections enhancements
> <https://bugs.openjdk.java.net/browse/JDK-8156070>. However I don't have
> specific ideas and I don't know which of the subtasks would most closely
> match my skills and knowledge, so I wondered if I could have some guidance
> as to what I could do and how I can get started.
>
> FYI, I've partially read the contribution instructions
> <http://openjdk.java.net/contribute/>, and I just submitted an OCA
> earlier today, so I don't expect to be actually ready to participate until
> I hear back from Oracle, but nonetheless I'd like to try to get the ball
> rolling as soon as possible.
>
> Kind regards,
> Jonathan Bluett-Duncan
>


Participating on https://bugs.openjdk.java.net/browse/JDK-8156070

2016-08-28 Thread Jonathan Bluett-Duncan
Hi all,

My name is Jonathan, and it's my first time posting to this mailing list.

I'm writing to you all as I'm rather interested in contributing to this
area of the OpenJDK. Particularly, I'm thinking I'd quite like to help out
with one of the subtasks for the Immutable Collections enhancements
<https://bugs.openjdk.java.net/browse/JDK-8156070>. However I don't have
specific ideas and I don't know which of the subtasks would most closely
match my skills and knowledge, so I wondered if I could have some guidance
as to what I could do and how I can get started.

FYI, I've partially read the contribution instructions
<http://openjdk.java.net/contribute/>, and I just submitted an OCA earlier
today, so I don't expect to be actually ready to participate until I hear
back from Oracle, but nonetheless I'd like to try to get the ball rolling
as soon as possible.

Kind regards,
Jonathan Bluett-Duncan