RFR: jsr166 jdk9 integration wave 5

[Stuart Marks]

Hi Martin,

I'm a bit confused about exactly what pieces need review here. Since you 
mentioned me with respect to 8150523, I took a look at the webrev that adds the 
timeout factors:

http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/timeoutFactor/

Do other webrevs still need review as well? I haven't looked at them. But there 
are others' names on them already....

Otherwise, overall, it looks fine, just a few minor questions/comments:

------------------------------------------------------------

In the following files, various delays of 7 sec, 20 sec, 30 sec, 60 sec, 100 
sec, 120 sec, and 1000 sec were changed to 10 sec (exclusive of timeout factor 
adjustment). Was that intentional? I guess making the "backstop" timeouts (e.g., 
waiting for an executor service to terminate) be uniform is reasonable, but 
there were some very long timeouts that are now much shorter. At least, 
something to keep an eye on.

test/java/util/concurrent/BlockingQueue/Interrupt.java:
test/java/util/concurrent/BlockingQueue/ProducerConsumerLoops.java
test/java/util/concurrent/BlockingQueue/SingleProducerMultipleConsumerLoops.java
test/java/util/concurrent/CompletableFuture/Basic.java
test/java/util/concurrent/ConcurrentHashMap/MapLoops.java
test/java/util/concurrent/ConcurrentQueues/ConcurrentQueueLoops.java
test/java/util/concurrent/Exchanger/ExchangeLoops.java
test/java/util/concurrent/Executors/AutoShutdown.java
test/java/util/concurrent/ScheduledThreadPoolExecutor/ZeroCorePoolSize.java
test/java/util/concurrent/ThreadPoolExecutor/Custom.java
test/java/util/concurrent/ThreadPoolExecutor/SelfInterrupt.java
test/java/util/concurrent/ThreadPoolExecutor/ThreadRestarts.java
test/java/util/concurrent/locks/Lock/FlakyMutex.java
test/java/util/concurrent/locks/LockSupport/ParkLoops.java
test/java/util/concurrent/locks/ReentrantLock/LockOncePerThreadLoops.java
test/java/util/concurrent/locks/ReentrantLock/SimpleReentrantLockLoops.java
test/java/util/concurrent/locks/ReentrantLock/TimeoutLockLoops.java
test/java/util/concurrent/locks/StampedLock/Basic.java

------------------------------------------------------------

test/java/util/concurrent/FutureTask/CancelledFutureLoops.java
test/java/util/concurrent/ThreadPoolExecutor/TimeOutShrink.java

It's slightly odd to see an additional multiplier at the use site of 
LONG_DELAY_MS, when this doesn't occur in most of the other tests that 
(formerly) had different timeouts. Put another way, why do these tests have 
different timeouts, whereas the tests above that had widely differing timeouts 
were all changed to 10 sec?

------------------------------------------------------------

In various scheduled thread pool executor tests, as well as in

test/java/util/concurrent/ThreadPoolExecutor/ThreadRestarts.java

should delays for scheduled tasks also be scaled as well? If the tests are 
running in a slow environment, and some timeouts are scaled but not others, it 
might result in some tasks executing too soon. I guess this depends on the 
semantics of what's being tested.

------------------------------------------------------------

test/java/util/concurrent/ConcurrentQueues/GCRetention.java

  - extra commented-out call to forceFullGc() ?
  - probably would be wise to scale the timeout in finalizeDone.await()

------------------------------------------------------------

test/java/util/concurrent/Exchanger/ExchangeLoops.java
test/java/util/concurrent/ExecutorCompletionService/ExecutorCompletionServiceLoops.java

The number of iterations was reduced from 100,000 to 2,000, particularly the 
initial "warm up" run (at least in ExchangeLoops). IIRC the C2 compiler kicks in 
at 10,000 iterations. The reduced the number of iterations (particularly in the 
initial "warm up" runs) doesn't meet this threshold. Could that be a problem?


s'marks




On 2/29/16 9:52 AM, Martin Buchholz wrote:
> I added
>
> 8150523: improve jtreg test timeout handling, especially -timeout:
>
> to this wave, inspired by smarks.
> I stress tested this with the flags that caused JDK-8150523 to fail,
> and they now seem robust, as long as a reasonable -timeout flag is
> provided to jtreg.
>
> On Tue, Feb 23, 2016 at 6:58 PM, Martin Buchholz <martinrb at google.com> wrote:
>> A very boring jsr166 integration, focused on reliability.
>> This one has the promised "even more unnecessarily robust" ThreadLocalRandom.
>>
>> http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/