review (M) for 4926272: methodOopDesc::method_from_bcp is unsafe
tom.rodriguez at oracle.com
Wed Jan 12 18:18:14 PST 2011
On Jan 12, 2011, at 4:42 PM, John Rose wrote:
> On Jan 12, 2011, at 3:17 PM, Coleen Phillimore wrote:
>> This is a really nice change.
> Exactly my thought.
> Tom, thanks for the multiple iterations of extra care. I like the new webrev.
> A final thought: I noticed you added methodOopDesc::code_at (with java_code_at), as part of the pattern of making the method argument more explicit everywhere. (Doesn't work everywhere, since sometimes the method is null, but it works lots of places.) You might consider (though it's not necessary) changing some of your use points of Bytecodes::code_at(m,x) to m->code_at(x). One spot I noticed was methodOopDesc::is_accessor.
I was looking for more places to use that but most of them want the bcp anyway so it wasn't much better. is_accessor is a good one though and I fixed a minor use in some printing code in deoptimization.cpp. Many of the other ones feel like they could be simplified by embedding more logic in Bytecode itself but I'm not going to go there.
Thanks for all the good feedback Coleen, John and Vladimir. I'm going to redo my testing now and push tomorrow if everything looks good.
> -- John
More information about the hotspot-dev