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
