struct optimizations

struct optimizations

[Andy Wingo]

Hi Ludovic,

Great work on the struct optimizations!

Some comments:

    -#define SCM_VTABLE_FLAG_RESERVED_0 (1L << 5)
    -#define SCM_VTABLE_FLAG_RESERVED_1 (1L << 6)
    +#define SCM_VTABLE_FLAG_SIMPLE (1L << 5) /* instances of this vtable have only "pr" fields */
    +#define SCM_VTABLE_FLAG_SIMPLE_RW (1L << 6) /* instances of this vtable have only "pw" fields */

The comments do not appear to be correct, given a later check:

    +  if (SCM_LIKELY (SCM_VTABLE_FLAG_IS_SET (vtable, SCM_VTABLE_FLAG_SIMPLE)
    +  		  && SCM_VTABLE_FLAG_IS_SET (vtable, SCM_VTABLE_FLAG_SIMPLE_RW)
    +  		  && p < SCM_STRUCT_DATA_REF (vtable, scm_vtable_index_size)))
    +    /* The fast path: HANDLE is a struct with only "p" fields.  */
    +    data[p] = SCM_UNPACK (val);

It seems that currently SIMPLE is for all pr *or* all pw slots. But we
should be more orthogonal than that; let's have SIMPLE be for having all
slots be readable p slots. (Note that this still allows a mix of
readable and writable slots.)

Then perhaps we can change SIMPLE_RW to be MUTABLE or something, to
indicate that all slots of this object are mutable.

What do you think?

Andy
-- 
http://wingolog.org/

Re: struct optimizations

[Ludovic Courtès]

Hi Andy,

Andy Wingo <wingo@pobox.com> writes:

> Some comments:
>
>     -#define SCM_VTABLE_FLAG_RESERVED_0 (1L << 5)
>     -#define SCM_VTABLE_FLAG_RESERVED_1 (1L << 6)
>     +#define SCM_VTABLE_FLAG_SIMPLE (1L << 5) /* instances of this vtable have only "pr" fields */
>     +#define SCM_VTABLE_FLAG_SIMPLE_RW (1L << 6) /* instances of this vtable have only "pw" fields */
>
> The comments do not appear to be correct, given a later check:
>
>     +  if (SCM_LIKELY (SCM_VTABLE_FLAG_IS_SET (vtable, SCM_VTABLE_FLAG_SIMPLE)
>     +  		  && SCM_VTABLE_FLAG_IS_SET (vtable, SCM_VTABLE_FLAG_SIMPLE_RW)
>     +  		  && p < SCM_STRUCT_DATA_REF (vtable, scm_vtable_index_size)))
>     +    /* The fast path: HANDLE is a struct with only "p" fields.  */
>     +    data[p] = SCM_UNPACK (val);
>
> It seems that currently SIMPLE is for all pr *or* all pw slots.

True.

> But we should be more orthogonal than that; let's have SIMPLE be for
> having all slots be readable p slots. (Note that this still allows a
> mix of readable and writable slots.)
>
> Then perhaps we can change SIMPLE_RW to be MUTABLE or something, to
> indicate that all slots of this object are mutable.
>
> What do you think?

I agree and I’ll look into it.

Thanks for the review!

Ludo’.

Re: struct optimizations

[Ludovic Courtès]

Hello,

Andy Wingo <wingo@pobox.com> writes:

> Some comments:
>
>     -#define SCM_VTABLE_FLAG_RESERVED_0 (1L << 5)
>     -#define SCM_VTABLE_FLAG_RESERVED_1 (1L << 6)
>     +#define SCM_VTABLE_FLAG_SIMPLE (1L << 5) /* instances of this vtable have only "pr" fields */
>     +#define SCM_VTABLE_FLAG_SIMPLE_RW (1L << 6) /* instances of this vtable have only "pw" fields */
>
> The comments do not appear to be correct, given a later check:
>
>     +  if (SCM_LIKELY (SCM_VTABLE_FLAG_IS_SET (vtable, SCM_VTABLE_FLAG_SIMPLE)
>     +  		  && SCM_VTABLE_FLAG_IS_SET (vtable, SCM_VTABLE_FLAG_SIMPLE_RW)
>     +  		  && p < SCM_STRUCT_DATA_REF (vtable, scm_vtable_index_size)))
>     +    /* The fast path: HANDLE is a struct with only "p" fields.  */
>     +    data[p] = SCM_UNPACK (val);
>
> It seems that currently SIMPLE is for all pr *or* all pw slots. But we
> should be more orthogonal than that; let's have SIMPLE be for having all
> slots be readable p slots. (Note that this still allows a mix of
> readable and writable slots.)

Fixed:

  http://git.sv.gnu.org/cgit/guile.git/commit/?id=e03b7f73e2927178f2d9485320435edb6260c311

> Then perhaps we can change SIMPLE_RW to be MUTABLE or something, to
> indicate that all slots of this object are mutable.

I ended up keeping the name ‘SIMPLE_RW’ because in theory there could be
non-‘p’ mutable fields, i.e., non-simple structs with mutable fields,
which aren’t interesting.

Thanks,
Ludo’.

Re: struct optimizations

[Andy Wingo]

Hi Ludo,

On Tue 26 Jan 2010 00:13, ludo@gnu.org (Ludovic Courtès) writes:

>> It seems that currently SIMPLE is for all pr *or* all pw slots.
>
> Fixed:

Cool :)

>> Then perhaps we can change SIMPLE_RW to be MUTABLE or something, to
>> indicate that all slots of this object are mutable.
>
> I ended up keeping the name ‘SIMPLE_RW’ because in theory there could be
> non-‘p’ mutable fields, i.e., non-simple structs with mutable fields,
> which aren’t interesting.

Wellll, not right now, no; but SIMPLE | MUTABLE is the same as your
current semantics of SIMPLE_RW. I can imagine MUTABLE being useful on
its own. But no, it's not useful right now, and should it become useful,
we can play some #define trickery.

Andy
-- 
http://wingolog.org/