Review Request for CR : 7144861 RMI activation tests are too slow (wrong webev link fixed)

David Holmes david.holmes at oracle.com
Fri May 11 01:14:57 UTC 2012


IE handling seems fine. No further comments from me.

Note I was not a full reviewer of this.

David

On 11/05/2012 11:04 AM, Stuart Marks wrote:
> 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:
>
> before: 18m21s
> after: 7m52s
>
> 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.
>
> Thanks!!
>
> s'marks
>
> 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:
>>> http://cr.openjdk.java.net/~olagneau/7144861/webrev.01/
>>
>> Html link above is buggy. Please go to this correct location:
>> http://cr.openjdk.java.net/~olagneau/7144861/webrev.01
>>
>>
>>>
>>> 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.
>>>
>>> Thanks,
>>> Olivier.
>>>
>>>
>>>
>>> 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
>>>>> such
>>>>> 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.
>>>>
>>>> Thanks.
>>>>
>>>> s'marks
>>>
>>



More information about the core-libs-dev mailing list