Hi,
Please review the fix for 8023477:
http://cr.openjdk.java.net/~jiangli/8023477/webrev.00/
Both tests reported by the bug failed due to the same problem. They both
are passing now.
The original memory reduction change for 8021948 turned out to be a
little trickier than I expected.
Thanks,
Jiangli
On 08/23/2013 11:00 AM, Jiangli Zhou wrote:
I found the issue in the SA code. For class without generic signature,
the InstanceKlass::_generic_signature_index is 0. Need to check for
this case in the SA code. I'm working on the fix and doing testing.
Thanks,
Jiangli
On 08/23/2013 08:21 AM, Jiangli Zhou wrote:
Hi Staffan,
Thanks for the info. Will look into that one.
Jiangli
On 08/23/2013 05:27 AM, Staffan Larsen wrote:
Unfortunately there are two more tests that started failing after
the initial change. See: JDK-8023477.
/Staffan
On 23 aug 2013, at 01:16, Jiangli Zhou <jiangli.z...@oracle.com> wrote:
Hi Coleen,
Yes, that's the case. Thanks for the review. I'll push this to
hotspot-rt.
Thanks,
Jiangli
On 08/22/2013 03:34 PM, Coleen Phillimore wrote:
Hi,
Is it the case that the old index isn't in the index map because
it didn't change? If so, this looks correct.
Thanks,
Coleen
On 08/22/2013 06:15 PM, Jiangli Zhou wrote:
On 08/22/2013 03:10 PM, serguei.spit...@oracle.com wrote:
Hi Jiangli,
The fix is good and safe.
I'm happy you fixed another case as well!
Let's consider current bug as a clean-up issue so that we do not
need to file a separate bug. :)
Ok.
Thanks,
Jiangli
Thanks,
Serguei
On 8/22/13 2:50 PM, Jiangli Zhou wrote:
Hi Serguei,
I've also made change to the case that you discovered. Please
let me know if you think a separate bug should be filed to
track it instead.
http://cr.openjdk.java.net/~jiangli/8023547/webrev.01/
Thanks,
Jiangli
On 08/22/2013 02:24 PM, Jiangli Zhou wrote:
Hi Serguei,
Thank you very much for the review and confirmation with the
test.
Jiangli
On 08/22/2013 02:18 PM, serguei.spit...@oracle.com wrote:
Hi Jiangli,
Thank you for the quick fix which looks fine to me.
I confirm the test is passed with it.
Staffan, thank you for the regression isolation.
I've noticed the following fragment in this file which seems
has a similar issue:
// We also need to rewrite the parameter name indexes, if
there is
// method parameter data present
if(method->has_method_parameters()) {
const int len = method->method_parameters_length();
MethodParametersElement* elem =
method->method_parameters_start();
for (int i = 0; i < len; i++) {
const u2 cp_index = elem[i].name_cp_index;
elem[i].name_cp_index = find_new_index(cp_index);
}
}
} // end rewrite_cp_refs_in_method()
The result of the find_new_index() above is not checked for 0.
Thanks,
Serguei
On 8/22/13 12:38 PM, Jiangli Zhou wrote:
Hi Staffan, Serguei and others,
Here is the webrev for the 8023547 fix:
http://cr.openjdk.java.net/~jiangli/8023547/webrev.00/
Thanks!
Jiangli