Re: RFR: 8019799: api/java_util/jar/Pack200 test failed with compactX profiles. (EZ)

[David Holmes]

On 10/07/2013 6:25 AM, Kumar Srinivasan wrote:

Hi,

We had earlier added a fail over to the java implementation, for reference:
http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/1432a6247ac9

The first time the unpacker is called  an UnsatisfiedLinkError will be
thrown, but
subsequent invocations in the same JDK/VM instance, a NoClassDefFoundError
will be thrown. This error also needs to be caught and handled.


Ugghhh. Presumably we need a test update to catch this?


Here is the modified webrev:
http://cr.openjdk.java.net/~ksrini/8019799/webrev.0/


Looks okay - but no test.

It would be better/more-efficient if the presence of the native 
implementation could be detected once during class initialization. But I 
won't try to force that on you. :)


Thanks,
David


Thanks
Kumar

8016036: RMI specification needs to be updated to allow Activation on remote hosts

[Stephen Flores]

Bob, Sean,

Please review this webrev

  http://cr.openjdk.java.net/~sflores/8016036/webrev.00/

for RFE/CCC: 8016036 RMI specification needs to be updated to allow 
Activation on remote hosts


A DESCRIPTION OF THE PROBLEM:

  A compact2 (client profile) JRE includes
  java.rmi.activation.ActivationSystem
  but not rmid and some mobile platforms cannot run rmid, this
  causes a problem when running the JCK, since it and rmid assume
  activation requests to be from the same platform.

Evaluation:

  To avoid creating a special RMI test server in a closed workspace,
  rmid should be changed allow the tester specify which remote client
  can access activation interface. This will allow the tester to run
  rmid on a server running the full JRE instead of the compact2 client.

  To avoid changing the JCK, the JRE should be changed so that the
  tester can specify what the default activation host is.

Description of fix:

  Change rmid to allow activation requests from a remote client
  specified in a system property instead of the local host.

  Change java.rmi.activation.ActivationGroup.getSystem to use
  a system property for the host (with a default of empty) when
  looking up the activation system.

Thanks,

  Steve.

hg: jdk8/tl/jdk: 8020318: Fix doclint issues in java.net

[jason . uh]

Changeset: 702556f7977e
Author:juh
Date:  2013-07-10 18:01 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/702556f7977e

8020318: Fix doclint issues in java.net
Reviewed-by: darcy, khazra

! src/share/classes/java/net/CookieStore.java
! src/share/classes/java/net/HttpURLPermission.java
! src/share/classes/java/net/Inet4Address.java
! src/share/classes/java/net/Inet6Address.java
! src/share/classes/java/net/InetAddress.java
! src/share/classes/java/net/ProtocolFamily.java
! src/share/classes/java/net/SocketOption.java
! src/share/classes/java/net/URI.java

Re: class SplittableRandom

[Doug Lea]

On 07/10/13 16:49, Aleksey Shipilev wrote:

(juggling an Occam's razor)


Thanks keep it sharp; the more skepticism the better.



D'oh, of course it is ludicrously faster than j.u.Random. I am not convinced
it beats TLR though, given TLR is super-optimized now in JDK 8. Are you
really implying you can beat < 10 cycles? I will check that once I get back
in office in about 12 hours.


Not "beats TLR". But "beats TLR in the context of parallel streams".
Which it currently does, in part because of the custom Stream
generator support (those ints(...), longs(...), doubles(...) methods)
that allow you to express things in a more efficient way.

As the primary author of TLR, and tied for the most performance-obsessed
person on this list, I'm reasonably aware of its strengths and weaknesses.
I'm convinced that SplittableRandom is the best shot we have of
addressing the weaknesses.


Also, I am not convinced passing DieHarder on some split sequence (was it
really the non-empty sequence?) extends to arbitrary splits.


It's been fully DieHarder-tested for each of the first few hundred splits.
The full tech-paper write-up is still being put together,
so at this point I can't link to tables etc, but I think you can
rely on your common sense that people putting together an algorithm
with this as its main contribution would actually test it :-)

-Doug

Re: RFR: 8017513: Support for closeable streams

[Zhong Yu]

A few things that require clarification:

If we have

interface A extends B, C
interface B extends MayHoldCloseableResource
interface C extends AutoCloseable

is A considered definitely holding resource?

If @HoldsResource is applied to a subtype of MayHoldCloseableResource

@HoldsResource
interface D extends B

What does it mean?

If it mean that D definitely holds resource, is @HoldResource still
necessary on  methods that return D?


Zhong Yu



On Wed, Jul 10, 2013 at 4:30 PM, Henry Jen  wrote:
> Hi,
>
> Please review webrev at
> http://cr.openjdk.java.net/~henryjen/ccc/8017513/0/webrev/index.html
>
> This webrev improve resource release experience on Streams by
> eliminating CloseableStream, instead, fold close capability into Stream.
>
> A new interface, java.util.MayHoldCloseableResource, indicates an
> implementation may or may not hold a resource need to be closed.
> Annotation {@link HoldsResource} may be used to guide users/static
> analysis tools that a MHCR instance that definitely hold a Closeable
> resource.
>
> Cheers,
> Henry

RFR: 8017513: Support for closeable streams

[Henry Jen]

Hi,

Please review webrev at
http://cr.openjdk.java.net/~henryjen/ccc/8017513/0/webrev/index.html

This webrev improve resource release experience on Streams by
eliminating CloseableStream, instead, fold close capability into Stream.

A new interface, java.util.MayHoldCloseableResource, indicates an
implementation may or may not hold a resource need to be closed.
Annotation {@link HoldsResource} may be used to guide users/static
analysis tools that a MHCR instance that definitely hold a Closeable
resource.

Cheers,
Henry

hg: jdk8/tl/jdk: 8020308: Fix doclint issues in java.lang.management

[joe . darcy]

Changeset: 69d9fe8175a0
Author:sspitsyn
Date:  2013-07-10 14:20 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/69d9fe8175a0

8020308: Fix doclint issues in java.lang.management
Reviewed-by: darcy
Contributed-by: serguei.spit...@oracle.com

! src/share/classes/java/lang/management/LockInfo.java
! src/share/classes/java/lang/management/ManagementFactory.java
! src/share/classes/java/lang/management/MemoryMXBean.java
! src/share/classes/java/lang/management/MemoryNotificationInfo.java
! src/share/classes/java/lang/management/MemoryPoolMXBean.java
! src/share/classes/java/lang/management/MemoryUsage.java
! src/share/classes/java/lang/management/MonitorInfo.java
! src/share/classes/java/lang/management/RuntimeMXBean.java
! src/share/classes/java/lang/management/ThreadInfo.java
! src/share/classes/java/lang/management/ThreadMXBean.java

Re: class SplittableRandom

[Aleksey Shipilev]

(juggling an Occam's razor)

Hi Doug,

On 10.07.2013, at 23:13, Doug Lea  wrote:
> This post serves as a request for comment, with shorter than usual
> turnaround (a couple of days) before considering a request to
> integrate into OpenJDK 8. So, please take a look.

I've glanced over the paper this work is based on, and did a few quick sweeps 
on SR code. My first reaction is: while interesting, not entirely convincing.

> Here are answers to some likely questions:
> 
> Q: How much faster is it than java.util.Random?
> 
> A: In sequential usages, usually at least twice as fast for long and
> double methods; usually only slightly faster for int methods.  In
> parallel usages, SplittableRandom is almost arbitrarily faster. The
> very first simple parallel Stream program I wrote (to generate and sum
> nextLong()'s) ran 2900 times faster than the java.util.Random
> equivalent on a 32-way machine.

D'oh, of course it is ludicrously faster than j.u.Random. I am not convinced it 
beats TLR though, given TLR is super-optimized now in JDK 8. Are you really 
implying you can beat < 10 cycles? I will check that once I get back in office 
in about 12 hours.


> Q: When can/should I use it instead of java.util.Random?
> 
> A: Whenever you are not sharing one across Threads. Instances of
> SplittableRandom are not thread-safe. They are designed to be split,
> not shared, across threads.  When class SplittableRandom applies (or
> you can rework your program to make it apply), it is usually a better
> choice.  Not only is it usually faster, it also has better statistical
> independence and uniformity properties.

Being the same linear congruental generator with a splash of bit twisting, it 
does not immediately follow how's the statistical properties are better. If it 
is actually true, why don't we fix TLR with more robust generation scheme and 
gain the same properties there?

Also, I am not convinced passing DieHarder on some split sequence (was it 
really the non-empty sequence?) extends to arbitrary splits.

> 
> Q: When can/should I use it instead of java.util.concurrent.ThreadLocalRandom?
> 
> A: When you are doing structured fork/join computations, so you can
> explicitly split one rather than relying on the per-thread singleton
> instance.

And gain what vs. TLR? That the pseudo-random sequence in each subtask will be 
oblivious to task scheduling? This seems to be the distinguishing feature, but 
not really obvious from the docs.

-Aleksey

Re: RFR - Changes to address CCC 8014135: Support for statically linked agents

[BILL PITTORE]

On 7/3/2013 6:32 PM, Jeremy Manson wrote:
I know that this is mentioned in the JEP, but does it really make 
sense to have -agentpath work here, rather than just -agentlib?  I 
would think that specifying a full pathname and getting the library 
loaded out of the launcher would be a little surprising to people who 
are basically saying "please load this agent from the given path".


Also, in practice, we do something very similar at Google, and, when 
testing, I find it occasionally useful to be able to say "please load 
this agent on the command line via the agentpath I gave you, rather 
than loading the one with the same name I baked into the launcher".


(FWIW, when we did this internally at Google, we added a special -XX 
flag for when the agent is in the launcher.  I know that you don't 
want to go that way, though.)


Thanks for the comments.  I would say the thinking here is that we want 
this to be as transparent as possible to the end user. In our view of 
the end user is someone perhaps using an IDE and the actual execution of 
the VM with agent arguments is hidden. In your case it sounds like you 
are actually developing agents which is a whole different ball game. I 
could certainly see adding a -XX argument that forces agentpath to load 
the external library which would be good for agent development and 
debugging. No need to relink the entire VM with the new agent. Our tech 
lead is out on vacation this week so I'll bring this up when he's back.


bill



On Wed, Jul 3, 2013 at 2:17 PM, BILL PITTORE > wrote:


These changes address bug 8014135 which adds support for
statically linked agents in the VM. This is a followup to the
recent JNI spec changes that addressed statically linked JNI
libraries( 8005716).
The JEP for this change is the same JEP as the JNI changes:
http://openjdk.java.net/jeps/178

Webrevs are here:

http://cr.openjdk.java.net/~bpittore/8014135/jdk/webrev.00/

http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.00/


The changes to jvmti.xml can also be seen in the output file that
I placed here:
http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.00/jvmti.html



Thanks,
bill

hg: jdk8/tl/jdk: 2 new changesets

[rob . mckenna]

Changeset: 607fa1ff3de2
Author:bpb
Date:  2013-07-09 11:26 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/607fa1ff3de2

6178739: (fmt) Formatter.format("%0.4f\n", 56789.456789) generates 
MissingFormatWidthException
Summary: Change the field width specification to require a positive value. The 
exception is still thrown but that is now explicitly consistent with the 
specification.
Reviewed-by: darcy
Contributed-by: Brian Burkhalter 

! src/share/classes/java/util/Formatter.java

Changeset: 2ee772cda1d6
Author:bpb
Date:  2013-07-09 12:47 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2ee772cda1d6

6480539: BigDecimal.stripTrailingZeros() has no effect on zero itself ("0.0")
Summary: Make stripTrailingZeros() return BigDecimal.ZERO if the BigDecimal is 
numerically equal to zero.
Reviewed-by: darcy
Contributed-by: Brian Burkhalter 

! src/share/classes/java/math/BigDecimal.java
! test/java/math/BigDecimal/StrippingZerosTest.java

class SplittableRandom

[Doug Lea]

[Note: I'm also posting this on the concurrency-interest list.]


We expect that using random numbers in parallel Stream computations
will be common. (We know it is common in parallel computing in
general.) But we had left support for it in an unsatisfactory state.
If you want to create a stream of random numbers to drive a parallel
computation, you'd choose among two options, neither of them providing
what you probably want: (1) Use a stream based on a single shared
java.util.Random object, in which case your program will encounter
stunning slowdowns when run with many cores; or (2) Use a stream based
on ThreadLocalRandom, which avoids contention, but gives you no
control over the use or properties of the per-thread singleton Random
object. While the ThreadLocalRandom option is great for many purposes,
you wouldn't want to use it in, say, a high-quality Monte Carlo
simulation.

Enter Guy Steele. Guy has been working on an algorithm that addresses
exactly the substantial range of uses not otherwise supported: It is,
in essence, the Random number generator analog of a Spliterator. Class
SplittableRandom supports method split() that creates a sub-generator
that when used in parallel with the original, maintains its
statistical properties.

When Brian Goetz and I heard that this was nearing completion, we
entered drop-everything mode to explore whether it could be added now
in time for JDK8. We conclude that it should. We've been helping with
JDK-ifying the basic algorithm, integrating java.util.Stream support,
etc, to enable addition as class java.util.SplittableRandom.

Just to be on the cautious side though, we are for the moment treating
this in the same way we treat jsr166 candidates for potential
OpenJDK integration. The initial version is available at
http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/main/java/util/SplittableRandom.java?view=log
With API docs at:
http://gee.cs.oswego.edu/dl/jsr166/dist/docs/java/util/SplittableRandom.html

This post serves as a request for comment, with shorter than usual
turnaround (a couple of days) before considering a request to
integrate into OpenJDK 8. So, please take a look.

Here are answers to some likely questions:

Q: How much faster is it than java.util.Random?

A: In sequential usages, usually at least twice as fast for long and
double methods; usually only slightly faster for int methods.  In
parallel usages, SplittableRandom is almost arbitrarily faster. The
very first simple parallel Stream program I wrote (to generate and sum
nextLong()'s) ran 2900 times faster than the java.util.Random
equivalent on a 32-way machine.

Q: When can/should I use it instead of java.util.Random?

A: Whenever you are not sharing one across Threads. Instances of
SplittableRandom are not thread-safe. They are designed to be split,
not shared, across threads.  When class SplittableRandom applies (or
you can rework your program to make it apply), it is usually a better
choice.  Not only is it usually faster, it also has better statistical
independence and uniformity properties.

Q: When can/should I use it instead of java.util.concurrent.ThreadLocalRandom?

A: When you are doing structured fork/join computations, so you can
explicitly split one rather than relying on the per-thread singleton
instance.

Q: Why is this in java.util, not java.util.concurrent?

A: Because, like java.util.Spliterator, SplittableRandom is a tool for
arranging isolated parallel computations that don't entail any
concurrency control themselves.

Q: Why isn't SplittableRandom a subclass of Random?

A: Class Random requires thread-safety in its spec. It would be
nonsensical for SplittableRandom to comply.

Q: Why don't you at least come up with a new interface that
defines methods shared with java.util.Random?

A: We spent a couple of days exploring this. We think it could and
probably should be done, but not now. Method names and specs of
SplittableRandom are chosen to make it possible. But we encountered
enough short-term obstacles to conclude that this is an unwise move
for JDK8. Among the issues are that we'd need to adjust some specs and
possibly some code in java.util.Random, and that we are at a loss
about whether or how to generalize SplittableRandom's added Stream
methods. In the mean time, it would be more than acceptable for
SplittableRandom to be used primarily in new code (or new adaptions of
old code) that wouldn't need or want to be interoperable with code
using java.util.Random.

Q: Are we going to revisit with SplittableRandom all those memory
contention issues we saw with ThreadLocalRandom?

A: Most likely not. Most of the memory contention issues surrounding
ThreadLocalRandom arise because they are long-lived. SplittableRandoms
will tend to be short-lived. In any case, now that we have the tools
to cope (@Contended), we can evaluate and adjust if more detailed
empirical analysis warrants.

Re: Build error with GCC4.8 on Fedora19

[Omair Majid]

On 07/09/2013 03:40 AM, Erik Joelsson wrote:
> I would like to see a comment explaining why the option was needed. Is
> this a bug in gcc or has the checking just become better?

-Werror is not a great default. It means "all warnings are errors". The
set of warnings that a compiler emits changes all the time. Users and
distributions can enable additional warnings by default. Newer compiler
versions may omit additional warnings. So on any compiler other than the
typical compiler used, extra warnings are possible. With -Werror, this
will probably fail. Lots of people discourage it [1].

We have been setting SCTP_WERROR="" to disable this -Werror on Fedora
builds [2].

Cheers,
Omair

[1] https://blog.flameeyes.eu/2009/02/future-proof-your-code-dont-use-werror
[2]
http://pkgs.fedoraproject.org/cgit/java-1.8.0-openjdk.git/tree/java-1.8.0-openjdk.spec#n425
-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681

hg: jdk8/tl/jdk: 8020294: Fix doclint issues in java.util.Spliterator

[joe . darcy]

Changeset: 7c44ea602cc8
Author:darcy
Date:  2013-07-10 11:05 -0700
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7c44ea602cc8

8020294: Fix doclint issues in java.util.Spliterator
Reviewed-by: psandoz

! src/share/classes/java/util/Spliterator.java

Re: Code Review Request: More tests for 7184826: (reflect) Add support for Project Lambda concepts in core reflection

[Joel Borggren-Franck]

Hi Amy, Tristan,

I'm not a Reviewer kind of reviewer, but I've started to look at the
code and can sponsor this.

Some comments on test/java/lang/reflect/Method/invoke/DefaultStaticTest.java:

As there are a lot of non-public top-level classes perhaps this test
should be in it own directory.

It is very hard to read the data table:

292 {interface1.class, 1, 1, new Object1(), new Object[]{},
293 new Object[]{}, "interface1", null},

I believe you should move the methodsNum constant and the declMethods
num constant to an annotation on the interface/class in question. For
example:

@MyTestData(numMethods=1, declMethods=1)
41 interface interface1 {
42 @DefaultMethod(isDefault = true)
43 @StaticMethod(isStatic = false)
44 @ReturnValue(value = "interface1.defaultMethod")
45 default String defaultMethod(){ return "interface1.defaultMethod"; };
46 }

That way it is much easier to inspect that the constants are right.

The same can probably be done with the return values encoded.

Instead of all these "new Objects[] {}" can't you create a field,

Object [] EMPTY_PARAMETERS = new Object() {}

and reuse it?

That way it will be much easier to verify that the encoded test data is
correct.

I'll comment on the other tests shortly. 

cheers
/Joel

On 2013-06-13, Amy Lu wrote:
> This has been pending on review for long ... Please help to review.
> I also need a sponsor for this.
> 
> Thank you very much.
> 
> /Amy
> 
> On 5/23/13 10:48 PM, Amy Lu wrote:
> >Please help to review:
> >
> >More tests for 7184826: (reflect) Add support for Project Lambda
> >concepts in core reflection
> >
> >This includes:
> >1. improvement for existing tests with more situations
> >2. Test for invoking interface default/static method by j.l.r.Method
> >3. Test for invoking interface default/static method by MethodHandle
> >
> >https://dl.dropboxusercontent.com/u/5812451/yl153753/7184826/webrev.00/index.html
> >
> >
> >Thanks,
> >Amy
> 

Re: RFR [8016838] java/lang/instrument/RedefineBigClass.sh needs modification

[Seán Coffey]

On 10/07/13 11:38, Ivan Gerasimov wrote:

Yes, I forgot to include the most important thing - a link to webrev!
Your link is correct.
http://cr.openjdk.java.net/~igerasim/8016838/3/webrev/

The tests fail on linux-x64 only, linux-i586 is fine.
Here's the link to the logs of the tests run by Daniel Daugherty:
http://prt-web.us.oracle.com//archive/2013/07/2013-07-05-045326.ddaugher.8016838_exp/logs/ 



Thus the memory leak seems to be specific to 64-bit Linux.
Ok - didn't know that. Thanks. looks fine to me then, but use Dan or 
someone else as reviewer since I'm not one.


regards,
Sean.



Sincerely yours,
Ivan

On 10.07.2013 13:15, Seán Coffey wrote:

Ivan,

I'll assume this is the latest webrev : 
http://cr.openjdk.java.net/~igerasim/8016838/3/webrev/ 



One comment - should the test be excluded for all linux variants 
(i.e. linux-all) ?


regards,
Sean.

On 09/07/2013 14:09, Ivan Gerasimov wrote:

Please have a chance to review an updated webrev.
It now includes a change to ProblemList.txt, so both modified tests 
are ignored for linux-x64.


Sincerely yours,
Ivan Gersimov

On 08.07.2013 21:27, Seán Coffey wrote:


On 08/07/13 17:55, Ivan Gerasimov wrote:

Thanks, Seán!

I located the build in which the memleak was first introduced -- 
it is jdk8-b69 (hs25-b13).
I've updated the bug 
http://bugs.sun.com/view_bug.do?bug_id=8019845 with this.


So what is the correct procedure to go forward now?
Should I update the webrev to include changes to the problem list?
I believe I shouldn't -- this list seems to be a sensitive stuff.
I'd suggest updating the webrev with the ProblemList 
modification/addition. It's best not to add a test to testruns if 
it's knowingly failing. The test can be removed from ProblemList 
when the jdk8 regression is fixed.


regards,
Sean.



Sincerely yours,
Ivan


On 05.07.2013 15:45, Seán Coffey wrote:
Nice work indeed Ivan. Good to have a reliable testcase to catch 
leaks in this area.


I'd also suggest that this test goes on the ProblemList until the 
new leak is sorted out for jdk8. The goal of JPRT runs is to have 
clean runs.  If it's on the problemList, then it's a known issue 
and is normally tagged with the relevant bug ID so that the 
responsible engineer knows the current state.


regards,
Sean.

On 05/07/2013 11:53, Ivan Gerasimov wrote:


On 05.07.2013 8:35, Daniel D. Daugherty wrote:

Ivan,

The changes look fine, I can sponsor your commit, looks like your
OpenJDK user name is 'igerasim', but I need to know a little bit
more about your testing of these fixes. Did you do a test JPRT
job to run the JLI tests (or just the two tests themselves)?

I've only run test from java/lang/instrument when checked the 
change with JDK6u60 (all passed) and with JDK6u51 (the test 
failed as expected, since the leak had still been there.)



Based on e-mail about this bug fix, I believe you've found a new
leak in JDK8/HSX-25 with test 
java/lang/instrument/RedefineBigClass.sh.

Right. The test shown a memleak with the the latest jdk8.
I filed a bug 8019845 about this leak.
Stefan Karlsson guessed that this may be related to 8003419 
(NPG: Clean up metadata created during class loading if failure)
Then I've checked the builds b57 (test failed) and b58 (test 
passed), so I confirmed that it may be the reason of the leak 
being observed now.

But now I think that the reason may be different.
It just turns out that the test shows failures for (at least) 
three different leaks - the one you, Daniel, solved (7121600), 
the one Stefan wrote about (8003419) and the one currently 
observed (8019845).



That's a good thing, but I think Alan and company would be a bit
grumpy with me if I pushed a test that failed as soon as someone
ran it. :-) Do you know if the revised RetransformBigClass.sh also
finds a new memory leak in JDK8/HSX-25?

The way to deal with that is to put the test on the Problem.list
as part of the same changeset. However, the T&L folks might not 
like

that either...


Well, the leak is there, and why not have a failing test as a 
reminder to have it fixed?


Sincerely yours,
Ivan Gerasimov



Dan


On 7/4/13 6:59 PM, Ivan Gerasimov wrote:

Thank you, Daniel!

Please find an updated webrev at 
http://cr.openjdk.java.net/~igerasim/8016838/2/webrev/.
It now includes the RetransformBigClass test modified in the 
same way as RedefineBigClass
If the changes look fine, may I ask you to sponsor the commit, 
as I'm not a committer?

Here's a link to hg export:
http://cr.openjdk.java.net/~igerasim/2commit/8016838-jdk8-ReBigClass-improved.patch 



Thanks in advance,
Ivan

On 04.07.2013 21:45, Daniel D. Daugherty wrote:

On 7/4/13 11:19 AM, Ivan Gerasimov wrote:

Daniel, thank you for review!

Here's the updated with all all your suggestions adopted.
http://cr.openjdk.java.net/~igerasim/8016838/1/webrev/


Looks good.




I haven't yet considered applying the approach to 
RetransformBigClass.
Do you want me to include this in

Re: RFR [8016838] java/lang/instrument/RedefineBigClass.sh needs modification

[Ivan Gerasimov]

Yes, I forgot to include the most important thing - a link to webrev!
Your link is correct.
http://cr.openjdk.java.net/~igerasim/8016838/3/webrev/

The tests fail on linux-x64 only, linux-i586 is fine.
Here's the link to the logs of the tests run by Daniel Daugherty:
http://prt-web.us.oracle.com//archive/2013/07/2013-07-05-045326.ddaugher.8016838_exp/logs/

Thus the memory leak seems to be specific to 64-bit Linux.

Sincerely yours,
Ivan

On 10.07.2013 13:15, Seán Coffey wrote:

Ivan,

I'll assume this is the latest webrev : 
http://cr.openjdk.java.net/~igerasim/8016838/3/webrev/ 



One comment - should the test be excluded for all linux variants (i.e. 
linux-all) ?


regards,
Sean.

On 09/07/2013 14:09, Ivan Gerasimov wrote:

Please have a chance to review an updated webrev.
It now includes a change to ProblemList.txt, so both modified tests 
are ignored for linux-x64.


Sincerely yours,
Ivan Gersimov

On 08.07.2013 21:27, Seán Coffey wrote:


On 08/07/13 17:55, Ivan Gerasimov wrote:

Thanks, Seán!

I located the build in which the memleak was first introduced -- it 
is jdk8-b69 (hs25-b13).
I've updated the bug http://bugs.sun.com/view_bug.do?bug_id=8019845 
with this.


So what is the correct procedure to go forward now?
Should I update the webrev to include changes to the problem list?
I believe I shouldn't -- this list seems to be a sensitive stuff.
I'd suggest updating the webrev with the ProblemList 
modification/addition. It's best not to add a test to testruns if 
it's knowingly failing. The test can be removed from ProblemList 
when the jdk8 regression is fixed.


regards,
Sean.



Sincerely yours,
Ivan


On 05.07.2013 15:45, Seán Coffey wrote:
Nice work indeed Ivan. Good to have a reliable testcase to catch 
leaks in this area.


I'd also suggest that this test goes on the ProblemList until the 
new leak is sorted out for jdk8. The goal of JPRT runs is to have 
clean runs.  If it's on the problemList, then it's a known issue 
and is normally tagged with the relevant bug ID so that the 
responsible engineer knows the current state.


regards,
Sean.

On 05/07/2013 11:53, Ivan Gerasimov wrote:


On 05.07.2013 8:35, Daniel D. Daugherty wrote:

Ivan,

The changes look fine, I can sponsor your commit, looks like your
OpenJDK user name is 'igerasim', but I need to know a little bit
more about your testing of these fixes. Did you do a test JPRT
job to run the JLI tests (or just the two tests themselves)?

I've only run test from java/lang/instrument when checked the 
change with JDK6u60 (all passed) and with JDK6u51 (the test 
failed as expected, since the leak had still been there.)



Based on e-mail about this bug fix, I believe you've found a new
leak in JDK8/HSX-25 with test 
java/lang/instrument/RedefineBigClass.sh.

Right. The test shown a memleak with the the latest jdk8.
I filed a bug 8019845 about this leak.
Stefan Karlsson guessed that this may be related to 8003419 (NPG: 
Clean up metadata created during class loading if failure)
Then I've checked the builds b57 (test failed) and b58 (test 
passed), so I confirmed that it may be the reason of the leak 
being observed now.

But now I think that the reason may be different.
It just turns out that the test shows failures for (at least) 
three different leaks - the one you, Daniel, solved (7121600), 
the one Stefan wrote about (8003419) and the one currently 
observed (8019845).



That's a good thing, but I think Alan and company would be a bit
grumpy with me if I pushed a test that failed as soon as someone
ran it. :-) Do you know if the revised RetransformBigClass.sh also
finds a new memory leak in JDK8/HSX-25?

The way to deal with that is to put the test on the Problem.list
as part of the same changeset. However, the T&L folks might not 
like

that either...


Well, the leak is there, and why not have a failing test as a 
reminder to have it fixed?


Sincerely yours,
Ivan Gerasimov



Dan


On 7/4/13 6:59 PM, Ivan Gerasimov wrote:

Thank you, Daniel!

Please find an updated webrev at 
http://cr.openjdk.java.net/~igerasim/8016838/2/webrev/.
It now includes the RetransformBigClass test modified in the 
same way as RedefineBigClass
If the changes look fine, may I ask you to sponsor the commit, 
as I'm not a committer?

Here's a link to hg export:
http://cr.openjdk.java.net/~igerasim/2commit/8016838-jdk8-ReBigClass-improved.patch 



Thanks in advance,
Ivan

On 04.07.2013 21:45, Daniel D. Daugherty wrote:

On 7/4/13 11:19 AM, Ivan Gerasimov wrote:

Daniel, thank you for review!

Here's the updated with all all your suggestions adopted.
http://cr.openjdk.java.net/~igerasim/8016838/1/webrev/


Looks good.




I haven't yet considered applying the approach to 
RetransformBigClass.
Do you want me to include this into this same change set or 
should I make it separately?


I would include it in the same changeset.

Dan




Sincerely yours,
Ivan


On 04.07.2013 19:34, Daniel D. Daugherty wrot

Re: RFR [8016838] java/lang/instrument/RedefineBigClass.sh needs modification

[Seán Coffey]

Ivan,

I'll assume this is the latest webrev : 
http://cr.openjdk.java.net/~igerasim/8016838/3/webrev/ 



One comment - should the test be excluded for all linux variants (i.e. 
linux-all) ?


regards,
Sean.

On 09/07/2013 14:09, Ivan Gerasimov wrote:

Please have a chance to review an updated webrev.
It now includes a change to ProblemList.txt, so both modified tests 
are ignored for linux-x64.


Sincerely yours,
Ivan Gersimov

On 08.07.2013 21:27, Seán Coffey wrote:


On 08/07/13 17:55, Ivan Gerasimov wrote:

Thanks, Seán!

I located the build in which the memleak was first introduced -- it 
is jdk8-b69 (hs25-b13).
I've updated the bug http://bugs.sun.com/view_bug.do?bug_id=8019845 
with this.


So what is the correct procedure to go forward now?
Should I update the webrev to include changes to the problem list?
I believe I shouldn't -- this list seems to be a sensitive stuff.
I'd suggest updating the webrev with the ProblemList 
modification/addition. It's best not to add a test to testruns if 
it's knowingly failing. The test can be removed from ProblemList when 
the jdk8 regression is fixed.


regards,
Sean.



Sincerely yours,
Ivan


On 05.07.2013 15:45, Seán Coffey wrote:
Nice work indeed Ivan. Good to have a reliable testcase to catch 
leaks in this area.


I'd also suggest that this test goes on the ProblemList until the 
new leak is sorted out for jdk8. The goal of JPRT runs is to have 
clean runs.  If it's on the problemList, then it's a known issue 
and is normally tagged with the relevant bug ID so that the 
responsible engineer knows the current state.


regards,
Sean.

On 05/07/2013 11:53, Ivan Gerasimov wrote:


On 05.07.2013 8:35, Daniel D. Daugherty wrote:

Ivan,

The changes look fine, I can sponsor your commit, looks like your
OpenJDK user name is 'igerasim', but I need to know a little bit
more about your testing of these fixes. Did you do a test JPRT
job to run the JLI tests (or just the two tests themselves)?

I've only run test from java/lang/instrument when checked the 
change with JDK6u60 (all passed) and with JDK6u51 (the test failed 
as expected, since the leak had still been there.)



Based on e-mail about this bug fix, I believe you've found a new
leak in JDK8/HSX-25 with test 
java/lang/instrument/RedefineBigClass.sh.

Right. The test shown a memleak with the the latest jdk8.
I filed a bug 8019845 about this leak.
Stefan Karlsson guessed that this may be related to 8003419 (NPG: 
Clean up metadata created during class loading if failure)
Then I've checked the builds b57 (test failed) and b58 (test 
passed), so I confirmed that it may be the reason of the leak 
being observed now.

But now I think that the reason may be different.
It just turns out that the test shows failures for (at least) 
three different leaks - the one you, Daniel, solved (7121600), the 
one Stefan wrote about (8003419) and the one currently observed 
(8019845).



That's a good thing, but I think Alan and company would be a bit
grumpy with me if I pushed a test that failed as soon as someone
ran it. :-) Do you know if the revised RetransformBigClass.sh also
finds a new memory leak in JDK8/HSX-25?

The way to deal with that is to put the test on the Problem.list
as part of the same changeset. However, the T&L folks might not like
that either...


Well, the leak is there, and why not have a failing test as a 
reminder to have it fixed?


Sincerely yours,
Ivan Gerasimov



Dan


On 7/4/13 6:59 PM, Ivan Gerasimov wrote:

Thank you, Daniel!

Please find an updated webrev at 
http://cr.openjdk.java.net/~igerasim/8016838/2/webrev/.
It now includes the RetransformBigClass test modified in the 
same way as RedefineBigClass
If the changes look fine, may I ask you to sponsor the commit, 
as I'm not a committer?

Here's a link to hg export:
http://cr.openjdk.java.net/~igerasim/2commit/8016838-jdk8-ReBigClass-improved.patch 



Thanks in advance,
Ivan

On 04.07.2013 21:45, Daniel D. Daugherty wrote:

On 7/4/13 11:19 AM, Ivan Gerasimov wrote:

Daniel, thank you for review!

Here's the updated with all all your suggestions adopted.
http://cr.openjdk.java.net/~igerasim/8016838/1/webrev/


Looks good.




I haven't yet considered applying the approach to 
RetransformBigClass.
Do you want me to include this into this same change set or 
should I make it separately?


I would include it in the same changeset.

Dan




Sincerely yours,
Ivan


On 04.07.2013 19:34, Daniel D. Daugherty wrote:

On 7/3/13 11:12 AM, Ivan Gerasimov wrote:

Hello everybody!

We have a request to improve jtreg test.
The test had been written to verify fix for memory leak 
during class redefinition.
The problem is that it always is reported as PASSED even in 
the presence of the leak.


The proposed change is platform specific.
It allows memory leak detection only on Linux.
This is because the memory leak was in native code, so 
there's no easy way to detect it from within Java code.


Here

Re: RFR: 8015320: Pull spliterator() up from Collection to Iterable

[Paul Sandoz]

On Jul 10, 2013, at 10:33 AM, Paul Sandoz  wrote:
> 
> On Jul 10, 2013, at 12:52 AM, Henry Jen  wrote:
> 
>> Hi,
>> 
>> Please review the webrev at
>> 
>> http://cr.openjdk.java.net/~henryjen/ccc/8015320.0/webrev/
>> 
>> This allows turning an Iterable into a stream with spliterator() methods
>> if the implementation provides one(like many Collection implementations)
>> rather than be forced to use less-efficient iterator().
>> 
>> Two small changes are not directly related,
>> - cleanup on SpliteratorCollisions.java test
>> - add a @see Spliterator for ConcurrentModificationException
>> 
> 
> Looks good.
> 

Drat, i spoke too soon.

I suggest the following change to the JavaDOc of Iterable.spliterator():

/**
 * Creates a {@link Spliterator} over the elements described by this
 * {@code Iterable}.
 *
 * @implSpec
 * The default implementation creates an
 * early-binding spliterator
 * from the iterable's {@code Iterator}.  The spliterator inherits the
 * fail-fast properties of the iterable's iterator.

 * @implNote
 * The default implementation should usually be overridden.  The spliterator
 * returned by the default implementation has poor splitting capabilities,
 * is unsized, and does not report any spliterator characteristics.
 * Implementing classes can nearly always provide a better implementation.
 *
 * @return a {@code Spliterator} over the elements described by this
 * {@code Iterable}.
 * @since 1.8
 */
default Spliterator spliterator() {


Paul.

Re: RFR: 8020062: Nest StreamBuilder interfaces inside relevant Stream interfaces

[Paul Sandoz]

On Jul 10, 2013, at 2:14 AM, Henry Jen  wrote:

> Hi,
> 
> Please review the webrev at
> 
> http://cr.openjdk.java.net/~henryjen/ccc/8020062/0/webrev/index.html
> 
> This webrev does as the bug suggested, is a refactoring that moves
> StreamBuilder interfaces implementations into relevant Stream interfaces.
> 

Looks good.

Paul.

> The patch is created against the version after applying two earlier RFR
> of 8015315+8019395, which we hope will be pushed before this one. That
> said, there is no hard dependency on those two changesets.
> 
> Cheers,
> Henry
> 

Re: RFR: 8015320: Pull spliterator() up from Collection to Iterable

[Paul Sandoz]

On Jul 10, 2013, at 12:52 AM, Henry Jen  wrote:

> Hi,
> 
> Please review the webrev at
> 
> http://cr.openjdk.java.net/~henryjen/ccc/8015320.0/webrev/
> 
> This allows turning an Iterable into a stream with spliterator() methods
> if the implementation provides one(like many Collection implementations)
> rather than be forced to use less-efficient iterator().
> 
> Two small changes are not directly related,
> - cleanup on SpliteratorCollisions.java test
> - add a @see Spliterator for ConcurrentModificationException
> 

Looks good.

--

As an aside there is lots of shared code in SpliteratorCollisions and 
SpliteratorTraversingAndSplittingTest that should be pulled out into a separate 
class.

Paul.

hg: jdk8/tl/jdk: 8020040: Improve and generalize the F/J tasks to handle right or left-balanced trees

[paul . sandoz]

Changeset: 882baa1e0a38
Author:psandoz
Date:  2013-07-10 10:24 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/882baa1e0a38

8020040: Improve and generalize the F/J tasks to handle right or left-balanced 
trees
Reviewed-by: briangoetz
Contributed-by: doug lea , paul sandoz 


! src/share/classes/java/util/stream/AbstractShortCircuitTask.java
! src/share/classes/java/util/stream/AbstractTask.java
! src/share/classes/java/util/stream/ForEachOps.java
! src/share/classes/java/util/stream/Nodes.java

hg: jdk8/tl/jdk: 8017447: Unmodifiable map entry becomes modifiable if taken from a stream of map entries

[paul . sandoz]

Changeset: ff5df05222d1
Author:psandoz
Date:  2013-07-10 09:52 +0200
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/ff5df05222d1

8017447: Unmodifiable map entry becomes modifiable if taken from a stream of 
map entries
Reviewed-by: briangoetz

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

hg: jdk8/tl/jdk: 8019267: NPE in AbstractSaslImpl when trace level >= FINER in KRB5

[weijun . wang]

Changeset: 780a64979c8d
Author:weijun
Date:  2013-07-10 15:11 +0800
URL:   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/780a64979c8d

8019267: NPE in AbstractSaslImpl when trace level >= FINER in KRB5
Reviewed-by: mullan

! src/share/classes/com/sun/security/sasl/util/AbstractSaslImpl.java
! test/sun/security/krb5/auto/SaslGSS.java