performancecopilot / speed

A Go implementation of the PCP instrumentation API
MIT License
37 stars 6 forks source link

github.com/performancecopilot/speed/bytewriter package does not compile on Windows. #37

Closed ChrisHines closed 7 years ago

ChrisHines commented 7 years ago
C:\>go version
go version go1.8 windows/amd64

C:\>go get -u -v github.com/performancecopilot/speed/bytewriter
github.com/performancecopilot/speed (download)
github.com/performancecopilot/speed/bytewriter
# github.com/performancecopilot/speed/bytewriter
..\..\performancecopilot\speed\bytewriter\memorymappedwriter.go:46: undefined: syscall.Mmap
..\..\performancecopilot\speed\bytewriter\memorymappedwriter.go:46: undefined: syscall.PROT_READ
..\..\performancecopilot\speed\bytewriter\memorymappedwriter.go:46: undefined: syscall.PROT_WRITE
..\..\performancecopilot\speed\bytewriter\memorymappedwriter.go:46: undefined: syscall.MAP_SHARED
..\..\performancecopilot\speed\bytewriter\memorymappedwriter.go:60: undefined: syscall.Munmap
natoscott commented 7 years ago

@suyash (or anyone else who may wish to work on this) - the Windows equivalent of the mmap(2) POSIX APIs can be seen in the core PCP libpcp library pmMemoryMap and pmMemoryUnmap functions: https://github.com/performancecopilot/pcp/blob/master/src/libpcp/src/win32.c

saurvs commented 7 years ago

I'd like to work on this. We can use the sys package (https://github.com/golang/sys), along with conditional compilation to avoid a runtime check for the OS. We should also enable builds on AppVeyor.

suyash commented 7 years ago

@saurvs vendoring x/sys might not be necessary, go ships with syscall.CreateFileMapping on windows. The only thing to consider here is if we want to do the work ourselves or use a simpler solution (https://github.com/edsrzf/mmap-go, https://github.com/edsrzf/mmap-go/blob/master/mmap_windows.go#L25-L72).

Could you add the AppVeyor configuration separately, so failures can be verified.

suyash commented 7 years ago

After looking at the source, I am leaning towards https://github.com/edsrzf/mmap-go. The unix version seems pretty minimal and compatible with our usecase, and the windows version seems pretty well designed.

suyash commented 7 years ago

Fixed in #41