code review 139620043: runtime: convert windows netpoll to Go

runtime: convert windows netpoll to Go
One C file less.
I've tried to upload it 10 times, but always get "old chunk mismatch" on netpoll_windows.go. Your best bet is unified diff.

[dvyukov, 2014-09-12 23:39:36]

Hello golang-codereviews@googlegroups.com (cc: alex.brainman@gmail.com,
khr@golang.org, rsc@golang.org),

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

[dvyukov, 2014-09-12 23:42:56]

Tried to recreate CL:
https://codereview.appspot.com/139630043/
but it also has "old chunk mismatch"...

[brainman, 2014-09-13 01:22:02]

Thank you. I will look at it tonight.

Alex

[brainman, 2014-09-13 07:09:07]

Build fails with

# Checking API compatibility.
Error running API checker: exit status 1
2014/09/13 16:51:39 error typechecking package runtime:
c:\go\root\src\runtime\netpoll_windows.go:22:4: undeclared name: overlapped
(windows-amd64)
exit status 1

I don't understand why.

Alex

https://codereview.appspot.com/139620043/diff/310001/src/runtime/netpoll_wind...
File src/runtime/netpoll_windows.go (right):

https://codereview.appspot.com/139620043/diff/310001/src/runtime/netpoll_wind...
src/runtime/netpoll_windows.go:112: +				return gp
Old version returns 0 here. Why did you change it?

[ality, 2014-09-13 07:23:28]

alex.brainman@gmail.com once said:
> Build fails with
> 
> # Checking API compatibility.
> Error running API checker: exit status 1
> 2014/09/13 16:51:39 error typechecking package runtime:
> c:\go\root\src\runtime\netpoll_windows.go:22:4: undeclared name:
> overlapped (windows-amd64)
> exit status 1
> 
> I don't understand why.

22s/overlapped/Overlapped/ 

Maybe Dmitriy forgot a file in this CL if
that type is supposed to be unexported.

  Anthony

[brainman, 2014-09-13 10:58:07]

On 2014/09/13 07:23:28, ality wrote:
> mailto:alex.brainman@gmail.com once said:
> > Build fails with
> > 
> > # Checking API compatibility.
> > Error running API checker: exit status 1
> > 2014/09/13 16:51:39 error typechecking package runtime:
> > c:\go\root\src\runtime\netpoll_windows.go:22:4: undeclared name:
> > overlapped (windows-amd64)
> > exit status 1
> > 
> > I don't understand why.
> 
> 22s/overlapped/Overlapped/ 
> 
> Maybe Dmitriy forgot a file in this CL if
> that type is supposed to be unexported.
> 

"overlapped" is defined in zruntime_defs_windows_amd64.go, and is supposed to be
unexported. I don't see why API checker fails with "undeclared name:
overlapped".

Alex

[bradfitz, 2014-09-13 13:21:04]

On Sat, Sep 13, 2014 at 6:58 AM, <alex.brainman@gmail.com> wrote:

> On 2014/09/13 07:23:28, ality wrote:
>
>> mailto:alex.brainman@gmail.com once said:
>> > Build fails with
>> >
>> > # Checking API compatibility.
>> > Error running API checker: exit status 1
>> > 2014/09/13 16:51:39 error typechecking package runtime:
>> > c:\go\root\src\runtime\netpoll_windows.go:22:4: undeclared name:
>> > overlapped (windows-amd64)
>> > exit status 1
>> >
>> > I don't understand why.
>>
>
>  22s/overlapped/Overlapped/
>>
>
>  Maybe Dmitriy forgot a file in this CL if
>> that type is supposed to be unexported.
>>
>
>
> "overlapped" is defined in zruntime_defs_windows_amd64.go, and is
> supposed to be unexported. I don't see why API checker fails with
> "undeclared name: overlapped".
>

See this part of goapi.go which is new for this release:

        if w.context != nil && file ==
fmt.Sprintf("zruntime_defs_%s_%s.go", w.context.GOOS, w.context.GOARCH) {
                // Just enough to keep the api checker happy.
                src := "package runtime; type (" +
                        " _defer struct{};" +
                        " _func struct{};" +
                        " _panic struct{};" +
                        " _select struct{}; " +
                        " _type struct{};" +
                        " alg struct{};" +
                        " chantype struct{};" +
                        " context struct{};" + // windows
                        " eface struct{};" +
                        " funcval struct{};" +
                        " g struct{};" +
                        " gobuf struct{};" +
                        " hchan struct{};" +
                        " iface struct{};" +
                        " interfacetype struct{};" +
                        " itab struct{};" +
                        " m struct{};" +
                        " maptype struct{};" +
                        " mcache struct{};" +
                        " mspan struct{};" +
                        " mutex struct{};" +
                        " note struct{};" +
                        " slicetype struct{};" +
                        " stkframe struct{};" +
                        " sudog struct{};" +
                        " waitq struct{};" +
                        " wincallbackcontext struct{};" +
                        " keventt struct{};" +
                        " timespec struct{};" +
                        " epollevent struct{};" +
                        "); " +
                        "const (" +
                        " cb_max = 2000;" +
                        " _CacheLineSize = 64;" +
                        " _Gidle = 1;" +
                        " _Grunnable = 2;" +
                        " _Grunning = 3;" +
                        " _Gsyscall = 4;" +
                        " _Gwaiting = 5;" +
                        " _Gdead = 6;" +
                        " _Genqueue = 7;" +
                        " _Gcopystack = 8;" +
                        " _NSIG = 32;" +
                        ")"
                f, err = parser.ParseFile(fset, filename, src, 0)
                if err != nil {
                        log.Fatalf("incorrect generated file: %s", err)
                }
        }

[dvyukov, 2014-09-13 14:38:01]

Fixed goapi error.
Reverted "return gp" to "return nil". Gp has to be nil there, otherwise we lose
notifications.
PTAL

[rsc, 2014-09-13 18:06:46]

Same thing I wrote on the other one:

Why are we doing this now? Is it fixing any bugs? I can't see that it is.

I thought the C to Go conversions were done for 1.4. Is this important
enough to make an exception? It needs to be solving an actual problem to be
worth the risk of modifying this code now.

[gobot, 2014-12-19 05:13:06]

R=close

To the author of this CL:

The Go project has moved to Gerrit Code Review.

If this CL should be continued, please see the latest version of
https://golang.org/doc/contribute.html for instructions on
how to set up Git and the Go project's Gerrit codereview plugin,
and then create a new change with your current code.

If there has been discussion on this CL, please give a link to it
(golang.org/cl/139620043 is best) in the description in your
new CL.

Thanks very much.