reflect changes

48 views
Skip to first unread message

Russ Cox

unread,
Mar 19, 2011, 12:49:17 AM3/19/11
to golang-dev
I have been talking vaguely about reflect changes for
a while. Here are some specifics. This is the first half,
the changes to reflect.Type. The changes to reflect.Value
would be similar.

http://codereview.appspot.com/4281055 reflect: changes
http://codereview.appspot.com/4301043 update tree for reflect changes
http://codereview.appspot.com/4285053 gofix: reflect changes

This is not a request for a code review yet, just a status update.

Russ

unread,
Mar 19, 2011, 1:49:21 PM3/19/11
to golang-dev
By random chance, I clicked on this post. I have read the first patch,
but I don't understand the reasons behind the proposed changes to the
"reflect" package. These are the points I don't understand:

- The new version seems to have no advantages over the previous
version.

- The compile-time safety of code using the new version is going to be
worse than with the old version. The new version of the interface
"Type" includes methods such as "Field(i int)" even if the type-object
is not a struct. The new version makes type switches impossible.

- The non-public nature of the types "funcType", "arrayType", etc,
will negatively affect performance.

So, I was wondering whether you are able to explain why the new
version is believed to be better than the old one.

On Mar 19, 4:49 am, Russ Cox <r...@golang.org> wrote:
> I have been talking vaguely about reflect changes for
> a while.  Here are some specifics.  This is the first half,
> the changes to reflect.Type.  The changes to reflect.Value
> would be similar.
>
> http://codereview.appspot.com/4281055 reflect: changeshttp://codereview.appspot.com/4301043 update tree for reflect changeshttp://codereview.appspot.com/4285053 gofix: reflect changes

Russ Cox

unread,
Mar 19, 2011, 9:21:27 PM3/19/11
to ⚛, golang-dev
> - The new version seems to have no advantages over the previous
> version.

Sure it does. It completely hides the implementation, so that
there is more flexibility in what the implementation is.
I believe the current API exposes too much.

If we decide to allow copying of structs with unexported fields,
we will not be able to expose the *ChanType and so on safely,
since clients could overwrite their contents. So we have to
hide everything behind an interface value.

The main motivation is that a similar change to Value's API
makes Value something (a struct with value methods) that can
be created, copied, and used with no memory allocations.
That will change the performance of reflect significantly, for the better.
No matter how good your garbage collector is, no allocations
beats some allocations.

I started with Type because it is the more elemental of the two.
I definitely want to have similar APIs for Type and Value.
I'll send the change out for an actual code review once both
are done.

> - The compile-time safety of code using the new version is going to be
> worse than with the old version. The new version of the interface
> "Type" includes methods such as "Field(i int)" even if the type-object
> is not a struct. The new version makes type switches impossible.

Yes, you switch on t.Kind() instead. The code is shorter and simpler
and has just as many checks. It is true that the compiler does less
static checking, but if you look at actual reflect code, the compiler
is not doing much anyway: most uses are littered with type assertions
that have no effect at compile time other than to silence the compiler,
clutter the code, and delay the checks. If clients are going to delay the
checks, we might as well remove the clutter.

I don't believe the compile-time type checking is giving much benefit
here, certainly not in proportion to how verbose the code is.

> - The non-public nature of the types "funcType", "arrayType", etc,
> will negatively affect performance.

I don't know what this means. I don't believe the indirect call
instruction is a significant performance problem.

Russ

Florian Weimer

unread,
Mar 20, 2011, 2:49:39 PM3/20/11
to golang-dev
* Russ Cox:

> I have been talking vaguely about reflect changes for
> a while. Here are some specifics. This is the first half,
> the changes to reflect.Type. The changes to reflect.Value
> would be similar.

Would it be possible to remove the unexported function uncommon() from
Type? Other packages could then implement Type, notably go/ast. I've
used a similar approach in Java, in order to be able to use the same
code for compile-time and run-time annotation validation.

Russ Cox

unread,
Mar 20, 2011, 11:22:45 PM3/20/11
to Florian Weimer, golang-dev
> Would it be possible to remove the unexported function uncommon() from
> Type?  Other packages could then implement Type, notably go/ast.  I've
> used a similar approach in Java, in order to be able to use the same
> code for compile-time and run-time annotation validation.

That's an interesting suggestion. It could not be allowed to
pass another implementation of Type back to reflect for use
in PtrTo or MakeZero or any of the other routines that take
a Type, because reflect's implementation depends on being
able to use the various fields directly, so the benefit would
only be to other code that doesn't yet exist. That code
could always define its own interface to start, since Go doesn't
need "implements", so I think we can leave it out until the need
is demonstrated. Still, it's intriguing.

Russ

unread,
Mar 21, 2011, 3:12:00 AM3/21/11
to golang-dev
On Mar 20, 1:21 am, Russ Cox <r...@golang.org> wrote:
> > - The new version seems to have no advantages over the previous
> > version.
>
> Sure it does.  It completely hides the implementation, so that
> there is more flexibility in what the implementation is.
> I believe the current API exposes too much.
>
> If we decide to allow copying of structs with unexported fields,
> we will not be able to expose the *ChanType and so on safely,
> since clients could overwrite their contents.  So we have to
> hide everything behind an interface value.

Why not create a "ChanType" interface whose methodset would include
all methods from the less concrete "Type" interface?

> The main motivation is that a similar change to Value's API
> makes Value something (a struct with value methods) that can
> be created, copied, and used with no memory allocations.
> That will change the performance of reflect significantly, for the better.
> No matter how good your garbage collector is, no allocations
> beats some allocations.

Ok, I will be looking forward to seeing the new Value API.

> I started with Type because it is the more elemental of the two.
> I definitely want to have similar APIs for Type and Value.
> I'll send the change out for an actual code review once both
> are done.
>
> > - The compile-time safety of code using the new version is going to be
> > worse than with the old version. The new version of the interface
> > "Type" includes methods such as "Field(i int)" even if the type-object
> > is not a struct. The new version makes type switches impossible.
>
> Yes, you switch on t.Kind() instead.  The code is shorter and simpler
> and has just as many checks.  It is true that the compiler does less
> static checking, but if you look at actual reflect code, the compiler
> is not doing much anyway: most uses are littered with type assertions
> that have no effect at compile time other than to silence the compiler,
> clutter the code, and delay the checks.  If clients are going to delay the
> checks, we might as well remove the clutter.
>
> I don't believe the compile-time type checking is giving much benefit
> here, certainly not in proportion to how verbose the code is.
>
> > - The non-public nature of the types "funcType", "arrayType", etc,
> > will negatively affect performance.
>
> I don't know what this means.  I don't believe the indirect call
> instruction is a significant performance problem.

What I meant was: information hiding, and ability to recover hidden
information. There should be a way of how to get back some of the
"lost bits" of information (i.e: to unhide the hidden information) in
the user code at compile-time and to store that information for later
reuse by the compiler. This has implications for both safety and run-
time performance.

Russ Cox

unread,
Mar 21, 2011, 9:44:10 AM3/21/11
to ⚛, golang-dev
>> If we decide to allow copying of structs with unexported fields,
>> we will not be able to expose the *ChanType and so on safely,
>> since clients could overwrite their contents.  So we have to
>> hide everything behind an interface value.
>
> Why not create a "ChanType" interface whose methodset would include
> all methods from the less concrete "Type" interface?

You can do that but can't use it in a type switch too reliably.
It forces different concrete types behind the scenes.
For example, if all Types were implemented as just one
*reflect.commonType in an interface, then a type check to
see if a Type implemented ChanType would be true of
all Types - including, say, those corresponding to pointers -
not just the ones that are channels.

Also some interfaces would be subsets of others anyway.
For example the interface for SliceType (Elem) is a subset of the
interface for ArrayType (Elem, Len). A type check to see if
a Type implemented SliceType would not guarantee that
the Type was itself a Slice.

> What I meant was: information hiding, and ability to recover hidden
> information. There should be a way of how to get back some of the
> "lost bits" of information (i.e: to unhide the hidden information) in
> the user code at compile-time and to store that information for later
> reuse by the compiler. This has implications for both safety and run-
> time performance.

I'm still not sure what you mean. The information I see being hidden
is the concrete type information, but it is available from t.Kind(), so not
really hidden. It is true that it is not available at compile time and so
cannot be checked at compile time anymore. We would be making a
conscious decision to give that up, on the basis that it is hurting
and being worked around more than it is helping.

Russ

unread,
Mar 21, 2011, 2:36:30 PM3/21/11
to golang-dev
On Mar 21, 1:44 pm, Russ Cox <r...@golang.org> wrote:
> >> If we decide to allow copying of structs with unexported fields,
> >> we will not be able to expose the *ChanType and so on safely,
> >> since clients could overwrite their contents.  So we have to
> >> hide everything behind an interface value.
>
> > Why not create a "ChanType" interface whose methodset would include
> > all methods from the less concrete "Type" interface?
>
> You can do that but can't use it in a type switch too reliably.
> It forces different concrete types behind the scenes.
> For example, if all Types were implemented as just one
> *reflect.commonType in an interface, then a type check to
> see if a Type implemented ChanType would be true of
> all Types - including, say, those corresponding to pointers -
> not just the ones that are channels.

One thing is that the code change you mentioned at http://codereview.appspot.com/4281055
instead of looking like this:

- if _, ok := v.Type().(*reflect.MapType).Key().
(*reflect.StringType); !ok {
+ if v.Type().Key().Kind() != reflect.String {

could alternatively look like this:

- if _, ok := v.Type().(*reflect.MapType).Key().
(*reflect.StringType); !ok {
+ if v.MapType().Key().Kind() != reflect.String {

The latter is assuming that the methodset of "MapValue" contains
method "MapType() reflect.MapType". In other words, if the programmer
knows that "v" is a "MapValue", then it is possible to call
"v.MapType()". Note that the code "v.MapType()" is safe (it cannot
cause a panic), while the code "v.Type().Key()" is not safe. Whether
"v.Type().Key()" panics or not depends on whether the programmer will
make a programming mistake - and as we all know, programmers do make
mistakes.

> Also some interfaces would be subsets of others anyway.
> For example the interface for SliceType (Elem) is a subset of the
> interface for ArrayType (Elem, Len).  A type check to see if
> a Type implemented SliceType would not guarantee that
> the Type was itself a Slice.

Indeed. The problem is that SliceType and ArrayType should be
distinct. Wouldn't it be possible to achieve it using the following
trick:

type SliceType interface {
sliceType_magicMethod()
...
}

type ArrayType interface {
arrayType_magicMethod()
...
}

This would make the methodsets of SliceType and ArrayType distinct,
while still allowing for both of them to implement the Type interface.

> > What I meant was: information hiding, and ability to recover hidden
> > information. There should be a way of how to get back some of the
> > "lost bits" of information (i.e: to unhide the hidden information) in
> > the user code at compile-time and to store that information for later
> > reuse by the compiler. This has implications for both safety and run-
> > time performance.
>
> I'm still not sure what you mean.  The information I see being hidden
> is the concrete type information, but it is available from t.Kind(), so not
> really hidden.  It is true that it is not available at compile time and so
> cannot be checked at compile time anymore.  We would be making a
> conscious decision to give that up, on the basis that it is hurting
> and being worked around more than it is helping.
>
> Russ

I think you are very close to what I meant.

The method "t.Kind()" returns a mere numeric value. This numeric value
has no relation to anything in the type system. Thus, if you execute
"t.Kind()", the compiler does not understand what just happened. Since
Go is a safe language, the compiler will *have* to make another run-
time check to make sure that "t" has the concrete type specified by
the programmer (such as in: t.(*arrayType)).

If you on the other hand execute a type switch, the compiler performs
an action which is pretty much *equivalent* to the run-time check
mentioned in the previous sentence.

The compiler will do the run-time thing in *both* cases. This takes X
nanoseconds. In the former case the program will also execute
"t.Kind()", which will take Y nanoseconds. Comparing (X) to (X+Y), it
is obvious which one is faster. In addition to the (X+Y) case being
slower, it also gives less compile-time guarantees. So, it appears to
be both slower and less safe.

Now, I agree the above does not really apply to the suggested new
version of "reflect", but that is only because it uses the "unsafe"
package. For example, the function "func (t *commonType) Len() int"
contains the code "tt := (*arrayType)(unsafe.Pointer(t))".

Russ Cox

unread,
Mar 21, 2011, 2:47:32 PM3/21/11
to ⚛, golang-dev
I understand that we could make reflect communicate
more information to the compiler. That's what we have now.
It's very verbose and in many cases the code defers the
real checks to run time anyway (by using unchecked type
assertions). I think reflect is sufficiently different from most
packages - its whole point is to be dynamic, to defer things
until run time - that giving up the few compile time checks
that remain is not a problem. Let's put this discussion to
the side until Value is ready for review.

Russ

Reply all
Reply to author
Forward
0 new messages