Re: Impact of code difference in Collection#contains() worth improving?

[Guy Steele]

On Aug 29, 2014, at 7:33 PM, John Rose  wrote:
> . . .
> Changing source code on based on the difference between 0 and -1 is almost as 
> pointless as removing whitespace and comments . . .

Well said, John!  But I cannot resist recalling that one of the earliest pieces 
of software in the implementation of EMACS (back when the implementation 
language was TECO, a text-editing language) was a routine that, when it loaded 
TECO macros from a file, would carefully remove comments and excess whitespace 
in order to improve the execution speed of the macros (and therefore the 
response time of the EMACS keystrokes)!  We have come a long, long way in 38 
years.

--Guy

Re: Impact of code difference in Collection#contains() worth improving?

[John Rose]

On Aug 29, 2014, at 5:27 PM, Martin Buchholz  wrote:

> I agree that the benefit is very small, but I am coming at this from source 
> code consistency and bytecode size (not jitted code performance), and I think 
> bytecode size is at least one of the problems with assert.

We agree about JDK-6445664.  See my JDK-6316156, which I just linked to your 
bug.  Unfortunately the current inlining heuristics create a performance 
linkage (an accidental one) to the size of a method's bytecode instructions.

(BTW, there is no such linkage to the overall class file size, lest anyone go a 
quest to shrink class files.  For example, you won't get penalized for using a 
CONSTANT_Long instead of a CONSTANT_Integer of the same value.)

I am not against individual ad hoc reductions in bytecode size, or 
regularization of code, but I am resolutely against any proposition that a 
steady stream of such suggestions is worth the expense of anyone's professional 
attention.

To be positive...  :-)  New useful APIs (streams), or real algorithmic changes 
to old libraries (TimSort), make me smile a lot.  More than that, bugs like 
JDK-6445664, which connect desirable code styles to required JIT optimizations, 
excite my interest, because they show opportunities that we can exploit if we 
coordinate our efforts.  (Streams are also in the latter category, since they 
are likely to require new JIT optimizations.)  In that spirit, I'll try some 
rabble-rousing on the JIT team about this...

— John

Re: Impact of code difference in Collection#contains() worth improving?

[Martin Buchholz]

Hi John,

I agree that the benefit is very small, but I am coming at this from source
code consistency and bytecode size (not jitted code performance), and I
think bytecode size is at least one of the problems with assert.

...

I filed that bug long ago, but no action yet 
https://bugs.openjdk.java.net/browse/JDK-6445664
Eliminate remaining performance penalty for using assert


On Fri, Aug 29, 2014 at 4:33 PM, John Rose  wrote:

> On Aug 29, 2014, at 1:05 PM, Ulf Zibis  wrote:
>
> > Thanks for explaining this, but a very little nit: the immediate (I.e.
> -1) uses additional 32/64 bits in code which must be loaded from memory and
> wastes space in CPU cache or am I wrong? This could be saved with >= 0.
>
> I have to say you're more wrong than right about this.  Optimizers
> routinely change the form of constants.  For example, a constant 0 will
> often show up as something like "xor eax,eax", not a 32-bit literal zero
> that loads from somewhere in memory.  A comparison of the form "x > -1"
> will be freely changed to "x >= 0" and back again; the latter form may (or
> may not, depending on chip version) transform to "test eax", with no "-1"
> or "0" in sight.
>
> Also, even if you can (on some sunny Friday) detect 32 or more one-bits in
> an instruction stream, it does not follow that tweaking your source code to
> make them disappear will prevent them from reappearing (in the dark of the
> next solstice or the next update release of the JVM).  And this won't be a
> bug, because data loads from instruction cache are extremely cheap, since
> in most present chips they are pipelined well ahead of any use.
>
> Changing source code on based on the difference between 0 and -1 is almost
> as pointless as removing whitespace and comments, or swapping "a+b" to
> "b+a", hoping somehow to improve efficiency.  Sure, it might happen if you
> are lucky but because it's luck, your luck will change.
>
> I hate to say it, but (as a different example) removing "asserts" is much
> more likely to improve performance than shuffling constant spellings.  And
> this is because of a more important bug in the JIT, where inline decisions
> wrongly take into account the presence of inactive asserts.
>
> I'm not trying to evade the present subject, but in the grand scheme of
> things this email thread is shuffling equivalent chunks of furniture, which
> the JIT is going to reshuffle behind your back anyway.
>
> If you really have a measurable performance problem (as with asserts),
> file a bug against the JIT rather than trying to control code shape by
> making semantically null changes to source code.
>
> — John

Re: RFR: 8055949: ByteArrayOutputStream capacity should be maximal array size permitted by VM

[Martin Buchholz]

I decided to care just enough about the last 2x of scalability, but not
about the last 8 elements.
Your code would resize the 2g elements 8 times before finally reaching
MAX_VALUE...

Now I'm back in not-caring-anymore mode.
At least about MAX_ARRAY_SIZE.


On Wed, Aug 27, 2014 at 3:41 AM, Ulf Zibis  wrote:

> Am 25.08.2014 um 19:37 schrieb Martin Buchholz:
>
>  https://bugs.openjdk.java.net/browse/JDK-8055949
>> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/
>> ByteArrayOutputStream-MAX_ARRAY_SIZE/
>>
>> The 2x capacity gap was noticed by real users!
>>
>
> Hi Martin,
>
> the MAX_ARRAY_SIZE code now is copied to many places in JDK. Couldn't you
> better centralize the code to one place, e.g. j.u.Arrays or some hidden
> class of sun.java...?
> Isn't there a property to retrieve MAX_ARRAY_SIZE from the running VM?
>
> Imagine, some VM throws OOME above Integer.MAX_VALUE-4 and minCapacity is
> Integer.MAX_VALUE-4.
> With this code a OOME will happen:
>  124 return (minCapacity > MAX_ARRAY_SIZE) ?
>  125 Integer.MAX_VALUE :
>  126 MAX_ARRAY_SIZE;
> With this code we would avoid the OOME:
>  124 return (minCapacity > MAX_ARRAY_SIZE) ?
>  125 minCapacity :
>  126 MAX_ARRAY_SIZE;
>
> -Ulf
>
>

Re: Impact of code difference in Collection#contains() worth improving?

[John Rose]

On Aug 29, 2014, at 1:05 PM, Ulf Zibis  wrote:

> Thanks for explaining this, but a very little nit: the immediate (I.e. -1) 
> uses additional 32/64 bits in code which must be loaded from memory and 
> wastes space in CPU cache or am I wrong? This could be saved with >= 0.

I have to say you're more wrong than right about this.  Optimizers routinely 
change the form of constants.  For example, a constant 0 will often show up as 
something like "xor eax,eax", not a 32-bit literal zero that loads from 
somewhere in memory.  A comparison of the form "x > -1" will be freely changed 
to "x >= 0" and back again; the latter form may (or may not, depending on chip 
version) transform to "test eax", with no "-1" or "0" in sight.

Also, even if you can (on some sunny Friday) detect 32 or more one-bits in an 
instruction stream, it does not follow that tweaking your source code to make 
them disappear will prevent them from reappearing (in the dark of the next 
solstice or the next update release of the JVM).  And this won't be a bug, 
because data loads from instruction cache are extremely cheap, since in most 
present chips they are pipelined well ahead of any use.

Changing source code on based on the difference between 0 and -1 is almost as 
pointless as removing whitespace and comments, or swapping "a+b" to "b+a", 
hoping somehow to improve efficiency.  Sure, it might happen if you are lucky 
but because it's luck, your luck will change.

I hate to say it, but (as a different example) removing "asserts" is much more 
likely to improve performance than shuffling constant spellings.  And this is 
because of a more important bug in the JIT, where inline decisions wrongly take 
into account the presence of inactive asserts.

I'm not trying to evade the present subject, but in the grand scheme of things 
this email thread is shuffling equivalent chunks of furniture, which the JIT is 
going to reshuffle behind your back anyway.

If you really have a measurable performance problem (as with asserts), file a 
bug against the JIT rather than trying to control code shape by making 
semantically null changes to source code.

— John

RFR (7u): 8046070 - Class Data Sharing clean up and refactoring

[Ioi Lam]

Bug: https://bugs.openjdk.java.net/browse/JDK-8046070
jdk9 review thread: 
http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2014-August/012235.html

jdk9 webrev: http://cr.openjdk.java.net/~iklam/8046070-cds-cleanup-vfinal/
jdk8u-40 webrev: 
http://cr.openjdk.java.net/~iklam/8046070-cds-cleanup-backport-8u40/



The JDK 9 changes in the open source code applied mostly cleanly with 
some minor manual reshuffling of the

source code but otherwise no semantic differences.

Thanks
Ioi

Re: Impact of code difference in Collection#contains() worth improving?

[Martin Buchholz]

Refactoring the hierarchy should be separate (and much more controversial)
change.  This change is a clean win, just very small.  Refactoring runs
into space/time tradeoffs.


On Fri, Aug 29, 2014 at 3:23 PM, Vitaly Davidovich 
wrote:

> :) so if you're going to do this, is there no base class in the hierarchy
> where this can be placed (I don't have source in front of me)? That way
> there's a higher likelihood that the pattern will stay consistent (with new
> implementations at least).
>
> Sent from my phone
> On Aug 29, 2014 5:56 PM, "Martin Buchholz"  wrote:
>
> Just think - one whole byte saved for each individual change!
> I have a webrev!
> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/pico-optimize-contains/
> https://bugs.openjdk.java.net/browse/JDK-8056951
>
> Can haz review please?
>
>
> On Fri, Aug 29, 2014 at 1:05 PM, Ulf Zibis  wrote:
>
> >
> > Am 28.08.2014 um 19:46 schrieb Vitaly Davidovich:
> >
> >
> >> There's no register pressure - the immediate (I.e. -1) is encoded
> >> directly into the instruction, just like 0 would be.  The time when 0 is
> >> particularly useful is when you test for it in the zero bit of the flags
> >> register (e.g. dec followed by jz, such as when counting a loop down to
> >> 0).  Otherwise, I don't see any advantage from machine code perspective.
> >>
> >>
> > Thanks for explaining this, but a very little nit: the immediate (I.e.
> -1)
> > uses additional 32/64 bits in code which must be loaded from memory and
> > wastes space in CPU cache or am I wrong? This could be saved with >= 0.
> >
> > So if unifying the code I agree to Martin's opinion.
> >
> > -Ulf
> >
> >  The aforementioned cmov instruction is not without its own downsides, so
> >> it's unclear which is better when branch probability isn't known a
> priori.
> >>
> >> The 1 byte code is unlikely to make any difference, unless jit is turned
> >> off and you're running this through a tight loop in the interpreter
> (but if
> >> one does that, perf conversation is moot :)).
> >>
> >> Sent from my phone
> >>
> >> On Aug 28, 2014 1:28 PM, "Ulf Zibis"  >> ulf.zi...@cosoco.de>> wrote:
> >>
> >>
> >> Am 27.08.2014 um 17:51 schrieb Martin Buchholz:
> >>
> >> The ArrayList version saves one byte of bytecode, and is
> >> therefore very
> >> slightly better.  We should bless that version and use it
> >> consistently.
> >>
> >>
> >> +1
> >> Additional argument:
> >> The LinkedList code requires to load 32/64-Bit -1 into CPU. This may
> >> take some time on some
> >> CPU and at least wastes memory footprint.
> >> Additionally register pressure increases.
> >> Vitaly, please correct me, if I'm wrong, just for learning more.
> >>
> >> Another advantage is that there is no problem if some implementation
> >> of indexOf() erroneously
> >> returns another negative value than -1. I remember some compare()
> >> implementations, which
> >> sometimes return different values than only -1, 0, +1.
> >>
> >> -Ulf
> >>
> >> ArrayList:
> >>
> >>  public boolean contains(Object o) {
> >>  return indexOf(o) >= 0;
> >>  }
> >>
> >> LinkedList:
> >>
> >>  public boolean contains(Object o) {
> >>  return indexOf(o) != -1;
> >>  }
> >>
> >>
> >>
> >
>
>

Re: Impact of code difference in Collection#contains() worth improving?

[Vitaly Davidovich]

:) so if you're going to do this, is there no base class in the hierarchy
where this can be placed (I don't have source in front of me)? That way
there's a higher likelihood that the pattern will stay consistent (with new
implementations at least).

Sent from my phone
On Aug 29, 2014 5:56 PM, "Martin Buchholz"  wrote:

Just think - one whole byte saved for each individual change!
I have a webrev!
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/pico-optimize-contains/
https://bugs.openjdk.java.net/browse/JDK-8056951

Can haz review please?


On Fri, Aug 29, 2014 at 1:05 PM, Ulf Zibis  wrote:

>
> Am 28.08.2014 um 19:46 schrieb Vitaly Davidovich:
>
>
>> There's no register pressure - the immediate (I.e. -1) is encoded
>> directly into the instruction, just like 0 would be.  The time when 0 is
>> particularly useful is when you test for it in the zero bit of the flags
>> register (e.g. dec followed by jz, such as when counting a loop down to
>> 0).  Otherwise, I don't see any advantage from machine code perspective.
>>
>>
> Thanks for explaining this, but a very little nit: the immediate (I.e. -1)
> uses additional 32/64 bits in code which must be loaded from memory and
> wastes space in CPU cache or am I wrong? This could be saved with >= 0.
>
> So if unifying the code I agree to Martin's opinion.
>
> -Ulf
>
>  The aforementioned cmov instruction is not without its own downsides, so
>> it's unclear which is better when branch probability isn't known a
priori.
>>
>> The 1 byte code is unlikely to make any difference, unless jit is turned
>> off and you're running this through a tight loop in the interpreter (but
if
>> one does that, perf conversation is moot :)).
>>
>> Sent from my phone
>>
>> On Aug 28, 2014 1:28 PM, "Ulf Zibis" > ulf.zi...@cosoco.de>> wrote:
>>
>>
>> Am 27.08.2014 um 17:51 schrieb Martin Buchholz:
>>
>> The ArrayList version saves one byte of bytecode, and is
>> therefore very
>> slightly better.  We should bless that version and use it
>> consistently.
>>
>>
>> +1
>> Additional argument:
>> The LinkedList code requires to load 32/64-Bit -1 into CPU. This may
>> take some time on some
>> CPU and at least wastes memory footprint.
>> Additionally register pressure increases.
>> Vitaly, please correct me, if I'm wrong, just for learning more.
>>
>> Another advantage is that there is no problem if some implementation
>> of indexOf() erroneously
>> returns another negative value than -1. I remember some compare()
>> implementations, which
>> sometimes return different values than only -1, 0, +1.
>>
>> -Ulf
>>
>> ArrayList:
>>
>>  public boolean contains(Object o) {
>>  return indexOf(o) >= 0;
>>  }
>>
>> LinkedList:
>>
>>  public boolean contains(Object o) {
>>  return indexOf(o) != -1;
>>  }
>>
>>
>>
>

Re: Optimization 2.0 for composing strings - Was: Replace concat String to append in StringBuilder parameters

[Vitaly Davidovich]

It's probably best to teach the JIT to handle more of these cases.  The
reason is I don't think the longer compile time will be worth it
considering that, on a large codebase, there will inevitably be lots of
places where these patterns appear but have no dominance at runtime (either
because they're not called or called just a few times).  Additionally,
these patterns are unlikely to appear in hot code paths; no matter what
type of concat/string building (practical) optimizations you do, that type
of code will kill performance.  Assuming that's true, what's the value add
given the costs?

Sent from my phone
On Aug 29, 2014 4:54 PM, "Ulf Zibis"  wrote:

> Hi compiler people,
>
> is there some chance that javac could be enhanced to optimize better as
> discussed in this thread? Than refactoring of up to now better readable
> code to ugly StringBuilder@append() code would be superfluous.
> I really like the String concatenation syntax, but unfortunately it often
> causes slower code and bigger footprint.
> Optimally javac would optimize mixed use of StringBuilder, StringJoiner,
> concatenation, toString(), append(String), append(Collection) etc. to a
> single StringBuilder instance, so that the resulting code, JITed or not,
> will have better performance.
> Additionally javac could guess a reasonable initial capacity from the
> given source code.
>
>
> Am 29.08.2014 um 10:01 schrieb Wang Weijun:
>
>> So it's not that the optimization fails but there is no optimization on
>> them yet.
>>
>> I do see the .append("x") case will be easy to deal with, but it looks
>> like historically javac has not been a place to do many optimizations. It
>> mostly converts the java source to byte codes in a 1-to-1 mapping and let
>> VM do whatever it wants (to optimize). When you talked about compiling
>> multiple concatenation into using a single StringBuilder, it's more like
>> choosing the correct implementation rather than an optimization.
>>
>> I don't expect to see big change on this in the near future, so shall we
>> go on with the current enhancement?
>>
>> Thanks
>> Max
>>
>> On Aug 29, 2014, at 2:17, Ulf Zibis  wrote:
>>
>>  I mean:
>>> It does not output byte code that only uses a single char array to
>>> compose the entire String in question.
>>> With "optimization fails", I also mean, there is used an additional
>>> "StringComposer" e.g. another StringBuilder or a StringJoiner in addition
>>> to the 1st StringBuilder.
>>>
>>> -Ulf
>>>
>>> Am 27.08.2014 um 14:02 schrieb Pavel Rappo:
>>>
 Could you please explain what you mean by "javac optimization fails"
 here?

 -Pavel

 On 27 Aug 2014, at 10:41, Ulf Zibis  wrote:

  4.) Now we see, that javac optimization fails again if StringBuilder,
> concatenation, toString(), append(String), append(Collection) etc. and
> StringJoiner use is mixed.
>

>>
>

Re: Impact of code difference in Collection#contains() worth improving?

[Martin Buchholz]

Just think - one whole byte saved for each individual change!
I have a webrev!
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/pico-optimize-contains/
https://bugs.openjdk.java.net/browse/JDK-8056951

Can haz review please?


On Fri, Aug 29, 2014 at 1:05 PM, Ulf Zibis  wrote:

>
> Am 28.08.2014 um 19:46 schrieb Vitaly Davidovich:
>
>
>> There's no register pressure - the immediate (I.e. -1) is encoded
>> directly into the instruction, just like 0 would be.  The time when 0 is
>> particularly useful is when you test for it in the zero bit of the flags
>> register (e.g. dec followed by jz, such as when counting a loop down to
>> 0).  Otherwise, I don't see any advantage from machine code perspective.
>>
>>
> Thanks for explaining this, but a very little nit: the immediate (I.e. -1)
> uses additional 32/64 bits in code which must be loaded from memory and
> wastes space in CPU cache or am I wrong? This could be saved with >= 0.
>
> So if unifying the code I agree to Martin's opinion.
>
> -Ulf
>
>  The aforementioned cmov instruction is not without its own downsides, so
>> it's unclear which is better when branch probability isn't known a priori.
>>
>> The 1 byte code is unlikely to make any difference, unless jit is turned
>> off and you're running this through a tight loop in the interpreter (but if
>> one does that, perf conversation is moot :)).
>>
>> Sent from my phone
>>
>> On Aug 28, 2014 1:28 PM, "Ulf Zibis" > ulf.zi...@cosoco.de>> wrote:
>>
>>
>> Am 27.08.2014 um 17:51 schrieb Martin Buchholz:
>>
>> The ArrayList version saves one byte of bytecode, and is
>> therefore very
>> slightly better.  We should bless that version and use it
>> consistently.
>>
>>
>> +1
>> Additional argument:
>> The LinkedList code requires to load 32/64-Bit -1 into CPU. This may
>> take some time on some
>> CPU and at least wastes memory footprint.
>> Additionally register pressure increases.
>> Vitaly, please correct me, if I'm wrong, just for learning more.
>>
>> Another advantage is that there is no problem if some implementation
>> of indexOf() erroneously
>> returns another negative value than -1. I remember some compare()
>> implementations, which
>> sometimes return different values than only -1, 0, +1.
>>
>> -Ulf
>>
>> ArrayList:
>>
>>  public boolean contains(Object o) {
>>  return indexOf(o) >= 0;
>>  }
>>
>> LinkedList:
>>
>>  public boolean contains(Object o) {
>>  return indexOf(o) != -1;
>>  }
>>
>>
>>
>

Re: Impact of code difference in Collection#contains() worth improving?

[Vitaly Davidovich]

Yes, the immediate is part of the instruction encoding, so takes up space.
 However, I believe the CMP instruction allows comparison of an immediate
with a wider register (e.g. cmp $-1, %eax), in which case the immediate
takes up 1 byte and is then sign extended to the size of the register.

But again, let's keep in mind that LinkedList is (likely) going to have
performance problems long before the difference being discussed here.  The
JIT compiler emits drastically different (and much more of it in my trials,
so instruction size difference in terms of a byte or two is irrelevant)
code when it's given profiling info (i.e. no -Xcomp flag), for both
ArrayList and LinkedList.

As mentioned before, if we were talking about some tight loop with nothing
but very basic/cheap instructions in there (e.g. integer arithmetic,
shifting, etc) and no memory chasing, then micro optimizations like this
may be interesting and lead to some perf gain.  But in the context of this
example, I don't think so.


On Fri, Aug 29, 2014 at 4:05 PM, Ulf Zibis  wrote:

>
> Am 28.08.2014 um 19:46 schrieb Vitaly Davidovich:
>
>
>> There's no register pressure - the immediate (I.e. -1) is encoded
>> directly into the instruction, just like 0 would be.  The time when 0 is
>> particularly useful is when you test for it in the zero bit of the flags
>> register (e.g. dec followed by jz, such as when counting a loop down to
>> 0).  Otherwise, I don't see any advantage from machine code perspective.
>>
>>
> Thanks for explaining this, but a very little nit: the immediate (I.e. -1)
> uses additional 32/64 bits in code which must be loaded from memory and
> wastes space in CPU cache or am I wrong? This could be saved with >= 0.
>
> So if unifying the code I agree to Martin's opinion.
>
> -Ulf
>
>  The aforementioned cmov instruction is not without its own downsides, so
>> it's unclear which is better when branch probability isn't known a priori.
>>
>> The 1 byte code is unlikely to make any difference, unless jit is turned
>> off and you're running this through a tight loop in the interpreter (but if
>> one does that, perf conversation is moot :)).
>>
>> Sent from my phone
>>
>> On Aug 28, 2014 1:28 PM, "Ulf Zibis" > ulf.zi...@cosoco.de>> wrote:
>>
>>
>> Am 27.08.2014 um 17:51 schrieb Martin Buchholz:
>>
>> The ArrayList version saves one byte of bytecode, and is
>> therefore very
>> slightly better.  We should bless that version and use it
>> consistently.
>>
>>
>> +1
>> Additional argument:
>> The LinkedList code requires to load 32/64-Bit -1 into CPU. This may
>> take some time on some
>> CPU and at least wastes memory footprint.
>> Additionally register pressure increases.
>> Vitaly, please correct me, if I'm wrong, just for learning more.
>>
>> Another advantage is that there is no problem if some implementation
>> of indexOf() erroneously
>> returns another negative value than -1. I remember some compare()
>> implementations, which
>> sometimes return different values than only -1, 0, +1.
>>
>> -Ulf
>>
>> ArrayList:
>>
>>  public boolean contains(Object o) {
>>  return indexOf(o) >= 0;
>>  }
>>
>> LinkedList:
>>
>>  public boolean contains(Object o) {
>>  return indexOf(o) != -1;
>>  }
>>
>>
>>
>

Optimization 2.0 for composing strings - Was: Replace concat String to append in StringBuilder parameters

[Ulf Zibis]

Hi compiler people,

is there some chance that javac could be enhanced to optimize better as discussed in this thread? 
Than refactoring of up to now better readable code to ugly StringBuilder@append() code would be 
superfluous.
I really like the String concatenation syntax, but unfortunately it often causes slower code and 
bigger footprint.
Optimally javac would optimize mixed use of StringBuilder, StringJoiner, concatenation, toString(), 
append(String), append(Collection) etc. to a single StringBuilder instance, so that the resulting 
code, JITed or not, will have better performance.

Additionally javac could guess a reasonable initial capacity from the given 
source code.


Am 29.08.2014 um 10:01 schrieb Wang Weijun:

So it's not that the optimization fails but there is no optimization on them 
yet.

I do see the .append("x") case will be easy to deal with, but it looks like 
historically javac has not been a place to do many optimizations. It mostly converts the 
java source to byte codes in a 1-to-1 mapping and let VM do whatever it wants (to 
optimize). When you talked about compiling multiple concatenation into using a single 
StringBuilder, it's more like choosing the correct implementation rather than an 
optimization.

I don't expect to see big change on this in the near future, so shall we go on 
with the current enhancement?

Thanks
Max

On Aug 29, 2014, at 2:17, Ulf Zibis  wrote:


I mean:
It does not output byte code that only uses a single char array to compose the 
entire String in question.
With "optimization fails", I also mean, there is used an additional 
"StringComposer" e.g. another StringBuilder or a StringJoiner in addition to the 1st 
StringBuilder.

-Ulf

Am 27.08.2014 um 14:02 schrieb Pavel Rappo:

Could you please explain what you mean by "javac optimization fails" here?

-Pavel

On 27 Aug 2014, at 10:41, Ulf Zibis  wrote:


4.) Now we see, that javac optimization fails again if StringBuilder, 
concatenation, toString(), append(String), append(Collection) etc. and 
StringJoiner use is mixed.

Re: RFR 8056249 Improve CompletableFuture resource usage

[Chris Hegarty]

> On 29 Aug 2014, at 16:56, Martin Buchholz  wrote:
> 
> Looks fine.

+1

-Chris.

> Instead of using Contributed-by: for Doug's work, you should make him the
> hg "user", as is done in previous changesets.  E.g. hg import has a --user
> flag.
> 
> All of jsr166 CVS src/main is ready for sync'ing with openjdk9, i.e. there
> are no known bugs, even though some future improvements are on the TODO
> list.
> 
> 
>> On Fri, Aug 29, 2014 at 3:01 AM, Paul Sandoz  wrote:
>> 
>> Hi,
>> 
>> Please review fixes by Doug to j.u.c.CompletableFuture to better control
>> resources for long completion chains (e.g. avoiding stack overflows). This
>> fix resulted in a lot of internal refactoring and clean up.  There are also
>> some doc clarifications for certain j.u.c.CompletationStage exception
>> handling methods (which most likely means a CCC is required).
>> 
>>  https://bugs.openjdk.java.net/browse/JDK-8056249
>> 
>> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8056249-cf-resource-usage/webrev/
>> 
>> 
>> --
>> 
>> Is the following snippet missing from the doc updates to the methods
>> CompletationStage.handle and handleAsync?
>> 
>>  If the supplied function itself encounters an
>>  exception, then the returned stage exceptionally completes with this
>>  exception unless this stage also completed exceptionally.
>> 
>> Paul.
>> 

Re: Impact of code difference in Collection#contains() worth improving?

[Ulf Zibis]

Am 28.08.2014 um 19:46 schrieb Vitaly Davidovich:


There's no register pressure - the immediate (I.e. -1) is encoded directly into the instruction, 
just like 0 would be.  The time when 0 is particularly useful is when you test for it in the zero 
bit of the flags register (e.g. dec followed by jz, such as when counting a loop down to 0).  
Otherwise, I don't see any advantage from machine code perspective.




Thanks for explaining this, but a very little nit: the immediate (I.e. -1) uses additional 32/64 
bits in code which must be loaded from memory and wastes space in CPU cache or am I wrong? This 
could be saved with >= 0.


So if unifying the code I agree to Martin's opinion.

-Ulf

The aforementioned cmov instruction is not without its own downsides, so it's unclear which is 
better when branch probability isn't known a priori.


The 1 byte code is unlikely to make any difference, unless jit is turned off and you're running 
this through a tight loop in the interpreter (but if one does that, perf conversation is moot :)).


Sent from my phone

On Aug 28, 2014 1:28 PM, "Ulf Zibis" mailto:ulf.zi...@cosoco.de>> wrote:


Am 27.08.2014 um 17:51 schrieb Martin Buchholz:

The ArrayList version saves one byte of bytecode, and is therefore very
slightly better.  We should bless that version and use it consistently.


+1
Additional argument:
The LinkedList code requires to load 32/64-Bit -1 into CPU. This may take 
some time on some
CPU and at least wastes memory footprint.
Additionally register pressure increases.
Vitaly, please correct me, if I'm wrong, just for learning more.

Another advantage is that there is no problem if some implementation of 
indexOf() erroneously
returns another negative value than -1. I remember some compare() 
implementations, which
sometimes return different values than only -1, 0, +1.

-Ulf

ArrayList:

 public boolean contains(Object o) {
 return indexOf(o) >= 0;
 }

LinkedList:

 public boolean contains(Object o) {
 return indexOf(o) != -1;
 }

Re: Impact of code difference in Collection#contains() worth improving?

[Ulf Zibis]

Am 28.08.2014 um 19:30 schrieb Louis Wasserman:
Comparator is spec'd to be allowed to return any number, positive, negative, or zero, but indexOf 
is specifically spec'd to return -1.


Yes, I know. I wanted to say, that from this knowing some developer might assume the same for 
indexOf when successfully using ArrayList@contains() with other negative value than -1.



If an indexOf method returns a negative value other than -1, that is a bug;


Yes, but how you suggest to deal with such a bug in existing code. If we would unify JDK code to 
LinkedList style, it would break such existing code. That's what I wanted to point on.


Does the original thread opener want to open a bug e.g. "Unify code of 
contains()"?
If not, the discussion doesn't make sense anymore.

-Ulf





On Thu, Aug 28, 2014 at 10:27 AM, Ulf Zibis mailto:ulf.zi...@cosoco.de>> wrote:


Am 27.08.2014 um 17:51 schrieb Martin Buchholz:

The ArrayList version saves one byte of bytecode, and is therefore very
slightly better.  We should bless that version and use it consistently.


+1
Additional argument:
The LinkedList code requires to load 32/64-Bit -1 into CPU. This may take 
some time on some
CPU and at least wastes memory footprint.
Additionally register pressure increases.
Vitaly, please correct me, if I'm wrong, just for learning more.

Another advantage is that there is no problem if some implementation of 
indexOf() erroneously
returns another negative value than -1. I remember some compare() 
implementations, which
sometimes return different values than only -1, 0, +1.

-Ulf


ArrayList:

 public boolean contains(Object o) {
 return indexOf(o) >= 0;
 }

LinkedList:

 public boolean contains(Object o) {
 return indexOf(o) != -1;
 }





--
Louis Wasserman

RFR: JDK-8056934: ZipInputStream does not correctly handle local header data descriptors with the optional signature missing

[Martin Buchholz]

Hi Xueming and Alan,

I'd like you to do a code review.

https://bugs.openjdk.java.net/browse/JDK-8056934
http://cr.openjdk.java.net/~martin/webrevs/openjdk9/zip-DataDescriptorSignatureMissing/

This seems like an atypical off-by-one, so I'm not sure how it happened,
and I have the nagging feeling I'm missing something.

Greg, this java code review contains a python program!

Re: RFR 8037819: Xerces Update: jaxp/validation/XMLSchemaFactory

[huizhe wang]

Thanks Lance. A TCK test I started when I saw your comment is just 
completed without any relevant failure.


Have a good weekend!

Best,
Joe

On 8/29/2014 9:47 AM, Lance Andersen wrote:

Hi Joe,

This looks OK.   I assume all TCK/JCK tests have been run and still 
pass as an additional sanity check ?


Have a good weekend.

Best,
Lance
On Aug 26, 2014, at 1:41 PM, huizhe wang > wrote:



Hi,

Please review update to XMLSchemaFactory along with changes to 
relevant classes. All new tests passed, as well as existing jaxp 
tests and JPRT.


Note that this updates XMLSchemaFactory, related classes such as 
XMLShemaValidator are only updated to the related revisions. Full 
update of XMLSchemaValidator and XMLSchemaLoader will be done separately.


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



Thanks,
Joe




Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 

Re: Review request for JDK-8051561: Convert JAXP function tests: javax.xml.xpath.* to jtreg (testNG) tests

[huizhe wang]

Hi Tristan,

Looks good. I left notes in the bug's comment section as a record and 
status of the original test development.


Thanks,
Joe

On 8/29/2014 9:50 AM, Tristan Yan wrote:

Hi Joe, Alan and others
I took over Eric’s last work and did some refactor for his code. 
Please help to review the code change again.
webrev: _http://cr.openjdk.java.net/~tyan/JDK-8051561/webrev.01/ 
_

bug: https://bugs.openjdk.java.net/browse/JDK-8051561

These code has been run with security manager and without security 
manager both and all passed.

Thank you
Tristan

On Jul 25, 2014, at 6:12 AM, Eric Wang > wrote:


Hi Joe, alan and every one

I'm working on jaxp functional test colocation which is traced by the 
bug JDK-8043091 .
We have finished to convert a few suite and the jaxp/xpath tracked by 
bug JDK-8051561  is 
the first one chosen for public review.


Can you please review the webrev below? your comments given would be 
helpful for our future work.

http://cr.openjdk.java.net/~ewang/JDK-8051561/webrev.00/

Thanks,
Eric

[9] RFR (S): 8056926: Improve caching of GuardWithTest combinator

[Vladimir Ivanov]

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

Cache GuardWithTest per erased to basic types signature.

GWT shape is made friendly to sharing:
  * GWT MH is implemented as BMH which stores 3 method handles
  * LF loads them from the associated MethodHandle

Testing: jdk/java/lang/invoke, jdk/java/util/streams, nashorn, octane w/ 
"-ea -esa" and COMPILE_THRESHOLD={0,30}.


Reviewed-by: vlivanov, ?
Contributed-by: john.r.r...@oracle.com

Thanks!

Best regards,
Vladimir Ivanov

Re: Review request for JDK-8051561: Convert JAXP function tests: javax.xml.xpath.* to jtreg (testNG) tests

[Tristan Yan]

Hi Joe, Alan and others
I took over Eric’s last work and did some refactor for his code. Please help to 
review the code change again. 
webrev: http://cr.openjdk.java.net/~tyan/JDK-8051561/webrev.01/ 

bug: https://bugs.openjdk.java.net/browse/JDK-8051561

These code has been run with security manager and without security manager both 
and all passed.
Thank you
Tristan

> On Jul 25, 2014, at 6:12 AM, Eric Wang  wrote:
> 
> Hi Joe, alan and every one
> 
> I'm working on jaxp functional test colocation which is traced by the bug 
> JDK-8043091 .
> We have finished to convert a few suite and the jaxp/xpath tracked by bug 
> JDK-8051561  is the first 
> one chosen for public review.
> 
> Can you please review the webrev below? your comments given would be helpful 
> for our future work.
> http://cr.openjdk.java.net/~ewang/JDK-8051561/webrev.00/ 
> 
> 
> Thanks,
> Eric
> 

Re: RFR 8037819: Xerces Update: jaxp/validation/XMLSchemaFactory

[Lance Andersen]

Hi Joe,

This looks OK.   I assume all TCK/JCK tests have been run and still pass as an 
additional sanity check ?

Have a good weekend.

Best,
Lance
On Aug 26, 2014, at 1:41 PM, huizhe wang  wrote:

> Hi,
> 
> Please review update to XMLSchemaFactory along with changes to relevant 
> classes. All new tests passed, as well as existing jaxp tests and JPRT.
> 
> Note that this updates XMLSchemaFactory, related classes such as 
> XMLShemaValidator are only updated to the related revisions. Full update of 
> XMLSchemaValidator and XMLSchemaLoader will be done separately.
> 
> http://cr.openjdk.java.net/~joehw/jdk9/8037819/webrev/
> 
> Thanks,
> Joe



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

Re: RFR 8056248: Improve ForkJoin thread throttling

[Martin Buchholz]

Approved.  Thanks for doing this.


On Fri, Aug 29, 2014 at 3:43 AM, Paul Sandoz  wrote:

> Hi,
>
> Please review fixes by Doug to For/Join to improve thread throttling (e.g.
> for nested parallel streams). This fix resulted in a lot of internal
> refactoring and clean up.
>
> In addition a new system property was added
> "java.util.concurrent.ForkJoinPool.common.maximumSpares" (which i suspect
> will only very rarely be used), so a CCC will be required:
>
>  http://cs.oswego.edu/pipermail/concurrency-interest/2014-July/012838.html
>
>  * {@code java.util.concurrent.ForkJoinPool.common.maximumSpares}
>  * - the maximum number of allowed extra threads to maintain target
>  * parallelism (default 256).
>
>   https://bugs.openjdk.java.net/browse/JDK-8056248
>
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8056248-fj-thread-throttling/webrev/
>
>
> --
>
> Once this review is complete i think we should also backport to 8u40, and
> likewise for the CompletableFuture updates (which i forgot to mention in a
> previously related email).
>
> Both these updates have been soaking in the 166 repo for a month or two.
> No related test failures were reported for a JPRT run with both patches.
>
> Paul.
>

Re: RFR 8056249 Improve CompletableFuture resource usage

[Martin Buchholz]

Looks fine.
Instead of using Contributed-by: for Doug's work, you should make him the
hg "user", as is done in previous changesets.  E.g. hg import has a --user
flag.

All of jsr166 CVS src/main is ready for sync'ing with openjdk9, i.e. there
are no known bugs, even though some future improvements are on the TODO
list.


On Fri, Aug 29, 2014 at 3:01 AM, Paul Sandoz  wrote:

> Hi,
>
> Please review fixes by Doug to j.u.c.CompletableFuture to better control
> resources for long completion chains (e.g. avoiding stack overflows). This
> fix resulted in a lot of internal refactoring and clean up.  There are also
> some doc clarifications for certain j.u.c.CompletationStage exception
> handling methods (which most likely means a CCC is required).
>
>   https://bugs.openjdk.java.net/browse/JDK-8056249
>
> http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8056249-cf-resource-usage/webrev/
>
>
> --
>
> Is the following snippet missing from the doc updates to the methods
> CompletationStage.handle and handleAsync?
>
>   If the supplied function itself encounters an
>   exception, then the returned stage exceptionally completes with this
>   exception unless this stage also completed exceptionally.
>
> Paul.
>

Re: RFR: 5043030 (reflect) unnecessary object creation in reflection

[Andrej Golovnin]

Hi Joel,

>> I have changed the test TestMethodReflectValueOf as you suggested and I have 
>> changed the summary of both tests too. You will find the changes in the 
>> attached patch. Here is the new webrev: https://db.tt/wQBLVceA
>> 
>> And here is the coverage report in the HTML format:
>> 
>> https://db.tt/JTZjpnMM
>> 
> 
> Out of curiosity, did you generate the coverage report running the jdk_lang 
> test suite?

Yes and I used the following config for JCov:

include_list=jcov_jdk_lang.txt,field=on,abstract=on,native=on,type=all,file=$JCOV_OUT,merge=merge

where jcov_jdk_lang.txt contains the single line:

sun.reflect.*


> I think this patch is good to go. I need to file some Oracle internal 
> requests, should take about a week, then I can sponsor this.

I am very pleased to hear that and I hope to contribute more.

Best regards,
Andrej Golovnin

Re: DMH to fields, casts and type profiling Re: [9] RFR (M): 8037209: Improvements and cleanups to bytecode assembly for lambda forms

[Paul Sandoz]

On Aug 29, 2014, at 12:45 AM, John Rose  wrote:

> On Aug 28, 2014, at 7:38 AM, Paul Sandoz  wrote:
> 
>> On Jul 8, 2014, at 9:09 PM, John Rose  wrote:
>> 
>>> Regarding the extra cast in accessor logic that Paul picked up on:  That 
>>> may be a left-over from obsolete versions of the code, or it may cover for 
>>> some corner cases, or it could possibly be a re-assurance to the JIT.
>>> 
>> 
>> I had some enlightening discussions with Roland on this.
>> 
>> It seems quite tricky to solve in general the removal of the null check due 
>> to the aggressive nature in which the null branch is reduce to a trap, but 
>> IIUC may be possible to turn Class.cast into an intrinsic to handle the 
>> specific case (although that seems costly).
>> 
>> I was labouring under the misapprehension that an explicit Class.cast was a 
>> profiling point but now i realize it's only certain byte codes (like 
>> checkcast/invokehandle). Nothing specific to the DHM access logic showed up 
>> with regards to type profiling when analysing the MethodData output from 
>> some simple examples [*]. Therefore i presume it's more likely to be the 
>> first or third reason you state.
>> 
>> So i propose to proceed with the experiment with a patch to replace the 
>> casts with asserts in the accessor logic and run that through the usual 
>> tests.
>> 
>> Paul.
>> 
>> 
>> [*] Also i have so far failed to concoct a simple example for VarHandles 
>> where i can trigger profile pollution and failed inlining
> 
> Here's something to try first:  Force a profile point before Class.cast, even 
> without Roland's enhancements.
> You should be able to type-profile "x" by inserting "push x; checkcast 
> java/lang/Object; pop".
> See last line of https://wiki.openjdk.java.net/display/HotSpot/MethodData
> 

Thanks, that's a neat trick. I will play around with that.

Some thoughts triggered (now i am a supposedly little wiser... perhaps :-)).

I could imagine things would get polluted fairly quickly within the compiled & 
shared DHM LFs for field access (same for VHs), plus cast-wise only the value 
is operated on and not the receiver. In general presumably what matters most is 
the type profile from the call site that would flow down to the LF?

But... what would there be in compiled DHM LFs for field access that they would 
require profiling so that generated code would be different for accessing a 
field of Bar rather than a field of Foo? since it all boils down to Unsafe 
calls, or are there some subtle details hidden within the Unsafe intrinsics? or 
perhaps a lack of that can result in some odd effects?

Paul.

Re: A List from Iterator

[Rémi Forax]

ArrayList list = ...
iterator.forEachRemaining(list::add);

Rémi

On 29 août 2014 00:22:11 UTC+02:00, Pavel Rappo  wrote:
>> If we were to provide a Collections util it would to wrap Enumeration
>as an Iterator but that's it.
>
>Mike, that's what I'm talking about. It's just for convenience.
>
>-Pavel

-- 
Envoyé de mon téléphone Android avec K-9 Mail. Excusez la brièveté.

Re: RFR 8056249 Improve CompletableFuture resource usage

[Paul Sandoz]

On Aug 29, 2014, at 3:38 PM, Chris Hegarty  wrote:

> 
> On 29/08/14 11:01, Paul Sandoz wrote:
>> Hi,
>> 
>> Please review fixes by Doug to j.u.c.CompletableFuture to better control 
>> resources for long completion chains (e.g. avoiding stack overflows). This 
>> fix resulted in a lot of internal refactoring and clean up.  There are also 
>> some doc clarifications for certain j.u.c.CompletationStage exception 
>> handling methods (which most likely means a CCC is required).
> 
> IMHO, the rewording and clarifications in the docs do not warrant a CCC.
> 

Agreed it's just a clarification.

Paul.

Re: RFR 8056249 Improve CompletableFuture resource usage

[Paul Sandoz]

On Aug 29, 2014, at 3:35 PM, Doug Lea  wrote:
> 
>> Is the following snippet missing from the doc updates to the methods 
>> CompletationStage.handle and handleAsync?
> 
> While touching up wording, we noticed that this sentence doesn't capture
> all the cases that are described in the top-level CompletionStage specs.
> So including it seems more confusing than omitting it, and just relying
> on the overall specs.
> 

I got it now, on first look i missed the subtleties of the exception handling.

Paul.

Re: RFR 8056249 Improve CompletableFuture resource usage

[Chris Hegarty]

On 29/08/14 11:01, Paul Sandoz wrote:

Hi,

Please review fixes by Doug to j.u.c.CompletableFuture to better control 
resources for long completion chains (e.g. avoiding stack overflows). This fix 
resulted in a lot of internal refactoring and clean up.  There are also some 
doc clarifications for certain j.u.c.CompletationStage exception handling 
methods (which most likely means a CCC is required).


IMHO, the rewording and clarifications in the docs do not warrant a CCC.

From what I can see, these cases are already covered by JCK tests. In 
the case of handle() then the returned CF mush complete with the 
function's result, given the method signature. This is just the docs 
explicitly stating what is already a given.



   https://bugs.openjdk.java.net/browse/JDK-8056249
   
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8056249-cf-resource-usage/webrev/


--

Is the following snippet missing from the doc updates to the methods 
CompletationStage.handle and handleAsync?

   If the supplied function itself encounters an
   exception, then the returned stage exceptionally completes with this
   exception unless this stage also completed exceptionally.


I don't think so.  Whether, or not, this CF completes exceptionally will 
have no impact of the result of the returned CF.


You could add all of what you suggested less, "unless this stage also 
completed exceptionally", but I don't think that is even necessary. I 
think it is already covered by "the function's result is used to 
complete the returned stage", function's result could be an exception. 
But I guess you could be explicit.


-Chris.



Paul.

Re: RFR 8056249 Improve CompletableFuture resource usage

[Doug Lea]

On 08/29/2014 06:01 AM, Paul Sandoz wrote:

Hi,

Please review fixes by Doug to j.u.c.CompletableFuture to better control 
resources for long completion chains (e.g. avoiding stack overflows). This fix 
resulted in a lot of internal refactoring and clean up.  There are also some 
doc clarifications for certain j.u.c.CompletationStage exception handling 
methods (which most likely means a CCC is required).

   https://bugs.openjdk.java.net/browse/JDK-8056249
   
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8056249-cf-resource-usage/webrev/



Thanks for doing this!



Is the following snippet missing from the doc updates to the methods 
CompletationStage.handle and handleAsync?


While touching up wording, we noticed that this sentence doesn't capture
all the cases that are described in the top-level CompletionStage specs.
So including it seems more confusing than omitting it, and just relying
on the overall specs.

-Doug



   If the supplied function itself encounters an
   exception, then the returned stage exceptionally completes with this
   exception unless this stage also completed exceptionally.

Paul.

Re: RFR: 5043030 (reflect) unnecessary object creation in reflection

[Joel Borggrén-Franck]

Hi Andrej,

On 22 jun 2014, at 00:00, Andrej Golovnin  wrote:

> Hi Joel,
> 
> sorry for late response. I was too busy with other things.
> 

Likewise!

> I have changed the test TestMethodReflectValueOf as you suggested and I have 
> changed the summary of both tests too. You will find the changes in the 
> attached patch. Here is the new webrev: https://db.tt/wQBLVceA
> 
> And here is the coverage report in the HTML format:
> 
> https://db.tt/JTZjpnMM
> 

Out of curiosity, did you generate the coverage report running the jdk_lang 
test suite?

I think this patch is good to go. I need to file some Oracle internal requests, 
should take about a week, then I can sponsor this.

Thanks for the contribution!

cheers
/Joel

Re: RFR: [8056310] Cleanup in WinNTFileSystem_md.c

[Alan Bateman]

On 28/08/2014 23:28, Ivan Gerasimov wrote:

Hello!

This is a proposal to make some minor cleanup in WinNTFileSystem_md.c:
- fix a typo in error message,

Looks okay.

- use wide char constant for consistency,

Looks okay.

- avoid an array reallocation for power-of-two sizes,
It's a bit of corner case but should okay. As some point we should just 
get rid of this code completely and re-implement the list method to use 
the new file system API.


- avoid a very unlikely memory leak (when frompath != NULL && topath 
== NULL).
I checked the MSDN docs and free(NULL) is a no-op as it is elsewhere so 
I think this is okay. Clearly if either is NULL then memory is 
completely exhausted and the VM will likely crash/terminate anyway.


-Alan.

RFR 8056248: Improve ForkJoin thread throttling

[Paul Sandoz]

Hi,

Please review fixes by Doug to For/Join to improve thread throttling (e.g. for 
nested parallel streams). This fix resulted in a lot of internal refactoring 
and clean up.  

In addition a new system property was added 
"java.util.concurrent.ForkJoinPool.common.maximumSpares" (which i suspect will 
only very rarely be used), so a CCC will be required:

 http://cs.oswego.edu/pipermail/concurrency-interest/2014-July/012838.html

 * {@code java.util.concurrent.ForkJoinPool.common.maximumSpares}
 * - the maximum number of allowed extra threads to maintain target
 * parallelism (default 256).

  https://bugs.openjdk.java.net/browse/JDK-8056248
  
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8056248-fj-thread-throttling/webrev/


--

Once this review is complete i think we should also backport to 8u40, and 
likewise for the CompletableFuture updates (which i forgot to mention in a 
previously related email). 

Both these updates have been soaking in the 166 repo for a month or two. No 
related test failures were reported for a JPRT run with both patches.

Paul.

RFR 8056249 Improve CompletableFuture resource usage

[Paul Sandoz]

Hi,

Please review fixes by Doug to j.u.c.CompletableFuture to better control 
resources for long completion chains (e.g. avoiding stack overflows). This fix 
resulted in a lot of internal refactoring and clean up.  There are also some 
doc clarifications for certain j.u.c.CompletationStage exception handling 
methods (which most likely means a CCC is required).

  https://bugs.openjdk.java.net/browse/JDK-8056249
  http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8056249-cf-resource-usage/webrev/


--

Is the following snippet missing from the doc updates to the methods 
CompletationStage.handle and handleAsync?

  If the supplied function itself encounters an
  exception, then the returned stage exceptionally completes with this
  exception unless this stage also completed exceptionally.

Paul.

Re: Review request for JDK-8051540: Convert JAXP functin tests: org.xml.sax to jtreg (testNG) tests

[Alan Bateman]

On 28/08/2014 08:31, Michael Kay wrote:

I haven't yet had a chance to look at the JAXP tests, but this kind of message suggests 
to me that they don't separate what is in JAXP (the interface) from what is in Xalan and 
Xerces (the implementations).Would that be a correct assumption?  The 
"redirect" extension is not a JAXP feature, it is a Xalan feature.

I've had the impression for some years that the management of JAXP as an 
interface is far too closely tied up with the management of the JAXP 
implementations within the JDK.

Just to add to Joe's comments then it is important to have a 
comprehensive set of tests that fully exercise the implementation and 
any JDK-specific implementation features. Clearly conformance tests for 
the JAXP API should only have tests that are developed based on the 
specification but that is the role of the conformance suite/JCK (which 
is not in OpenJDK).


-Alan.

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

So it's not that the optimization fails but there is no optimization on them 
yet.

I do see the .append("x") case will be easy to deal with, but it looks like 
historically javac has not been a place to do many optimizations. It mostly 
converts the java source to byte codes in a 1-to-1 mapping and let VM do 
whatever it wants (to optimize). When you talked about compiling multiple 
concatenation into using a single StringBuilder, it's more like choosing the 
correct implementation rather than an optimization.

I don't expect to see big change on this in the near future, so shall we go on 
with the current enhancement?

Thanks
Max

On Aug 29, 2014, at 2:17, Ulf Zibis  wrote:

> I mean:
> It does not output byte code that only uses a single char array to compose 
> the entire String in question.
> With "optimization fails", I also mean, there is used an additional 
> "StringComposer" e.g. another StringBuilder or a StringJoiner in addition to 
> the 1st StringBuilder.
> 
> -Ulf
> 
> Am 27.08.2014 um 14:02 schrieb Pavel Rappo:
>> Could you please explain what you mean by "javac optimization fails" here?
>> 
>> -Pavel
>> 
>> On 27 Aug 2014, at 10:41, Ulf Zibis  wrote:
>> 
>>> 4.) Now we see, that javac optimization fails again if StringBuilder, 
>>> concatenation, toString(), append(String), append(Collection) etc. and 
>>> StringJoiner use is mixed.
>> 
> 

Re: Review request: 8055856: checkdeps build target doesn't work for cross-compilation builds

[Erik Joelsson]

Hello Mandy,

Looks good. Just noticed another thing in CheckModules.gmk. You can 
remove the explicit FIXPATH as the variable JAVA already contains it. In 
general, only spec.gmk should need to worry about FIXPATH.


If there are no other changes, I don't need to see another round before 
you push.


/Erik

On 2014-08-28 19:21, Mandy Chung wrote:

On 8/27/14 11:38 PM, Erik Joelsson wrote:

Hello Mandy,

That certainly looks better. A couple of more thoughts, and sorry for 
not pointing this out earlier, but the new structure is still new to 
me too.


* The rmic targets also generate classes, so for modules.xml to be 
correct, I suspect you need to depend on that too. Simply add "rmic" 
after java on the dependency line. I assume the verification doesn't 
care about resources? If it does, then you would also need to depend 
on the rest of gendata, something like $(filter-out jdk.dev-gendata, 
$(GENDATA_TARGETS)).


Good catch.  rmic needs to be added in the dependency.  jdeps verifies 
class files only and doesn't care about resources.




* In Gendata-jdk.dev.gmk, there is an ifndef OPENJDK. We are trying 
to move away from that construct when possible. It's a bit cumbersome 
but to avoid it. To do it in the current model, create a closed 
version of Gendata-jdk.dev.gmk. Add "$(eval $(call 
IncludeCustomExtension, jdk, gendata/Gendata-jdk.dev.gmk))" after 
"include GendataCommon.gmk". Change the first assignment of 
METADATA_FILES to += and move the closed addition to the closed 
version of the file. There is no need for ifndef OPENJDK in the 
closed file. It only gets included when we build closed.


That's another good change in the build.

Updated webrev:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055856/webrev.02/

I also moved jdk/make/CheckModules.gmk to top/make/CheckModules.gmk 
per Magnus's suggestion.


Mandy



/Erik

On 2014-08-27 18:00, Mandy Chung wrote:

Erik, Magnus,

This is much easier than I have thought.  I really like this new build.
I have separated out Gendata-jdk.dev.gmk and removed the modules-xml
target completely.

Webrev at:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8055856/webrev.01/

Mandy