RFR(L) 8026054: New type profiling points: type of return values at calls

[Christian Thalinger]

On Oct 11, 2013, at 1:32 AM, Roland Westrelin <roland.westrelin at oracle.com> wrote:

> Hi Chris,
> 
>>>> +           ciTypeEntriesAtCall* args_and_ret = data->is_CallTypeData() ? ((ciCallTypeData*)data)->args_and_ret() : ((ciVirtualCallTypeData*)data)->args_and_ret();
>>>> 
>>>> !   const TypeEntriesAtCall* args_and_ret() const { return &_args_and_ret; }
>>>> 
>>>> I should've mentioned this in the previous review but can't we make args_and_ret() a virtual method?
>>> 
>>> In what class? ProfileData?
>> 
>> I don't know; I've lost track of the class hierarchy.  If the class hierarchy is sane there must be a super class that's suitable.
>> 
>>> Wouldn't that expose some implementation details at a level where they don't make much sense. Also on ci* side, there's no common root class and we probably don't want ci* stuff to leak in methodData.hpp.
>> 
>> Why isn't there a common super class in ci?
> 
> The common class in ci is ProfileData. 
> 
> There are places where I test:
> data->is_CallTypeData() || data->is_VirtualCallTypeData()
> to find out whether type profiling is on. This one I could replace with a virtual method in ProfileData. is_profiling_types_at_call() maybe.
> 
> Other candidates are:
> has_return()
> number_of_arguments()
> args_data_offset()
> 
> What would they become if they were virtual methods in ProfileData? at_call_and_has_return()? has_return() doesn't make any sense in ProfileData because it is the common class for all profiling. There is no common class for calls.
> 
> And then what about these:
> args() which returns a ciTypeStackSlotEntries*
> ret() which returns a ciReturnTypeEntry*
> 
> They would need to be virtual in ProfileData but return objects from ci? That doesn't seem right.
> 
> Or we could add static methods to TypeEntriesAtCall and hide the tests there:
> 
> static bool has_return(ProfileData* data) {
>  assert(data->is_profiling_types_at_call(), "");
>  return data->is_CallTypeData() ? ((ciCallTypeData*)data)->has_return() : ((ciVirtualCallTypeData*)data)->has_return();
> }
> 
> has_return() makes sense in the context of TypeEntriesAtCall. I don't think it does in ProfileData.

It seems we'd have to redesign the class hierarchy for virtual methods to work.  Leave it as it is for now.  Maybe we can think about it for 9.

> 
> Roland.