code review 98510044: runtime: convert interface routines from C to Go.

runtime: convert interface routines from C to Go.

[khr, 2014-08-03 21:25:56]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://khr%40golang.org@code.google.com/p/go/

[dave_cheney.net, 2014-08-04 06:25:28]

On 2014/08/03 21:25:56, khr wrote:
> Hello mailto:golang-codereviews@googlegroups.com,
> 
> I'd like you to review this change to
> https://khr%40golang.org@code.google.com/p/go/

Test's pass on linux/arm, and performance is improved in some cases, nice!

benchmark                          old ns/op       new ns/op       delta       
BenchmarkBinaryTree17              64535797119     64597564697     +0.10%      
BenchmarkFannkuch11                53078521729     54127319336     +1.98%      
BenchmarkFmtFprintfEmpty           1471            1359            -7.61%      
BenchmarkFmtFprintfString          4706            4085            -13.20%     
BenchmarkFmtFprintfInt             3126            3087            -1.25%      
BenchmarkFmtFprintfIntInt          4818            4876            +1.20%      
BenchmarkFmtFprintfPrefixedInt     4813            4633            -3.74%      
BenchmarkFmtFprintfFloat           11839           10318           -12.85%     
BenchmarkFmtManyArgs               22192           21423           -3.47%      
BenchmarkGobDecode                 257550048       248046875       -3.69%      
BenchmarkGobEncode                 194787597       195220947       +0.22%      
BenchmarkGzip                      6218627929      6206115722      -0.20%      
BenchmarkGunzip                    929061889       962463379       +3.60%      
BenchmarkHTTPClientServer          626876          619613          -1.16%      
BenchmarkJSONEncode                504257202       497772216       -1.29%      
BenchmarkJSONDecode                1731201172      1666778564      -3.72%      
BenchmarkMandelbrot200             98226928        97607421        -0.63%      
BenchmarkGoParse                   73983764        73570251        -0.56%      
BenchmarkRegexpMatchEasy0_32       1964            2008            +2.24%      
BenchmarkRegexpMatchEasy0_1K       7249            7357            +1.49%      
BenchmarkRegexpMatchEasy1_32       1919            2055            +7.09%      
BenchmarkRegexpMatchEasy1_1K       17172           17124           -0.28%      
BenchmarkRegexpMatchMedium_32      3143            3264            +3.85%      
BenchmarkRegexpMatchMedium_1K      1044662         1037658         -0.67%      
BenchmarkRegexpMatchHard_32        58115           58126           +0.02%      
BenchmarkRegexpMatchHard_1K        1790496         1786224         -0.24%      
BenchmarkRevcomp                   143634033       140875244       -1.92%      
BenchmarkTemplate                  2399627685      2244934082      -6.45%      
BenchmarkTimeParse                 9894            9536            -3.62%      
BenchmarkTimeFormat                17581           17057           -2.98%      

benchmark                         old MB/s     new MB/s     speedup     
BenchmarkGobDecode                2.98         3.09         1.04x       
BenchmarkGobEncode                3.94         3.93         1.00x       
BenchmarkGzip                     3.12         3.13         1.00x       
BenchmarkGunzip                   20.89        20.16        0.97x       
BenchmarkJSONEncode               3.85         3.90         1.01x       
BenchmarkJSONDecode               1.12         1.16         1.04x       
BenchmarkGoParse                  0.78         0.79         1.01x       
BenchmarkRegexpMatchEasy0_32      16.29        15.93        0.98x       
BenchmarkRegexpMatchEasy0_1K      141.26       139.17       0.99x       
BenchmarkRegexpMatchEasy1_32      16.67        15.57        0.93x       
BenchmarkRegexpMatchEasy1_1K      59.63        59.80        1.00x       
BenchmarkRegexpMatchMedium_32     0.32         0.31         0.97x       
BenchmarkRegexpMatchMedium_1K     0.98         0.99         1.01x       
BenchmarkRegexpMatchHard_32       0.55         0.55         1.00x       
BenchmarkRegexpMatchHard_1K       0.57         0.57         1.00x       
BenchmarkRevcomp                  17.70        18.04        1.02x       
BenchmarkTemplate                 0.81         0.86         1.06x

[bradfitz, 2014-08-04 17:45:40]

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go
File src/pkg/runtime/iface.go (right):

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:16: var ifaceLock lock
what does this guard?  (I know this is just porting from iface.goc where it also
didn't have comments, but maybe in the rewrite to Go we can change styles and
document things?)

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:37: panic(&TypeAssertionError{"", *typ._string,
*inter._string, *i.name})
optional comment, as this is low-level code anyway, but in code reviews, we
normally tell people not to use untagged struct literals when the struct has
multiple fields of the same type. In this case, 4 strings.  because then if the
struct is rearranged, this continues to compile silently.

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:49: var locked int
could be a bool now instead of int

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:109: panic(&TypeAssertionError{"", *typ._string,
*inter._string, *iname})
likewise

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:277: panic(&TypeAssertionError{"", "", *inter._string,
""})
this might actually be shorter with a tagged struct

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:372: panic(&TypeAssertionError{"", "", *inter._string,
""})
likewise

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:386: ok = false
unnecessary. already zero.

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:393: ok = false
also unnecessary

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:410: panic(&TypeAssertionError{"", "", *inter._string,
""})
tagged

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:428: if *t._string == *p2._type._string {
this seems new and an unnecessary cost? or was it there before somewhere?

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:452: if *p1.tab._type._string == *p2.tab._type._string
{
likewise. leftover debugging?

[dave_cheney.net, 2014-08-05 02:29:44]

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go
File src/pkg/runtime/iface.go (right):

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:161: memmove(x, elem, size)
newobject zeroes the memory returned by x, when we overwrite it with elem. Is
there a non zeroing version available ?

[dvyukov, 2014-08-05 08:52:22]

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go
File src/pkg/runtime/iface.go (right):

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:15: var hash [hashSize]*itab
var (
    ...
)

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:21: type fInterface interface {
can't we continue to use iface/eface?
we already have these types in zruntime_defs, and you seems to just cast
fInterface to iface in all functions
there are lots of code that uses iface/eface naming, and it looks like a good
and simple naming convention to me

https://codereview.appspot.com/98510044/diff/200001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:116: panic("invalid itab locking")
I see that we panic in C code as well, but this really must be gothrow.

[khr1, 2014-08-06 21:57:32]

On Mon, Aug 4, 2014 at 10:45 AM, <bradfitz@golang.org> wrote:

>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go
> File src/pkg/runtime/iface.go (right):
>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode16
> src/pkg/runtime/iface.go:16: var ifaceLock lock
> what does this guard?  (I know this is just porting from iface.goc where
> it also didn't have comments, but maybe in the rewrite to Go we can
> change styles and document things?)
>
>
Done.


> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode37
> src/pkg/runtime/iface.go:37: panic(&TypeAssertionError{"", *typ._string,
> *inter._string, *i.name})
> optional comment, as this is low-level code anyway, but in code reviews,
> we normally tell people not to use untagged struct literals when the
> struct has multiple fields of the same type. In this case, 4 strings.
> because then if the struct is rearranged, this continues to compile
> silently.
>
>
Let me convert to tagged literals as a separate CL.  For this CL we can
just verify that the list is the same, in a separate CL we can verify that
the right names are given to the right parts.


> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode49
> src/pkg/runtime/iface.go:49: var locked int
> could be a bool now instead of int
>
>
You mean like
   for _, locked := range [2]bool{false,true}?
I'm not sure it is worth it.


> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode109
> src/pkg/runtime/iface.go:109: panic(&TypeAssertionError{"",
> *typ._string, *inter._string, *iname})
> likewise
>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode277
> src/pkg/runtime/iface.go:277: panic(&TypeAssertionError{"", "",
> *inter._string, ""})
> this might actually be shorter with a tagged struct
>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode372
> src/pkg/runtime/iface.go:372: panic(&TypeAssertionError{"", "",
> *inter._string, ""})
> likewise
>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode386
> src/pkg/runtime/iface.go:386: ok = false
> unnecessary. already zero.
>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode393
> src/pkg/runtime/iface.go:393: ok = false
> also unnecessary
>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode410
> src/pkg/runtime/iface.go:410: panic(&TypeAssertionError{"", "",
> *inter._string, ""})
> tagged
>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode428
> src/pkg/runtime/iface.go:428: if *t._string == *p2._type._string {
> this seems new and an unnecessary cost? or was it there before
> somewhere?
>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode452
> src/pkg/runtime/iface.go:452: if *p1.tab._type._string ==
> *p2.tab._type._string {
> likewise. leftover debugging?
>
>
Thanks, deleted those two.


> https://codereview.appspot.com/98510044/
>

[khr1, 2014-08-06 22:03:47]

On Mon, Aug 4, 2014 at 7:29 PM, <dave@cheney.net> wrote:

>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go
> File src/pkg/runtime/iface.go (right):
>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode161
> src/pkg/runtime/iface.go:161: memmove(x, elem, size)
> newobject zeroes the memory returned by x, when we overwrite it with
> elem. Is there a non zeroing version available ?
>
>
Not at the moment.  The problem is we run GC at the end of a malloc, so if
GC happened to trigger during allocation it would see an uninitialized
object.  Perhaps we could get around that by having a malloc+init call for
use by the runtime.

We have an allocator to get uninitialized memory (rawmem + friends), but it
returns unscannable memory.  We can't use that here (in general, at least).

Add it to the list of optimizations to be done.


> https://codereview.appspot.com/98510044/
>

[khr1, 2014-08-06 22:10:07]

On Tue, Aug 5, 2014 at 1:52 AM, <dvyukov@google.com> wrote:

>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go
> File src/pkg/runtime/iface.go (right):
>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode15
> src/pkg/runtime/iface.go:15: var hash [hashSize]*itab
> var (
>     ...
> )
>
> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode21
> src/pkg/runtime/iface.go:21: type fInterface interface {
> can't we continue to use iface/eface?
> we already have these types in zruntime_defs, and you seems to just cast
> fInterface to iface in all functions
> there are lots of code that uses iface/eface naming, and it looks like a
> good and simple naming convention to me
>
>
This is the place where we can't use iface or eface, as we're declaring an
interface whose data element might not be a pointer.


> https://codereview.appspot.com/98510044/diff/200001/src/
> pkg/runtime/iface.go#newcode116
> src/pkg/runtime/iface.go:116: panic("invalid itab locking")
> I see that we panic in C code as well, but this really must be gothrow.
>
>
Fixed.


> https://codereview.appspot.com/98510044/
>

[bradfitz, 2014-08-06 22:21:34]

https://codereview.appspot.com/98510044/diff/280001/src/pkg/runtime/iface.go
File src/pkg/runtime/iface.go (right):

https://codereview.appspot.com/98510044/diff/280001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:17: ifaceLock lock // lock for accessing hash
we generally put locks before the fields they guard.

[bradfitz, 2014-08-06 22:21:49]

On Wed, Aug 6, 2014 at 2:57 PM, Keith Randall <khr@google.com> wrote:

>
>
>
> On Mon, Aug 4, 2014 at 10:45 AM, <bradfitz@golang.org> wrote:
>
>>
>> https://codereview.appspot.com/98510044/diff/200001/src/
>> pkg/runtime/iface.go
>> File src/pkg/runtime/iface.go (right):
>>
>> https://codereview.appspot.com/98510044/diff/200001/src/
>> pkg/runtime/iface.go#newcode16
>> src/pkg/runtime/iface.go:16: var ifaceLock lock
>> what does this guard?  (I know this is just porting from iface.goc where
>> it also didn't have comments, but maybe in the rewrite to Go we can
>> change styles and document things?)
>>
>>
> Done.
>
>
>> https://codereview.appspot.com/98510044/diff/200001/src/
>> pkg/runtime/iface.go#newcode37
>> src/pkg/runtime/iface.go:37: panic(&TypeAssertionError{"", *typ._string,
>> *inter._string, *i.name})
>> optional comment, as this is low-level code anyway, but in code reviews,
>> we normally tell people not to use untagged struct literals when the
>> struct has multiple fields of the same type. In this case, 4 strings.
>> because then if the struct is rearranged, this continues to compile
>> silently.
>>
>>
> Let me convert to tagged literals as a separate CL.  For this CL we can
> just verify that the list is the same, in a separate CL we can verify that
> the right names are given to the right parts.
>
>
>> https://codereview.appspot.com/98510044/diff/200001/src/
>> pkg/runtime/iface.go#newcode49
>> src/pkg/runtime/iface.go:49: var locked int
>> could be a bool now instead of int
>>
>>
> You mean like
>    for _, locked := range [2]bool{false,true}?
> I'm not sure it is worth it.
>

Yeah. I think it's more readable (especially not having to read those != 0
and wonder what other values it may take), but I write almost exclusively
in Go.

I figure we might as well use the language if we get to use it now.

But I don't care that much.

[khr1, 2014-08-06 22:35:46]

I think I'll leave it as is.  It gets called during most type conversions
and I don't want to initialize and then load from a [2]bool when the index
already has the right value.

After checking this in I'll try it the Go way and if there's no performance
degredation we can use it.


On Wed, Aug 6, 2014 at 3:21 PM, Brad Fitzpatrick <bradfitz@golang.org>
wrote:

>
>
>
> On Wed, Aug 6, 2014 at 2:57 PM, Keith Randall <khr@google.com> wrote:
>
>>
>>
>>
>> On Mon, Aug 4, 2014 at 10:45 AM, <bradfitz@golang.org> wrote:
>>
>>>
>>> https://codereview.appspot.com/98510044/diff/200001/src/
>>> pkg/runtime/iface.go
>>> File src/pkg/runtime/iface.go (right):
>>>
>>> https://codereview.appspot.com/98510044/diff/200001/src/
>>> pkg/runtime/iface.go#newcode16
>>> src/pkg/runtime/iface.go:16: var ifaceLock lock
>>> what does this guard?  (I know this is just porting from iface.goc where
>>> it also didn't have comments, but maybe in the rewrite to Go we can
>>> change styles and document things?)
>>>
>>>
>> Done.
>>
>>
>>> https://codereview.appspot.com/98510044/diff/200001/src/
>>> pkg/runtime/iface.go#newcode37
>>> src/pkg/runtime/iface.go:37: panic(&TypeAssertionError{"", *typ._string,
>>> *inter._string, *i.name})
>>> optional comment, as this is low-level code anyway, but in code reviews,
>>> we normally tell people not to use untagged struct literals when the
>>> struct has multiple fields of the same type. In this case, 4 strings.
>>> because then if the struct is rearranged, this continues to compile
>>> silently.
>>>
>>>
>> Let me convert to tagged literals as a separate CL.  For this CL we can
>> just verify that the list is the same, in a separate CL we can verify that
>> the right names are given to the right parts.
>>
>>
>>> https://codereview.appspot.com/98510044/diff/200001/src/
>>> pkg/runtime/iface.go#newcode49
>>> src/pkg/runtime/iface.go:49: var locked int
>>> could be a bool now instead of int
>>>
>>>
>> You mean like
>>    for _, locked := range [2]bool{false,true}?
>> I'm not sure it is worth it.
>>
>
> Yeah. I think it's more readable (especially not having to read those != 0
> and wonder what other values it may take), but I write almost exclusively
> in Go.
>
> I figure we might as well use the language if we get to use it now.
>
> But I don't care that much.
>
>

[dave_cheney.net, 2014-08-06 22:55:37]

Thanks for your reply. Would you be able to add a short comment just above
that line to capture this as a TODO? I don't think it warrants an issue
being opened to track it.
On 7 Aug 2014 08:03, "Keith Randall" <khr@google.com> wrote:

>
>
>
> On Mon, Aug 4, 2014 at 7:29 PM, <dave@cheney.net> wrote:
>
>>
>> https://codereview.appspot.com/98510044/diff/200001/src/
>> pkg/runtime/iface.go
>> File src/pkg/runtime/iface.go (right):
>>
>> https://codereview.appspot.com/98510044/diff/200001/src/
>> pkg/runtime/iface.go#newcode161
>> src/pkg/runtime/iface.go:161: memmove(x, elem, size)
>> newobject zeroes the memory returned by x, when we overwrite it with
>> elem. Is there a non zeroing version available ?
>>
>>
> Not at the moment.  The problem is we run GC at the end of a malloc, so if
> GC happened to trigger during allocation it would see an uninitialized
> object.  Perhaps we could get around that by having a malloc+init call for
> use by the runtime.
>
> We have an allocator to get uninitialized memory (rawmem + friends), but
> it returns unscannable memory.  We can't use that here (in general, at
> least).
>
> Add it to the list of optimizations to be done.
>
>
>> https://codereview.appspot.com/98510044/
>>
>
>

[khr1, 2014-08-07 19:45:38]

Dmitry, could you take a final look?


On Wed, Aug 6, 2014 at 3:55 PM, Dave Cheney <dave@cheney.net> wrote:

> Thanks for your reply. Would you be able to add a short comment just above
> that line to capture this as a TODO? I don't think it warrants an issue
> being opened to track it.
> On 7 Aug 2014 08:03, "Keith Randall" <khr@google.com> wrote:
>
>>
>>
>>
>> On Mon, Aug 4, 2014 at 7:29 PM, <dave@cheney.net> wrote:
>>
>>>
>>> https://codereview.appspot.com/98510044/diff/200001/src/
>>> pkg/runtime/iface.go
>>> File src/pkg/runtime/iface.go (right):
>>>
>>> https://codereview.appspot.com/98510044/diff/200001/src/
>>> pkg/runtime/iface.go#newcode161
>>> src/pkg/runtime/iface.go:161: memmove(x, elem, size)
>>> newobject zeroes the memory returned by x, when we overwrite it with
>>> elem. Is there a non zeroing version available ?
>>>
>>>
>> Not at the moment.  The problem is we run GC at the end of a malloc, so
>> if GC happened to trigger during allocation it would see an uninitialized
>> object.  Perhaps we could get around that by having a malloc+init call for
>> use by the runtime.
>>
>> We have an allocator to get uninitialized memory (rawmem + friends), but
>> it returns unscannable memory.  We can't use that here (in general, at
>> least).
>>
>> Add it to the list of optimizations to be done.
>>
>>
>>> https://codereview.appspot.com/98510044/
>>>
>>
>>

[dvyukov, 2014-08-07 20:26:34]

https://codereview.appspot.com/98510044/diff/280001/src/pkg/runtime/stubs.go
File src/pkg/runtime/stubs.go (right):

https://codereview.appspot.com/98510044/diff/280001/src/pkg/runtime/stubs.go#...
src/pkg/runtime/stubs.go:86: func goatomicloadp(p unsafe.Pointer) unsafe.Pointer
    // return *p
sync, I've already added these

https://codereview.appspot.com/98510044/diff/280001/src/pkg/runtime/stubs.goc
File src/pkg/runtime/stubs.goc (right):

https://codereview.appspot.com/98510044/diff/280001/src/pkg/runtime/stubs.goc...
src/pkg/runtime/stubs.goc:96: x = runtime·persistentalloc(size, 0,
&mstats.other_sys);
how does it work?
runtime·persistentalloc is in C and not even nosplit and calls whole lot of
other functions

does #pragma textflag NOSPLIT work in goc files? why does not compiler complain?

https://codereview.appspot.com/98510044/diff/300001/src/pkg/runtime/iface.go
File src/pkg/runtime/iface.go (right):

https://codereview.appspot.com/98510044/diff/300001/src/pkg/runtime/iface.go#...
src/pkg/runtime/iface.go:29: panic("internal error - misuse of itab")
gothrow

[dvyukov, 2014-08-07 20:27:27]

it LGTM if I am missing something regarding persistentalloc, and it actually
works

[khr1, 2014-08-07 20:45:39]

Persistentalloc needs to move to the M stack.  I'll do that in a separate
CL.



On Thu, Aug 7, 2014 at 1:27 PM, <dvyukov@google.com> wrote:

> it LGTM if I am missing something regarding persistentalloc, and it
> actually works
>
> https://codereview.appspot.com/98510044/
>

[dvyukov, 2014-08-07 20:47:38]

OK, then LGTM with the gothrow fix and sync
But I am concerned that compiler does not complain about broken
NOSPLIT. Either the pragma does not work or  the pragma is broken.


On Fri, Aug 8, 2014 at 12:45 AM, Keith Randall <khr@google.com> wrote:
> Persistentalloc needs to move to the M stack.  I'll do that in a separate
> CL.
>
>
>
> On Thu, Aug 7, 2014 at 1:27 PM, <dvyukov@google.com> wrote:
>>
>> it LGTM if I am missing something regarding persistentalloc, and it
>> actually works
>>
>> https://codereview.appspot.com/98510044/
>
>

[khr1, 2014-08-07 20:52:17]

NOSPLIT is not enforced recursively.  NOSPLIT functions can call
non-NOSPLIT functions.  The compiler only checks that NOSPLIT call chains
use a bounded amount of stack.



On Thu, Aug 7, 2014 at 1:47 PM, Dmitry Vyukov <dvyukov@google.com> wrote:

> OK, then LGTM with the gothrow fix and sync
> But I am concerned that compiler does not complain about broken
> NOSPLIT. Either the pragma does not work or  the pragma is broken.
>
>
> On Fri, Aug 8, 2014 at 12:45 AM, Keith Randall <khr@google.com> wrote:
> > Persistentalloc needs to move to the M stack.  I'll do that in a separate
> > CL.
> >
> >
> >
> > On Thu, Aug 7, 2014 at 1:27 PM, <dvyukov@google.com> wrote:
> >>
> >> it LGTM if I am missing something regarding persistentalloc, and it
> >> actually works
> >>
> >> https://codereview.appspot.com/98510044/
> >
> >
>

[khr, 2014-08-07 21:00:51]

*** Submitted as https://code.google.com/p/go/source/detail?r=94a57b1af785 ***

runtime: convert interface routines from C to Go.

LGTM=dvyukov
R=golang-codereviews, dave, bradfitz, dvyukov, khr
CC=golang-codereviews
https://codereview.appspot.com/98510044