Re: fixes to goops + light structs + 'u' slots

[Andy Wingo]

Hey Ludovic,

Thanks for poking the patch!

On Thu 10 Apr 2008 23:33, address@hidden (Ludovic Courtès) writes:

>> From: Andy Wingo <address@hidden>
>
> You forgot to set your email address.  :-)

Whoops, thanks for the heads-up :)

>> Subject: [PATCH] respect slot allocation, e.g. for <read-only-slot>
>>
>>      * libguile/goops.c (get_slot_value, set_slot_value): In the struct
>>      allocation case, don't poke the slots array directly -- we should
>>      go through struct-ref/struct-set! code so that we get the
>>      permissions and allocation ('u' versus 'p') correct.
>
> This one is of course correct, but I'm concerned about the performance
> implications of all this permission checking.  `scm_struct_ref ()' is
> not trivial, and it's not inlined, etc.  Maybe some preprocessing could
> be made at vtable creation time so that we don't have to reinterpret the
> whole layout string at each ref/set?

Well, there are opportunities for improvement.

I have heard that in a well-done CLOS implementation, accessors are are
generally faster than slot-ref, because they can perform this class/slot
allocation calculation once, and have the method implementation just
dispatch using that table, compiling down to struct-ref instead of
slot-ref. So working this into accessors and method compilation would be
one way to do this.

That would also avoid the linear search of the class' slot definition
list that happens on every ref/set!.

Another option would be, as you say, storing more information in the
classes. But on the other hand what we currently have strikes a fine
balance, and is extensible via compute-get-n-set. I don't see GOOPS
objects suddenly becoming faster than structs themselves. But perhaps
benchmarking is really the way to go here.

>> * libguile/struct.c (scm_struct_ref, scm_struct_set_x): "Light" structs
>> have no hidden words (members of the SCM_STRUCT_DATA(x) array accessed
>> with negative indices). In that case, determine the number of fields
>> from the length of the struct layout descriptor. (Most GOOPS instances
>> are light structs.)
>
> Is there a simple test case that reproduces this?  For instance, are
> instances of <class> light structs?

Well, with the old code:

(define-class <foo> () (bar #:init-keyword #:bar))
(define x (make <foo> #:bar 1))

In theory, the struct has one slot, 'bar, stored in slot 0 of its
struct.

(struct? x) => #t

But:

(struct-ref x 0) => undefined

Because struct-ref reads `nfields' from one of the hidden words.

For some crazy reason, only instances with 1 or more slots are set to be
"light", so be sure to define a slot when you are testing this. Also,
only pure instances will be light: instances of <class> are classes, so
they will not be light.

To be honest I do not understand the light/non-light thing; to me all
instances should be what is called "light" and all vtables should also
be "light", but hold the necessary information in normal (i.e.,
non-negatively indexed) slots of the struct. It seems the difference
only exists because historically things were one way, and the better way
did not have time to fully supplant the old.

Cheers,

Andy
-- 
http://wingolog.org/