Pushed:
http://hg.openjdk.java.net/jdk8/tl/jdk/rev/97eb7a4b1fdd
s'marks
On 7/3/12 6:54 PM, Eric Wang wrote:
Opps, It is my carelessness, I will be more careful in the next bug fixes.
Thank you to review and change it.
Regards,
Eric
On 2012/7/4 8:58, Stuart Marks wrote:
Sure, I can take care of that.
If this is the only change, no need to generate another webrev, Eric.
s'marks
On 7/3/12 4:53 PM, David Holmes wrote:
Please fix the missing space in
if(ref.get()
before pushing.
Thanks,
David
On 4/07/2012 7:04 AM, Stuart Marks wrote:
Webrev now posted at
http://cr.openjdk.java.net/~smarks/yiming.wang/6948101/webrev.2/
The changes look good to me. If there's no further discussion, I'll push
them in a couple days (U.S. holiday coming up).
s'marks
On 7/3/12 12:20 AM, Stuart Marks wrote:
This approach looks good to me. Please go ahead and update 7123972 as
well, and
I'll post these webrevs tomorrow (my time).
Thanks.
s'marks
On 7/2/12 11:41 PM, Eric Wang wrote:
David,
I have added the comments before the loop, please help to review. If
it is OK,
I'll update the 7123972 as well.
Thanks,
Eric
On 2012/7/3 12:22, David Holmes wrote:
Hi Eric,
On 3/07/2012 1:28 PM, Eric Wang wrote:
Hi Stuart and David,
Thanks for the suggestion about the timeout. so there are three
ways to
handle the timeout.
1. Add comment to explain that test failed due to default timeout
2. Specify the the timeout option
3. Timeout programly
Is it ok for you that i'd like to choose the second way "explict
timeout" as the test looks simply and lucid?
If by #2 you mean add something like:
@run main/timeout=10
then no. Earlier this year there were changes to a bunch of tests
that set
timeouts shorter than the jtreg default, to delete the timeouts.
Given that,
my misgivings about relying on the harness are not relevant so we
need only
do #1 and add a comment to make it clear that a failing test will be
indicated via the harness timeout.
David
-----
Regards,
Eric
On 2012/7/3 10:38, David Holmes wrote:
On 3/07/2012 7:08 AM, Stuart Marks wrote:
On 7/1/12 5:39 PM, David Holmes wrote:
Now, how can the test fail? If ref is never cleared, the while
loop
will
never terminate, and we rely on jtreg to timeout and kill this
test. It
would be good to have a brief comment above the while loop that
explains
this.
Agreed. Something like
// Might require multiple calls to System.gc() for weak-references
processing
to be complete.
// If the weak-reference is not cleared as expected we will hang
here
// until timed out by the test harness
Though now I write that I'm unhappy that this will only behave
nicely
if run
from jtreg. We do use explicit timeouts in other tests.
So, are you unhappy enough that we should ask Eric to add an
explicit
timeout?
No. But I'm not the authoritive voice in this area :)
I don't think it would be that difficult. Perhaps a technique
like the
following would suffice. Get System.currentTimeMillis() before
the loop,
and call it again each time through the loop, and fail if the
difference
is greater than the timeout (e.g., 60 sec). Doesn't involve other
threads or even Timer/TimerTask.
Or simply limit the number of loops so that N*sleepTime = timeout.
This then requires adding back the check for null outside the loop.
On the other hand if one is running this test directly instead of
through jtreg, one can just ^C it if it's taking an unexpectedly
long
time.
True.
David
-----
s'marks
