Thanks, Staffan!

Jiangli

On 08/26/2013 01:13 AM, Staffan Larsen wrote:
Looks good.

Thanks,
/Staffan

On 23 aug 2013, at 23:49, Jiangli Zhou <jiangli.z...@oracle.com> wrote:

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

Reply via email to