New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: opt: reuse interface payload during interface conversion #24582

Open
alandonovan opened this Issue Mar 28, 2018 · 6 comments

Comments

Projects
None yet
4 participants
@alandonovan
Contributor

alandonovan commented Mar 28, 2018

The current representation of interface values is a two-word (type, data) pair where data is always a reference; multiword values such as strings must be copied to a hidden variable whose address is saved in the data field. The compiler is currently smart about avoiding unnecessary allocations in some cases, such as this one:

func f(x interface{}) interface{} {
    return x.(string)
}

However, if the interface type has methods, the compiler does not appear to perform this optimization:

type S string
func (S) f() {}
type I interface{ f() }
func f(x interface{}) I {
        return x.(S) // calls runtime.convT2Istring
}

It would be nice if it did; I suspect the hard part is mostly done already.

The fact that assigning a string value to an interface variable allocates memory caught me out when I first noticed it in a profile.

@josharian josharian added this to the Unplanned milestone Mar 28, 2018

@randall77

This comment has been minimized.

Contributor

randall77 commented Mar 29, 2018

In your first example, the compiler does not avoid an allocation. It does still call convT2Estring.
I don't see any significant difference in the generated code in these two scenarios.

I'm confused as to why you have code like this. You are converting from an interface to a concrete type and then back to an interface. Why not just convert directly from interface A to interface B? That would avoid the allocation.

@ns-cweber

This comment has been minimized.

ns-cweber commented Mar 29, 2018

Why not just convert directly from interface A to interface B? That would avoid the allocation.

Conceivably one might want to do something S-specific before returning the S as an I? Seems like a legitimate thing to do although I lack a concrete example.

@randall77

This comment has been minimized.

Contributor

randall77 commented Mar 29, 2018

Sure, in general this can happen. But if the original interface is available you can always go the direct route. In this case, just return x instead of (interface{})(x.(string)) in the first example and x.(I) instead of x.(S).(I) in the second.

@alandonovan

This comment has been minimized.

Contributor

alandonovan commented Mar 29, 2018

In your first example, the compiler does not avoid an allocation. It does still call convT2Estring.

You're right. I wrote the first example without looking at the generated assembly because last time I checked the compiler was applying this optimization, but now that I check, it indeed is not, and go1.9's gc doesn't seem to do so either. Sorry for the confusion. Apparently the optimization is a figment of my imagination. So, let me change this issue to: please add this optimization for both cases.

As to why it's needed, consider functions of this form that visit a tree of values:

func f(x interface{}) interface{} {
  switch y := x.(type) {
  case int:
     return y
  case string:
      return y
  case []interface{}:
     return ...[f(e) for e in y]...
  } 
}

Such a function need not allocate a new string header for each leaf in the tree, but that's what it does.
Of course, you can avoid the allocation by returning x instead of y in the string case, but it's very subtle, especially if you're doing other things to x in that case.

@randall77

This comment has been minimized.

Contributor

randall77 commented Mar 29, 2018

Of course, you can avoid the allocation by returning x instead of y in the string case, but it's very subtle,

I guess I don't see this as that subtle. Returning x is clear that no work is needed. Returning y might take work; it only appears as free because the string->interface{} cast is implicit.

What would the optimization entail? We'd have to notice that at a concrete type -> interface type conversion, the concrete type came from an interface -> concrete conversion, and forward the data pointer. Doesn't seem too hard in the abstract. Concretely it might be kinda tricky at the moment. Forwarding like that is easy in SSA but hard on the AST. Unfortunately, lowering interface conversions to runtime calls happens before converting to SSA. So it might involve moving interface conversions into SSA.

@ns-cweber

This comment has been minimized.

ns-cweber commented Apr 3, 2018

But if the original interface is available you can always go the direct route.

Good point.

Doesn't seem too hard in the abstract. Concretely it might be kinda tricky at the moment.

That's fair. It seems like anything that eliminates an unnecessary implicit allocation is a good thing, but maybe returning the concrete value is infrequent anyway?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment