Review Request for CR : 7144861 RMI activation tests are too slow (wrong webev link fixed)
stuart.marks at oracle.com
Fri May 11 01:04:38 UTC 2012
Looks good. Just one thing: in JavaVM.java, the declaration line for "boolean
started" still has a comment that says "updated by started() method". That
method has been renamed to setStarted(). Either fix the comment or perhaps
better, remove it entirely. It's pretty easy to find uses of private variables.
If the comment isn't there it won't get out of date! :-)
BTW I ran before-and-after tests of the java/rmi/activation tests (32 tests),
and the results are as follows:
This is great!
If the comment typo is the only change then I don't think you need another
webrev before you push. Oh wait, you can't push, can you. OK, I'll do the push
then, and I can apply the comment fix if there aren't any other changes. I'll
wait overnight to hear from Alan or David, and if there's nothing else, I'll go
ahead with it tomorrow.
On 5/10/12 3:20 PM, Olivier Lagneau wrote:
> Olivier Lagneau said on date 5/11/2012 12:13 AM:
>> Please find the second webrev with your remarks applied here:
> Html link above is buggy. Please go to this correct location:
>> In addition to the way we have agreed (see below) to handle
>> InterruptedException in this fix,
>> I have applied all the other requests for change.
>> Regarding RMID.start() with outer and inner timer loops, I have kept the
>> current structure of the code,
>> but for sure this should be totally rewritten in a further cleanup of the code.
>> Stuart Marks said on date 5/10/2012 6:28 AM:
>>> On 5/9/12 8:26 AM, Olivier Lagneau wrote:
>>>> Given that we want to push the code quickly, I don't think I should go for
>>>> a large IE cleanup for this fix,
>>>> which is meant to provide better exceution speed only.
>>>> I suggest to follow Stuart's proposal first (i.e. reassert and return
>>>> immediately in my code changes) ,
>>>> and create a dedicated new CR regarding proper handling of IE in all the rmi
>>>> tests (low priority).
>>>> Do you agree with this ?
>>> Yes, this seems like a sensible approach. The new CR might also cover the
>>> cleaning/unification of all the retry-repeatedly-with-time-limit loops that
>>> are spread through this code.
More information about the core-libs-dev