Improving inspect on class objects

[Allison Randal]

Patrick R. Michaud wrote:
> On Sun, Jan 04, 2009 at 05:00:46PM -0800, Allison Randal wrote:
>> Jonathan Worthington wrote:
>>> Patrick R. Michaud wrote:
>>>
>>>> Lastly, there's also the issue that the cloning makes inspection
>>>> of classes' attributes and methods into a much more expensive  
>>>> operation, and helps to explain why P6object, PCT, and Rakudo
>>>> were generating far more hashes than I expected.
>> What are you using 'inspect' for? It should be rarely used (i.e. it's  
>> intended for advanced debugging, not for highly-optimized ordinary  
>> program operation).
> 
> Currently it's used for two purposes:
> 
> 1.  When creating a new instance of the class, it's used to identify
>     (a) the list of attributes that need initialization and 
>     (b) the metadata needed to initialize each attribute
> 
> 2.  When defining the class itself, I'm (ab)using it to modify
>     the attribute metadata, since at present there doesn't
>     appear to be another interface available for doing so.
> 
>> If 'inspect' is the only way to get at some piece of data you use  
>> frequently, we likely need to give you a simpler way of accessing it.
> 
> "List of attributes and metadata" is what we will want/need frequently,
> as identified in (1) above.

Okay, (2) is taken care of with 'get_attr_*'/'set_attr_*'. (1) works 
fine as a clone (not modifying, just reading), but creating a clone of 
the attribute metadata every time you instantiate a new object is way 
too expensive.

I suggest exploring what we need out of this a little further. Again the 
'inspect' vtable function isn't really right for it. It might be a class 
method.

>> In the vtable signature for 'add_attribute', 'type' should actually be  
>> 'metadata' which can alternatively be a hash of metadata for the  
>> attribute, rather than just type:
>>
>>   VTABLE void add_attribute(STRING *name, PMC *type) {
> 
> That would work just fine for me -- I'm guessing that the
> keys in 'type' ('metadata') would end up being used as the
> keys in attrib_metadata ?

Yes, should be. Pseudocode something like "if the PMC argument is a 
Hash, the keys become the keys in attrib_metadata" and "if the PMC 
argument is anything other than a Hash, store it as the 'type' in the 
attribute metadata".

>> This doesn't allow for modification of metadata for an existing  
>> attribute. I'm open to adding 'get_attr_*' and 'set_attr_*' vtable  
>> functions to the Class PMC for modifying attribute metadata. It's  
>> sensible that the 'getattribute' and 'setattribute' opcodes on a Class  
>> work with attribute metadata, while on an Object they work with  
>> attribute instance data.
> 
> This would be a very workable solution from my perspective.
> I'll be glad to make these changes to trunk if you're willing
> to commit to them.

Sounds good.

> In the 'rvar' branch I've already adjusted the inspect_str vtable 
> so that it returns a shallow clone of hashes instead of a deep clone,
> and I can merge that into trunk also.

Since we'll add 'get_attr_*' and 'set_attr_*', keep 'inspect' on 
attribute metadata and the attribute index as a deep clone (to entirely 
prevent modifying them via 'inspect').

The 'parents', 'all_parents', 'methods', 'vtable_overrides', and 'roles' 
should be shallow cloned, so you get a different array, containing the 
original class/role/method objects.

The new PMCs created for 'name','id', and 'flags' shouldn't be cloned at 
all.

Allison