On 1/1/2015 12:22 PM, Peter Levart wrote:
Hi Brad,

Here's next webrev which tries to cover all your comments:

http://cr.openjdk.java.net/~plevart/jdk9-dev/FileInputStreamPool.8047769/webrev.04/

I downloaded the webrev.05 (Chris' later followup email) and ran it through JPRT. The only error was the previously seen -Dseed which is not your problem.

Again, I only ran:

    jdk_lang, jdk_math, jdk_util, jdk_io, jdk_net, jdk_nio,
    jdk_security*, jdk_rmi, jdk_text, jdk_time, jdk_other, core_tools.

If you need anything else run, let me know.

Looks like you have a committer status, will you be pushing this?

I can, yes. As soon as we clear-out the remaining questions, right?

Yes.  The comments below are minor and shouldn't need another review
cycle.

I'm worried about the failure of the test you observed while running
from NetBeans. Perhaps a 0.5s wait is sometimes not enough for
ReferenceHandler thread to process (enqueue) a WeakReference. Since
there is already a facility in place to help ReferenceHandler thread
instead of wait for it, I used it in new version of the test.

BTW, there's now an unnecessary import from java.lang.AssertionError added in webrev.04.

TEST RESULT: Failed. Compilation failed: Compilation failed

I changed the test to be self-contained now so one can run it without
testlib in classpath.

Thanks.  It's compiling fine now.

Two minor nits?   SeedGenerator.java:  Lines 507/518

Done that too.

Thanks.

Maybe issue multiple reads to exercise in1 and in2?  e.g. 2 bytes of
in1, 4 bytes of in2, then 2 bytes of in1?

The 1st assert makes sure in1 == in2, so there's no point in invoking
the same instance via two aliases.

True.

IIRC, when I ran this under NetBeans last week, the second testCaching
didn't clear the WeakReference.

This should not happen any more now that the test is helping to enqueue
the WeakReferences instead of waiting for ReferenceHandler to enqueue
them.

Yes, that hit the refQueue.poll().

It's always interesting to work with core-libs folks, learn something new everyday. Mixing Lambdas/try-with.

I need a time-machine for your CFV/jdk8 Committer:

    http://mail.openjdk.java.net/pipermail/jdk8-dev/2013-August/002896.html

I vote yes.

The test can now fail only if System.gc() does not trigger
WeakReference processing in the VM. Can you give it a try on your
NetBeans environment?

One last comment.  It's now 2015.  ;)

Brad

Reply via email to