code review 11676043: runtime: reduce frame size for runtime.cgocallback_gofunc

runtime: reduce frame size for runtime.cgocallback_gofunc

Tying preemption to stack splits means that we have to able to
complete the call to exitsyscall (inside cgocallbackg at least for now)
without any stack split checks, meaning that the whole sequence
has to work within 128 bytes of stack, unless we increase the size
of the red zone. This CL frees up 24 bytes along that critical path
on amd64. (The 32-bit systems have plenty of space because all
their words are smaller.)

[rsc, 2013-07-22 18:54:09]

Hello dvyukov (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[dvyukov, 2013-07-22 19:08:19]

https://codereview.appspot.com/11676043/diff/3002/src/pkg/runtime/cgocall.c
File src/pkg/runtime/cgocall.c (right):

https://codereview.appspot.com/11676043/diff/3002/src/pkg/runtime/cgocall.c#n...
src/pkg/runtime/cgocall.c:246: cb =
(CallbackArgs*)((byte*)m->g0->sched.sp+4*sizeof(void*));
why it is "+4*sizeof(void*)" here, but "+3*sizeof(void*)" below?
can we do "cb = ..." only once in the beginning?
please test with -race

[rsc, 2013-07-22 19:20:14]

PTAL

On Mon, Jul 22, 2013 at 3:08 PM, <dvyukov@google.com> wrote:

> https://codereview.appspot.**com/11676043/diff/3002/src/**
>
pkg/runtime/cgocall.c<https://codereview.appspot.com/11676043/diff/3002/src/pkg/runtime/cgocall.c>
> File src/pkg/runtime/cgocall.c (right):
>
> https://codereview.appspot.**com/11676043/diff/3002/src/**
>
pkg/runtime/cgocall.c#**newcode246<https://codereview.appspot.com/11676043/diff/3002/src/pkg/runtime/cgocall.c#newcode246>
> src/pkg/runtime/cgocall.c:246: cb =
> (CallbackArgs*)((byte*)m->g0->**sched.sp+4*sizeof(void*));
> why it is "+4*sizeof(void*)" here, but "+3*sizeof(void*)" below?
> can we do "cb = ..." only once in the beginning?
>

I was trying to avoid cb needing to be stored on the stack, so that maybe
some day the C compiler can avoid allocating stack space for it.

please test with -race
>

go test -short -race runtime/race failed with the 4, now passes with 3. But
it runs too long (15s) to be part of run.bash. Per our chat, please send a
CL adding something that runs *quickly* to run.bash and that will exercise
that code path.

Thanks.
Russ

[dvyukov, 2013-07-22 19:32:14]

LGTM

[rsc, 2013-07-23 22:40:08]

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

runtime: reduce frame size for runtime.cgocallback_gofunc

Tying preemption to stack splits means that we have to able to
complete the call to exitsyscall (inside cgocallbackg at least for now)
without any stack split checks, meaning that the whole sequence
has to work within 128 bytes of stack, unless we increase the size
of the red zone. This CL frees up 24 bytes along that critical path
on amd64. (The 32-bit systems have plenty of space because all
their words are smaller.)

R=dvyukov
CC=golang-dev
https://codereview.appspot.com/11676043