text:
proposal: file system notifications library

Propose new package(s) under golang.org/x/ that provide wrappers around x/sys (syscall) or cgo implementations for the following file notification systems:

inotify and/or fanotify on Linux/Android
FSEvents on OS X
ReadDirectoryChangesW and/or USN Journals on Windows
kqueue on *BSD, OS X, and iOS
FEN on Solaris
(optionally) an efficient polling library intended for watching file shares and for Plan 9

Compared to syscall:

Provide a slightly higher level API.
For example, using EpollCreate1 with the file descriptor returned by InotifyInit.
Specifically for file system notifications.
For example, only supporting EVFILT_VNODE for kqueue.
Rigorous test suite.
Checks for compatibility across architectures (Go builders).
Go-friendly APIs.
Write or NoteWrite rather than NOTE_WRITE.

Compared to fsnotify:

No attempt to provide common behaviour across notification systems.
Expose the full underlying API. For example InCloseWrite and InMoveTo for inotify.
Not a common API. Each notification system would have a slightly different API.
High quality reviewed code. The Windows code in fsnotify is particularly difficult to follow.

The intent is that libraries such as fsnotify and @rjeczalik's notify could then depend on fsn instead of depending on syscall directly. Such libraries could offer common APIs and behaviour, including user-space recursive watching, filtering, etc. Various APIs could be explored in this space, while relying on the same solid underpinnings.
related: https://github.com/golang/go/issues/4068

@minux suggested x/sys/fsn which makes sense since it would utilize x/sys/windows and x/sys/unix.

Yes, I think x/sys is the correct place for this. I'm not terribly keen on fsn, though. The fs part is obvious. The n is not. If there are no plans to put a common API in place, why do we even need a new paclage? It can just be added to x/sys/unix or x/sys/windows? Or: we can add x/sys/linux, x/sys/freebsd, etc.

Yah, separate packages might be better.
Maybe x/sys/inotify, x/sys/fanotify, x/sys/fsevents, etc.? I was thinking of grouping everything under a folder called something (but not fsnotify if it's not trying to provide the API of fsnotify).
Nathan.

I'm the author of windows and linux part of @rjeczalik's notify package, so I think that my comment may be helpful:

Not a common API. Each notification system would have a slightly different API.

Actually, they can and should have a common API. All file notification systems uses one(or more) watching strategies(according to my knowledge):

file watcher
directory watcher
recursive watcher

Inotify
x
x

FSEvents

x

ReadDirectoryChangesW

x
x

kqueue
x

One can use file watcher to mimic directory watcher and recursive watcher and directory watcher to mimic recursive watcher and file watcher etc. That's what @rjeczalik notify package tries to do.
Now, we have three interfaces. Each notification system can implement some of them and we can create generic package that can mimic missing interfaces using common strategies.
Of course, each notification system package can expose its own methods. Then, we may end up with a "driver" like structure where we have independent low-level notification systems, maybe agnostic driver that selects the proper driver based on underlying operating system, and, of top of that, fsnotify package that registers a driver and tries to mimic missing interfaces.
I think this may be the right approach for usable file system notification library.
All in all, I'm not sure if x/sys is the right place for this(higher level) stuff. Nothing that can be build with methods from x/sys should be present in x/sys...

@ppknap Thanks for your insights and for your work on notify.
notify is much further along than fsnotify in this regard, as fsnotify still doesn't support recursive watches.
What I am trying to think through is how to build usable file system notifications library in layers.
Ultimately we want a common API with common behaviour across the selected notification systems. As a user, that's the kind of library I would want.
At the top: common behaviour by filling in the gaps
This provides an easy to API where the user doesn't need to know or care about the underlying file notifications systems on different platforms. The library makes the experience as consistent as possible across every platform that is supported.
This top layer comes with more opinions on how notifications should work and which features of the low-level APIs are exposed. For example, one opinion may be to use polling on BSD as with Plan 9 and NFS shares -- rather than trying to emulate both directory and recursive watching on top of kqueue's file-handle limited watcher. Another could be which underlying flags to expose and how to expose them, such as inotify's InCloseWrite which fsnotify doesn't currently offer or use (but probably should).
Part of the idea behind my original proposal is to enable multiple experiments at this layer to see what works best, but without having to rewrite the lower layers each time. The internals of fsnotify v1.x could also be gutted,  pulling in the low-level parts from a dependency, while maintaining its current API and set of behaviours.
A driver model: common API but not common behaviour
At a layer below that, contributors to fsnotify and the fsevents package have been discussing a "driver" like model for sometime. By the sounds of it, notify already has one.
My thinking here is to provide a common API but not common behaviour.

A single platform may have multiple drivers (FSEvents & kqueue, inotify & fanotify, etc.)
Not all drivers are available on all platforms, but a polling driver always is.
The user selects which driver to use. This helps align expectations with reality -- each system behaves a little differently, like choosing between MySQL and PostgreSQL.
Additional drivers could be added over time, such as FEN on Solaris, USN Journals on Windows, and fanotify on Linux.

At this layer, a kqueue driver should be accessible from a generalized API, but behave just like kqueue. No extras or emulations of other platforms. But it's important to think about how higher level abstractions could be built above the drivers.
When I first started on fsnotify, I was working on a pipeline for processing events, a bit like a middleware stack for adding user-space functionality like recursive watching and filtering. That project was discarded, but I still think it was a good idea. It's an approach I would consider trying again for fsnotify v2. I was also considering a capabilities API of sorts so that the underlying file notification systems could be utilized for various features when available.
I was originally going to propose a driver model here, but I'm not sure how a common API should expose differences in flags and features that are available. How would you do it?
If the common API is already squeezing out some features and choosing which flags are available, it would seem that there is still another level down to go.
At the bottom
This comes back to my original proposal for "fsn" or whatever it's called. Really it is just a collection of packages for each notification system using cgo or syscall, along with cross-architecture testing.
It shouldn't take much to wrap these up in a driver interface.
But I don't know, maybe this layer isn't needed. I'm very curious how others imagine building a stack for useable file notifications. Cheers.

Another goal I have is to help divide up the work and make it easier to contribute. Right now it is quite daunting to contribute to fsnotify. I think that has a lot to do with it supporting multiple platforms with all the low-level bits tangled in with the user-space extras intended to make it more consistent and usable for users.
By splitting this into layers, a Linux user interested interested the low-level bits could work on a package for inotify or even a driver for inotify without worrying so much about cross-cutting concerns. Meanwhile, someone could work on filling in the gaps with user-space code across multiple notification systems, working with the inotify package/driver rather than directly with InotifyInit and Epoll.

I was originally going to propose a driver model here, but I'm not sure how a common API should expose differences in flags and features that are available. How would you do it?

Yes, I do have an idea how this may be resolved. However, this is not an official proposal for the file system notification library, but rather loose thoughts about the design, so all variable/type names should not be taken as an official naming.
fsnotify package - it's like sql package from std. It defines:

An event as int64 type.(int32 was sufficient for notify package. However, we sometimes used it to carry additional watcher state information (example). It would be nice to have extra 32 bits only for that)

Three interfaces:

type FileWatcher interface {
    WatchFile(path string, events ...Event)
}

type DirectoryWatcher interface {
    WatchDirectory(path string, events ...Event)
}

type RecursiveWatcher interface {
    WatchRecursively(path string, events ...Event)
}

The package Watcher type will use drivers and implement all three interfaces. Eventually, it will mimic these interfaces which were not defined in imported driver.
func Use("driver_name", ...) (*Watcher, err) // similar to sql.DB.Open

It would be nice if Watcher supports filtering etc.
This approach makes it easy to mock and test this package since it doesn't know anything about underlying notification systems.
driver packages -they are pure system dependent things. So:

there are no generic events like inotify.Create etc.
only system specific event are defined (`inotify.InCreate, inotify.InDeleteSelf etc.)
they implement only these interfaces they can support eg. fsnotify.DirectoryWatcher and fsnotify.RecursiveWatcher for ReadDireectoryChangesW on windows.
they may expose extra system dependent functionality.
they have their own system dependent tests.

Since no one who writes system independent code wants to import different drivers for different systems. There is one special driver:
agnostic driver - it defines a common behavior across different systems:

defines generic events like agnostic.Create, agnostic.Delete etc.
defines a wrapper that transforms system dependent events to system independent ones.
tests are system independent.
the interfaces it implements still depend on underlying operating system.

So:

fsnotify package acts like tree from rjeczalik/notify package.
agnostic driver package acts like notify and fsnotify.
driver packages are just low level implementations.

If you want to use fsnotify in system independent way:
import (
    "whatever/fsnotify"
    "whatever/fsnotify/agnostic"
)
// ...
w, err := fsnotify.Use("agnostic", ...)
// ...
w.WatchDirectory("something/path", agnostic.Create)
// ...

If you want to use system specific thing you can use eg. inotify directly or if you still want to have benefits of filtering etc:
import (
    "whatever/fsnotify"
    "whatever/fsnotify/inotify"
)
// ...
w, err := fsnotify.Use("inotify", ...)
// ...
w.WatchDirectory("something/path", inotify.InCreate)
// ...

However, just as I said, these are just my thoughts after implementing different notification watchers to one common interface.

Thanks for sharing. None of this is an official proposal yet, just thoughts towards writing a proposal document if we proceed.
So the WatchDirectory, etc. interfaces are essentially a way of exposing the capabilities of the underlying drivers, which only satisfy the interfaces they support?

It would be nice if Watcher supports filtering etc.

Yes, I would like to take advantage of the kernel's filtering. It's quite different between Windows and Linux, so this again is a place where capabilities matter.
The fall back in a top-level API is to watch more events than necessary and filter them out before the user sees them. That provides a consistent behaviour, but at the cost of the kernel waking up to mention an event that is subsequently thrown away. So I would want to give the kernel some hints about what to watch.
I will keep thinking on these things and I hope you will too.
My initial idea here is to build out the lower level packages and test suite. But we do need a better idea of how they will be used (as drivers or not) to do this right.

So the WatchDirectory, etc. interfaces are essentially a way of exposing the capabilities of the underlying drivers, which only satisfy the interfaces they support?

exactly

My initial idea here is to build out the lower level packages and test suite. But we do need a better idea of how they will be used (as drivers or not) to do this right.

Yes, I agree. If we have at least the sketch of how this may look like, I can implement ReadDirectoryChangesW driver(or not) as a proof of concept.

I think we can simplify this proposal.
What I feel is needed right now is aninotify package with a test suite that is tested across all Linux platforms that Go supports. Whether that is x/inotify, syscall/inotify, or somewhere else the Go builders can reach.
We don't have a problem testing fsnotify on macOS and Windows with third-party CI services, and we don't yet have a Solaris implementation.
This separation would be a step towards fsnotify (and alternatives) focusing more on building a common high-level API and less on the low-level InotifyInit and EpollCreate1 details.
Perhaps more importantly, it would really help improve confidence that changes to fsnotify aren't breaking platforms that we don't have CI testing for. That has been making me nervous about merging any pull request we get.

closes in favour of #17312.

authors: bradfitz nathany ppknap

repo: golang/go


text:
runtime: syscalls are ~ 1.7x slower than in C

Please answer these questions before submitting your issue. Thanks!
What did you do?
Hello up there. First of all let me please say right from the beginning that I understand that in Go, due to goroutines, there is more work to make a system call compared to C. Still please consider the following two programs which measure time to do a pread in Go and C:
package main

import (
        "flag"
        "fmt"
        "log"
        "os"
        "time"
)

//             12345678
const Niter = 1000000

// xpread is pread(2) but aborts on an error or when fewer bytes was read
func xpread(f *os.File, buf []byte, offset int64) {
        n, err := f.ReadAt(buf, offset)
        if err != nil {
                panic(err)
        }
        if n != len(buf) {
                panic(fmt.Errorf("pread(%v) -> %v", len(buf), n))
        }
}

const BufSize = 4096
var buf [BufSize]byte

func main() {
        flag.Parse()
        if len(flag.Args()) < 1 {
                log.Fatalf("Usage: %s <file>", os.Args[0])
        }

        f, err := os.Open(flag.Args()[0])
        if err != nil {
                log.Fatal(err)
        }

        for size := 0; size <= BufSize; {
                sbuf := buf[:size]

                // warmup
                xpread(f, sbuf, /*offset=*/0)

                Tstart := time.Now()

                for i := 0; i < Niter; i++ {
                        xpread(f, sbuf, /*offset=*/0)
                }

                Tend := time.Now()

                fmt.Printf("%v\t%.1f\n", size, float64(Tend.Sub(Tstart).Nanoseconds()) / Niter)

                switch {
                case size < 16:
                        size += 1       // detailed start
                case size < 128:
                        size += 16
                default:
                        size += 128
                }
        }
}

(https://play.golang.org/p/eY1WVf7cZV)
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/time.h>

//               12345678
#define NITER   1000000

// microtime returns current time as double
double microtime() {
        int err;
        struct timeval tv;

        err = gettimeofday(&tv, NULL);
        if (err == -1) {
                perror("gettimeofday");
                abort();
        }

        return tv.tv_sec + 1E-6 * tv.tv_usec;
}

#define BUFSIZE 4096
static char buf[BUFSIZE];

// xpread is pread(2) but aborts on an error or when fewer bytes was read
void xpread(int fd, void *buf, size_t count, off_t offset) {
        ssize_t n;
        n = pread(fd, buf, count, offset);
        if (n == -1) {
                perror("pread");
                abort();
        }

        if (n != count) {
                fprintf(stderr, "pread(%ld) -> %ld\n", count, n);
                abort();
        }
}

int main(int argc, char *argv[]) {
        int fd, i;
        size_t size;

        if (argc < 2) {
                fprintf(stderr, "Usage: %s <file>\n", argv[0]);
                exit(1);
        }

        fd = open(argv[1], O_RDONLY);
        if (fd == -1) {
                perror("open");
                abort();
        }

        for (size = 0; size <= BUFSIZE;) {
                // warmup
                xpread(fd, &buf, size, /*offset=*/0);

                double Tstart, Tend;
                Tstart = microtime();

                for (i = 0; i < NITER; i++) {
                        xpread(fd, buf, size, /*offset=*/0);
                }

                Tend = microtime();

                printf("%ld\t%.1f\n", size, (Tend - Tstart) / NITER / 1E-9);

                if (size < 16) {
                        size += 1;      // detailed start
                } else if  (size < 128) {
                        size += 16;
                } else {
                        size += 128;
                }
        }
}

below is a plot of their output when run on 4K file on a tmpfs (dd if=/dev/urandom of=/dev/shm/x.dat bs=4K count=1):

From the plot it can be seen that the time to do a pread for n bytes is α + β⋅n. On my computer β ≈ 0.01ns and α is ≈ 175ns for C and ≈ 325ns for Go case. The Go syscall is thus taking ~ 150ns more time than the C version.
For programs that make many system calls that can become noticable.
The profile for Go version is below:
File: t_syspread_go
Type: cpu
Time: Mar 15, 2017 at 10:11pm (MSK)
Duration: 18.42s, Total samples = 183.20ms ( 0.99%)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top10
Showing nodes accounting for 156.70ms, 85.53% of 183.20ms total
Dropped 8 nodes (cum <= 0.92ms)
Showing top 10 nodes out of 24
      flat  flat%   sum%        cum   cum%
  112.80ms 61.57% 61.57%   141.70ms 77.35%  syscall.Syscall6 /home/kirr/src/tools/go/go/src/syscall/asm_linux_amd64.s
    9.20ms  5.02% 66.59%     9.20ms  5.02%  runtime.casgstatus /home/kirr/src/tools/go/go/src/runtime/proc.go
    6.30ms  3.44% 70.03%    12.90ms  7.04%  runtime.reentersyscall /home/kirr/src/tools/go/go/src/runtime/proc.go
    4.90ms  2.67% 72.71%     4.90ms  2.67%  internal/poll.(*fdMutex).rwlock /home/kirr/src/tools/go/go/src/internal/poll/fd_mutex.go
    4.80ms  2.62% 75.33%     4.80ms  2.62%  internal/poll.(*fdMutex).rwunlock /home/kirr/src/tools/go/go/src/internal/poll/fd_mutex.go
    4.10ms  2.24% 77.57%     5.60ms  3.06%  runtime.exitsyscallfast /home/kirr/src/tools/go/go/src/runtime/proc.go
    3.90ms  2.13% 79.69%     3.90ms  2.13%  runtime.newdefer /home/kirr/src/tools/go/go/src/runtime/panic.go
    3.80ms  2.07% 81.77%    12.80ms  6.99%  runtime.exitsyscall /home/kirr/src/tools/go/go/src/runtime/proc.go
    3.60ms  1.97% 83.73%   179.80ms 98.14%  os.(*File).ReadAt /home/kirr/src/tools/go/go/src/os/file.go
    3.30ms  1.80% 85.53%     6.30ms  3.44%  runtime.deferreturn /home/kirr/src/tools/go/go/src/runtime/panic.go

(svg version here)
from it it could be suggested to investigate the following:

the defer usage in IO poller is quite noticable. Either making defer faster (#14939, #19466, #18923, ...) or not using defer could be a solution.

runtime·entersyscall and runtime·exitsyscall takes good part of a time. Maybe something could be done about it? (probably related: #17327)

internal/poll locking: at least on Linux it is ok to issue several preads simultaneously (please correct me if I'm wrong). From pread man page:
The  pread()  and  pwrite()  system calls are especially useful in multithreaded applications.  They allow multiple
threads to perform I/O on the same file descriptor without being affected by changes to the file  offset  by  other
threads.

(http://man7.org/linux/man-pages/man2/pread.2.html)
Thus no locking is needed to serialize pread vs pread or vs read. Maybe some locking is still needed for internal bookkeeping, but may I wonder it can be made lighter?

maybe something else.

Once again I understand that in Go, due to goroutines, there is more work to make a system call compared to C, but it would be nice if the overhead is minimized.
Thanks beforehand,
Kirill
P.S.
All sources for this benchmark are accessible here: https://lab.nexedi.com/kirr/misc/tree/bfa03189
What did you expect to see?
Making syscall in Go costs only a bit than in C.
What did you see instead?
Making pread syscall in Go is ~ 1.7x slower on average than in C for reads not bigger than one page (4K).
Does this issue reproduce with the latest release (go1.8)?
Yes, but there with go18 it was only ~ 90ns slower:

in other words pread in go tip is slower than pread in go18 by ~ 60ns.
System details
go version devel +43afcb5c96 Wed Mar 15 04:27:49 2017 +0000 linux/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/kirr/go"
GORACE=""
GOROOT="/home/kirr/src/tools/go/go"
GOTOOLDIR="/home/kirr/src/tools/go/go/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build990167505=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOROOT/bin/go version: go version devel +43afcb5c96 Wed Mar 15 04:27:49 2017 +0000 linux/amd64
GOROOT/bin/go tool compile -V: compile version devel +43afcb5c96 Wed Mar 15 04:27:49 2017 +0000 X:framepointer
uname -sr: Linux 4.9.0-2-amd64
Distributor ID:	Debian
Description:	Debian GNU/Linux 9.0 (stretch)
Release:	9.0
Codename:	stretch
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Debian GLIBC 2.24-9) stable release version 2.24, by Roland McGrath et al.
gdb --version: GNU gdb (Debian 7.12-6) 7.12.0.20161007-git

I think you buried the lede. Yuu should update the title to reflect the
fact that syscalls got 60ns slower on tip.
On Thu, 16 Mar 2017, 08:09 Josh Bleecher Snyder notifications@github.com
wrote:

cc @bcmills https://github.com/bcmills @ianlancetaylor
https://github.com/ianlancetaylor
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/golang/go/issues/19563#issuecomment-286880138, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAcAyOiZH8mkblkB2AmQ5MGaAuVfmxsks5rmFOagaJpZM4MehVB
.

What's odd is that I don't see any actual changes to EnterSyscall6 in syscall/asm_linux_amd64.s since 2015, but the bulk of the time in the profile is in EnterSyscall6 directly.
That would seem to imply one of a few things:

The difference is due to cache effects, some change in the function prologue or epilogue, or inlining.
The actual source of the slowdown isn't being captured in the profile for some reason.
The actual timing hasn't changed, but the measurement has (e.g. due to the monotonic-time changes or changes to debug maps).

Can you replicate the same regression from 1.7 to 1.8 to tip using testing.B instead of time.Now?  That would help to confirm or refute (3).

Tip is bound to be slower than 1.8 because of the support for the runtime poller.  This brings various advantages but does take more time.  But I'm frankly surprised that this time is not swamped by the overall kernel time required to implement the syscall and copy the bytes.

Ah, now I understand why the pprof output looks so strange: the program is not even remotely CPU-bound in the application code.
Duration: 18.42s, Total samples = 183.20ms ( 0.99%)

So the pprof output likely looks strange because it's drawing from a very small sample size: the sampling error swamps out most of the interesting detail.

First of all everyone - thanks for feedback.
@bcmills the timing measurments are outside benchmarking loop, so timer changes should not probably affect anything. Said that below is the benchmark to read 1 byte via pread from tmpfs for us to be sure:
package main

import (
        "crypto/rand"
        "fmt"
        "os"
        "testing"
)

// xpread is pread(2) but aborts on an error or when fewer bytes was read
func xpread(f *os.File, buf []byte, offset int64) {
        n, err := f.ReadAt(buf, offset)
        if err != nil {
                panic(err)
        }
        if n != len(buf) {
                panic(fmt.Errorf("pread(%v) -> %v", len(buf), n))
        }
}

// BenchmarkPread1 benchmarks pread syscall when reading 1 byte from a file on tmpfs
func BenchmarkPread1(b *testing.B) {
        // setup 4K file on a tmpfs
        f, err := os.Create("/dev/shm/x.dat")
        if err != nil {
                b.Fatal(err)
        }

        buf4K := make([]byte, 4096)
        _, err = rand.Read(buf4K)
        if err != nil {
                // rand promises n = len(buf) <=> err == nil
                b.Fatal(err)
        }

        _, err = f.Write(buf4K)
        if err != nil {
                b.Fatal(err)
        }

        buf1B := make([]byte, 1)

        // warm up
        xpread(f, buf1B, 0)

        b.ResetTimer()
        for i := 0; i < b.N; i++ {
                xpread(f, buf1B, 0)
        }
        b.StopTimer()

        err = f.Close()
        if err != nil {
                b.Fatal(f)
        }
}

(https://lab.nexedi.com/kirr/misc/blob/dcdba0e3/syspread_test.go)
and below are timings for this benchmark for all go versions starting from go14 statted over 10 runs each:
name \ time/op  go14.txt    go15.txt    go16.txt    go17.txt    go18.txt    gotip.txt
Pread1          267ns ± 0%
Pread1-4                    277ns ± 1%  274ns ± 1%  273ns ± 1%  265ns ± 3%  325ns ± 1%

(code to run/stat here: https://lab.nexedi.com/kirr/misc/blob/dcdba0e3/Makefile#L20)
we can see, once again, that go tip increases the time significantly.
about pprof output - I'm not so experienced here to check, but the time difference can be also seen via running programs directly, without internal benchmarks, and measuring whole-program time with bash's time - the difference measured this way is real.
Thanks again,
Kirill

Your benchmark seems to be comparing direct pread syscalls to (*os.File).ReadAt calls.  Can you benchmark direct syscalls from Go?  It would be interesting to see whether the difference is coming from the syscall itself or some other part of ReadAt — for example, there could be a lock-contention issue for *os.File that is unrelated to the syscall proper.

@bcmills good point. It is:
name \ time/op  go14.txt    go15.txt    go16.txt    go17.txt    go18.txt    gotip.txt
ReadAt1         268ns ± 1%
Pread1          259ns ± 1%
ReadAt1-4                   274ns ± 1%  277ns ± 1%  274ns ± 1%  263ns ± 1%  323ns ± 0%
Pread1-4                    262ns ± 1%  266ns ± 1%  262ns ± 2%  252ns ± 1%  251ns ± 1%

(updated benchmark, diff)
So:

There is no regression in syscall.Pread itself - go18/gotip even improves compared to go17 by 10ns; however even with this improvement syscall.Pread is slower than pread in C by 65 - 70ns, i.e. go is ~ 35-40% slower for the syscall itself.

For syscall.Pread itself I get the following profile (maybe something dumb on my side; sorry):
File: t_syspread_go
Type: cpu
Time: Mar 17, 2017 at 11:21am (MSK)
Duration: 14.81s, Total samples = 146.50ms ( 0.99%)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top10
Showing nodes accounting for 145.50ms, 99.32% of 146.50ms total
Dropped 4 nodes (cum <= 0.73ms)
Showing top 10 nodes out of 13
      flat  flat%   sum%        cum   cum%
  115.30ms 78.70% 78.70%   143.70ms 98.09%  syscall.Syscall6 /home/kirr/src/tools/go/go/src/syscall/asm_linux_amd64.s
    9.90ms  6.76% 85.46%     9.90ms  6.76%  runtime.casgstatus /home/kirr/src/tools/go/go/src/runtime/proc.go
       7ms  4.78% 90.24%    13.90ms  9.49%  runtime.reentersyscall /home/kirr/src/tools/go/go/src/runtime/proc.go
    4.90ms  3.34% 93.58%     5.90ms  4.03%  runtime.exitsyscallfast /home/kirr/src/tools/go/go/src/runtime/proc.go
    3.50ms  2.39% 95.97%    13.20ms  9.01%  runtime.exitsyscall /home/kirr/src/tools/go/go/src/runtime/proc.go
    1.50ms  1.02% 97.00%   146.40ms 99.93%  main.main /home/kirr/src/wendelin/z/rust/bench/t_syspread_go.go
       1ms  0.68% 97.68%        1ms  0.68%  runtime.exitsyscallfast_reacquired /home/kirr/src/tools/go/go/src/runtime/proc.go
    0.80ms  0.55% 98.23%     0.80ms  0.55%  runtime.getcallerpc /home/kirr/src/tools/go/go/src/runtime/asm_amd64.s
    0.80ms  0.55% 98.77%     0.80ms  0.55%  runtime.save /home/kirr/src/tools/go/go/src/runtime/proc.go
    0.80ms  0.55% 99.32%   144.50ms 98.63%  syscall.Pread /home/kirr/src/tools/go/go/src/syscall/zsyscall_linux_amd64.go

(svg)
So for the syscall itself it looks like the overhead is mainly in runtime·entersyscall and runtime·exitsyscall and probably related goroutiny things in scheduler.

The differencein between os.File.ReadAt(len(buf)=1) and syscall.Pread(len(buf)=1) was ~ 10ns during go14 - go18. With go tip it raises to ~70ns, so it is indeed 60ns regression for os.File.ReadAt.

I've moved os.File regression to #19586.

If the os.File slowdown is moved to #19586, what is left to do in this issue?

That making system call in Go is ~ 1.4x slower than making it in C.

Do you have a benchmark for that?  I don't see one above; sorry if I missed it.
It's pretty hard to fix.  While we might be able to shave some time off, fundamentally the Go function syscall.Syscall must update the scheduler in some way, whereas the C function syscall does not need to do anything similar.  I'm actually surprised that the slowdown is only 1.4x.

@ianlancetaylor the benchmark for syspread in C and in Go is this:

action
time (s)

pread(count=1), C
175ns

pread(count=4K), C
175ns + ~40ns

syscall.Pread(len(buf)=1), Go tip
251ns

syscall.Pread(len(buf)=4K), Go tip
251ns + ~40ns

This data was already presented there before in scattered way. The above table summarizes it once again with having actual syscal in focus. I appologize for the confusion.

About overhead: let me once again say that  I understand that in Go, due to goroutines, there is more work to make a system call compared to C, but it would be nice if the overhead is minimized.
In my original post I already gave some ideas, regarding syscalls imho the low-hanging small fruit is to try to intrinsify getcallerpc (#17327). Usually if we also audit things with performance in mind something else could probably be found in runtime·entersyscall and runtime·exitsyscall land. We can use this issue as tracking for this kind of work.
Thanks,
Kirill

Your profile is still not even close to CPU-bound: less than 1% of the time spent in the benchmark is executing instructions in the Go part of the program.
Duration: 14.81s, Total samples = 146.50ms ( 0.99%)

I will also note that 40ns is approximately the cost of an L3 cache miss on modern AMD64 hardware.

@bcmills I'm not sure about profile, because I had not played with go profiler enough, however I've made the following test:
package main

import (
        "flag"
        "fmt"
        "log"
        "os"
        "runtime/pprof"
        "time"
)

//             12345678
const Niter = 100000000

var cpuprofile = flag.String("cpuprofile", "", "write cpu profile `file`")

//go:noinline
func aaa() {
}

func main() {
        flag.Parse()

        if *cpuprofile != "" {
                f, err := os.Create(*cpuprofile)
                if err != nil {
                        log.Fatal("could not create CPU profile: ", err)
                }
                if err := pprof.StartCPUProfile(f); err != nil {
                        log.Fatal("could not start CPU profile: ", err)
                }
                defer pprof.StopCPUProfile()
        }

        Tstart := time.Now()

        for i := 0; i < Niter; i++ {
                aaa()
        }

        Tend := time.Now()

        fmt.Printf("%.1f\n", float64(Tend.Sub(Tstart).Nanoseconds()) / Niter)
}

kirr@mini:~/src/wendelin/z/rust/bench$ time ./t_loop_go -cpuprofile cpu.prof
14.6

real    0m1,615s
user    0m1,464s
sys     0m0,012s

kirr@mini:~/src/wendelin/z/rust/bench$ go tool pprof ./t_loop_go cpu.prof
File: t_loop_go
Type: cpu
Time: Mar 17, 2017 at 10:27pm (MSK)
Duration: 1.60s, Total samples = 14.50ms (  0.9%)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top
Showing nodes accounting for 14.50ms, 100% of 14.50ms total
      flat  flat%   sum%        cum   cum%
   13.70ms 94.48% 94.48%    14.40ms 99.31%  main.main /home/kirr/src/wendelin/z/rust/bench/t_loop_go.go
    0.70ms  4.83% 99.31%     0.70ms  4.83%  main.aaa /home/kirr/src/wendelin/z/rust/bench/t_loop_go.go
    0.10ms  0.69%   100%     0.10ms  0.69%  runtime._ExternalCode /home/kirr/src/tools/go/go/src/runtime/proc.go
         0     0%   100%     0.10ms  0.69%  runtime._System /home/kirr/src/tools/go/go/src/runtime/proc.go
         0     0%   100%    14.40ms 99.31%  runtime.main /home/kirr/src/tools/go/go/src/runtime/proc.go

This program is completely cpu-bound so to me above Duration: 1.60s, Total samples = 14.50ms (  0.9%) either says the profiler shows fraction of time how much it sampled (?) or it is some other bug.

About 40ns - I will also note that it is not a cache miss here: if you look once again at original plot to pread/copy data:

please ignore go lines above and look for syspread/c and copy/c lines. They clearly show the time grow linearly as the number of bytes to read/copy grows. The syspread/c - copy/c line also shows that the kernel is actually doing a straight memcpy from pagecache to destination buffer. 40ns is just the time it takes to copy 4K on my computer when both source and destination are hot in cache, because for such case the average time to copy is 0.01ns per byte here.

Ah, I think I'm misinterpreting the new pprof format (reported as https://github.com/google/pprof/issues/128).
I believe you're correct that the program is actually CPU-bound; I apologize for my confusion.

Np, and thanks for bringing this up with pprof team.

Btw, I've looked around a bit and noticed: in runtime·entersyscall / runtime·exitsyscall land a lot of time is spent on atomics, e.g. in casgstatus and it is know that atomics are quite expensive. Prior to go14 g.status was updated in non-atomic way: see commit 0a7c7ac8 (https://golang.org/cl/132960044) - the change was added there to support concurrent GC.
Here is updated benchmark for pread syscall for all Go releases:
name \ time/op  go10.txt    go11.txt    go12.txt    go13.txt    go14.txt    go15.txt    go16.txt    go17.txt    go18.txt    gotip.txt
Pread1-4        226ns ± 0%  229ns ± 0%  240ns ± 1%  242ns ± 2%  261ns ± 1%  263ns ± 1%  267ns ± 0%  262ns ± 1%  255ns ± 2%  252ns ± 1%

(updated benchmark, test)
and it indeed shows go13 -> go14 20ns slowdown (and another go10/go11 -> go12 15ns slowdown I had not checked reason for).
So if atomics for e.g. g.status update were introduced to support concurrent GC, maybe what is also possible to do is to update g.status not always in atomic manner, but in a way similar to what happens in runtime·writebarrierptr() - there before updating a pointer the runtime checks for whether write barrier is at all needed, and if not it becomes simple store. And if I understand correctly write barriers are needed only during certain phase of running garbage collection - in other words "usually not" most of the time. Probably we could have some similar flag for g.status + friends updates too.
I do not know the GC details, so please forgive me in advance if the above is dumb in one way or another. It sounds reasonable to me offhand, however.
Thanks beforehand,
Kirill
/cc @RLH, @aclements

Btw, I've looked around a bit and noticed: in runtime·entersyscall / runtime·exitsyscall land a lot of time is spent on atomics, e.g. in casgstatus and it is know that atomics are quite expensive.

Atomics got a bad reputation from their early implementations, which were extremely expensive. But on a modern x86 an atomic operation on an uncontended cache line in the L1 is only ~2.5x more expensive than a plain write.
That said, you're definitely right that the atomic operations are the hottest individual operations in the user-space side of the syscall path (partly because there's not that much else going on :). According to PEBS profiling (perf -e cycles:pp), your Pread1 benchmark spends about 4% of its total time in the cmpxchg on the gstatus, 1.5% in the P status write in reentersyscall, and 1.4% in the CAS of the P status in exitsyscallfast.

maybe what is also possible to do is to update g.status not always in atomic manner

That's a clever idea. I haven't completely convinced myself that this is safe, but I did a simple experiment to see what impact it would have. I disabled GC with GOGC=off and added the following patch:
diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 159a9bd4bc..1538274c77 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -749,6 +749,17 @@ func casgstatus(gp *g, oldval, newval uint32) {
                throw("casgstatus")
        }

+       if gcphase == _GCoff {
+               if gp.atomicstatus != oldval {
+                       throw("bad old status")
+               }
+               gp.atomicstatus = newval
+               if newval == _Grunning {
+                       gp.gcscanvalid = false
+               }
+               return
+       }
+
        // See http://golang.org/cl/21503 for justification of the yield delay.
        const yieldDelay = 5 * 1000
        var nextYield int64

Here are the results on my laptop before and after this patch (go test -c main_test.go && GOGC=off perflock ./main.test -test.bench . -test.count 20):
name       old time/op  new time/op  delta
ReadAt1-4   514ns ± 2%   504ns ± 1%  -2.02%  (p=0.000 n=20+20)
Pread1-4    431ns ± 2%   414ns ± 0%  -3.86%  (p=0.000 n=20+16)

So this saves 10–17ns in the syscall path when GC is inactive. Given the cost of most syscalls, I'm not sure that's worth the added complexity and potential for extremely subtle bugs, but maybe it is.

@aclements first of all thanks for your feedback, and for heads-up that atomics became not so expensive. Looks like cache coherency logic implementation in x86 became much more advanced over time. Anyway

Thanks for confirming my observations with perf
Thanks a lot for commenting on the idea and for try-patch.

On my side with it for pread-related benchmarks I get:
name       old time/op  new time/op  delta
ReadAt1-4   323ns ± 1%   317ns ± 0%  -1.84%  (p=0.000 n=18+19)
Pread1-4    251ns ± 0%   240ns ± 0%  -4.19%  (p=0.000 n=20+20)

which more or less confirms you findings.

Given the cost of most syscalls, I'm not sure that's worth the added complexity and potential for extremely subtle bugs, but maybe it is.

Here comes something to consider: during e.g. web-serving workload, there are a lot of system calls made. If I understand correctly, actually on e.g. any socket, for Read or Write first a direct system call is made, and only if it returns EAGAIN the fd is registerd to netpoller (or operation succeeds right offhand if it was ready). The syscalls to read and write are also used later to send/receive data when fd becomes ready. So there are many system calls involved.
With this in mind I've tried bench/go1's BenchmarkHTTPClientServer without and with your patch and for it it is the following on my computer:
name                old time/op  new time/op  delta
HTTPClientServer-4  40.4µs ± 1%  39.5µs ± 1%  -2.15%  (p=0.000 n=18+18)

The effect is quite visible, right? And it is only with partly mitigated go13 -> go14 syscall-related change.
So what I would suggest is:

consider going on with atomics avoidance approach.
bisect and see what made syscalls slower during go10/11 -> go12 transition; try to fix that too.
try to remove checks that are not really needed from hot paths.
maybe something else...

All that because runtime·entersyscall / runtime·exitsyscall are present there during every system call made and usually programs make many of them. At least if we consider WEB-servers or programs that work with files.
Thanks again,
Kirill
P.S. thanks also for cycles:pp 2 and perflock references.

With this in mind I've tried bench/go1's BenchmarkHTTPClientServer without and with your patch and for it it is the following on my computer:

That's pretty intriguing, actually. I've kicked off the go1 and x/benchmarks suites to see what the results there are.

P.S. thanks also for cycles:pp 2 and perflock references.

Thanks for reminding me that I never published my new version of perflock! What I was actually using is https://github.com/aclements/perflock, which has other nice properties like ensuring FIFO ordering, taking control of the CPU governor, and being written in Go. :) I'm hoping to do more with it, like using cgroups for isolation, but haven't yet.

@aclements thanks for feedback. It is always nice when discussions make us put on light forgotten things, etc. Thanks for the correct link :)
Thanks also for starting the benchmarks. Let's see the results when you have it.

I note that we know that the network I/O syscalls are known to be non-blocking, and therefore it may be possible to use RawSyscall rather than Syscall, which cuts out the atomic instructions entirely.  However, implementing that safely may be difficult, since these code paths are now also used for file I/O operations, and using RawSyscall for a file I/O operation can fail when using a user-space FUSE file system (https://groups.google.com/d/msg/golang-nuts/11rdExWP6ac/UnP5P5m3VPEJ).

@ianlancetaylor thanks for heads up on RawSyscall vs Syscall. As replied in the thread (my post still waiting moderation to appear) it is not FUSE-only, but can be with any file system which operations block (e.g. close on NFS is blocking).
Said that maybe what could be done on Go side for both sockets and  files such as pipe/fifo/etc is:

track whether file descriptor mode is O_NONBLOCK or not,

when there is a need to make Read or similar, do an if on that flag, and if we know:

the mode is non-blocking, and
for this fd we can trust the kernel to respect O_NONBLOCK (i.e. it is not regular file) (*)

do a RawSyscall. Otherwise do Syscall.

Such if should cost a very little and imho the logic could be put into pollDesc.
Or it can be the other way: only proper file descriptors are registered to poller and everything else uses direct Syscall for Read/Write.
Said that as well it is needed to note that regular files remain not covered this way. So maybe it is better to just optimize regular Syscall for the benefit of all. And even if we go the RawSyscall way, since file IO cannot be really done without syscalls(+), removing the overhead is important. Not to mention that even with sockets there are still things like EpollWait, get/setsockopt, fcntl, etc which are using Syscall entry to kernel.
Thanks again,
Kirill
(*) from http://man7.org/linux/man-pages/man2/open.2.html
O_NONBLOCK or O_NDELAY
              ...

              Note that this flag has no effect for regular files and block
              devices; that is, I/O operations will (briefly) block when
              device activity is required, regardless of whether O_NONBLOCK
              is set...

(+) see e.g. http://marc.info/?l=linux-kernel&m=149002565506733&w=2

@navytux, here are the benchmark results (sorry, left them running and then forgot about them :): https://perf.golang.org/search?q=upload%3A20170330.4+|+commit%3A01ac5b8dcfe5342af3770b0834220b87ea328fad+vs+commit%3A9d96f6d72012c7ef902eecbec43ff7cf2a37c79d
So there's actually very little difference, at least in these benchmarks.

@aclements thanks for your feedback. I was going to blame myself for doing something stupid last time because today I tried and could not reproduce my previous results for BenchmarkHTTPClientServer neither on today's tip (5a45a157) nor on tree state more-or-less corresponding to time of my comment that claimed -2.15% on the benchmark.
However with all that I could not believe myself to blame because I know I did not cooked the numbers and here they are in raw form for the runs on Mar 20:
kirr@deco:~/src/tools/go/go/test/bench/go1/old$ ll --full-time
-rw-r--r-- 1 kirr kirr 1171 2017-03-20 20:24:38.439971290 +0300 0.txt
-rw-r--r-- 1 kirr kirr 1171 2017-03-20 20:25:58.121720781 +0300 1.txt

kirr@deco:~/src/tools/go/go/test/bench/go1/old$ cat 0.txt # without patch
goos: linux
goarch: amd64
BenchmarkHTTPClientServer-4        30000             42846 ns/op
BenchmarkHTTPClientServer-4        30000             40022 ns/op
BenchmarkHTTPClientServer-4        30000             40354 ns/op
BenchmarkHTTPClientServer-4        30000             40427 ns/op
BenchmarkHTTPClientServer-4        30000             40177 ns/op
BenchmarkHTTPClientServer-4        50000             42325 ns/op
BenchmarkHTTPClientServer-4        30000             40564 ns/op
BenchmarkHTTPClientServer-4        30000             40597 ns/op
BenchmarkHTTPClientServer-4        30000             40321 ns/op
BenchmarkHTTPClientServer-4        50000             40198 ns/op
BenchmarkHTTPClientServer-4        30000             40224 ns/op
BenchmarkHTTPClientServer-4        30000             40491 ns/op
BenchmarkHTTPClientServer-4        30000             40140 ns/op
BenchmarkHTTPClientServer-4        30000             40580 ns/op
BenchmarkHTTPClientServer-4        50000             40381 ns/op
BenchmarkHTTPClientServer-4        50000             40575 ns/op
BenchmarkHTTPClientServer-4        50000             40123 ns/op
BenchmarkHTTPClientServer-4        30000             40274 ns/op
BenchmarkHTTPClientServer-4        50000             40764 ns/op
BenchmarkHTTPClientServer-4        30000             40157 ns/op
PASS

kirr@deco:~/src/tools/go/go/test/bench/go1/old$ cat 1.txt # with patch
goos: linux
goarch: amd64
BenchmarkHTTPClientServer-4        30000             42884 ns/op
BenchmarkHTTPClientServer-4        50000             41835 ns/op
BenchmarkHTTPClientServer-4        50000             39534 ns/op
BenchmarkHTTPClientServer-4        50000             39542 ns/op
BenchmarkHTTPClientServer-4        50000             39326 ns/op
BenchmarkHTTPClientServer-4        50000             39487 ns/op
BenchmarkHTTPClientServer-4        50000             39352 ns/op
BenchmarkHTTPClientServer-4        50000             39342 ns/op
BenchmarkHTTPClientServer-4        50000             39323 ns/op
BenchmarkHTTPClientServer-4        50000             39490 ns/op
BenchmarkHTTPClientServer-4        50000             39472 ns/op
BenchmarkHTTPClientServer-4        50000             39496 ns/op
BenchmarkHTTPClientServer-4        50000             39633 ns/op
BenchmarkHTTPClientServer-4        50000             39433 ns/op
BenchmarkHTTPClientServer-4        50000             39490 ns/op
BenchmarkHTTPClientServer-4        50000             39713 ns/op
BenchmarkHTTPClientServer-4        30000             39519 ns/op
BenchmarkHTTPClientServer-4        50000             39577 ns/op
BenchmarkHTTPClientServer-4        50000             39629 ns/op
BenchmarkHTTPClientServer-4        50000             39358 ns/op
PASS

So after trying many things and looking here and there I've noticed: the average time to run one operation is sometimes a bit more the more iterations the benchmark driver makes. E.g. (from 0.txt):
BenchmarkHTTPClientServer-4        30000             40177 ns/op
BenchmarkHTTPClientServer-4        50000             42325 ns/op    <-- NOTE
BenchmarkHTTPClientServer-4        30000             40564 ns/op

That means that if the whole time to run 1 benchmark for 30000 iterations was ~ 1s (go test defaults for  -benchtime), the time to run 1 benchmark for 50000 iterations was ~ 1.7s - i.e. almost twice as longer.
Then I've looked at cpu profile for BenchmarkHTTPClientServer ran with -benchtime 1s and -benchtime 10s  and they are visibly different:
(g.env) kirr@deco:~/src/tools/go/go/test/bench/go1$ go test -bench Client -cpuprofile cpu.out -benchtime 1s
goos: linux
goarch: amd64
BenchmarkHTTPClientServer-4        30000             43363 ns/op
PASS
ok      _/home/kirr/src/tools/go/go/test/bench/go1      3.144s
(g.env) kirr@deco:~/src/tools/go/go/test/bench/go1$ go tool pprof go1.test cpu.out
File: go1.test
Type: cpu
Time: Mar 31, 2017 at 5:37pm (MSK)
Duration: 1.90s, Total samples = 20.80ms ( 1.09%)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top20
Showing nodes accounting for 11.90ms, 57.21% of 20.80ms total
Dropped 103 nodes (cum <= 0.10ms)
Showing top 20 nodes out of 141
      flat  flat%   sum%        cum   cum%
    5.60ms 26.92% 26.92%     5.80ms 27.88%  syscall.Syscall /home/kirr/src/tools/go/go/src/syscall/asm_linux_amd64.s
    0.80ms  3.85% 30.77%     2.30ms 11.06%  runtime.findrunnable /home/kirr/src/tools/go/go/src/runtime/proc.go
    0.70ms  3.37% 34.13%     0.70ms  3.37%  runtime.usleep /home/kirr/src/tools/go/go/src/runtime/sys_linux_amd64.s
    0.50ms  2.40% 36.54%     0.50ms  2.40%  runtime._ExternalCode /home/kirr/src/tools/go/go/src/runtime/proc.go
    0.40ms  1.92% 38.46%     0.40ms  1.92%  runtime.futex /home/kirr/src/tools/go/go/src/runtime/sys_linux_amd64.s
    0.30ms  1.44% 39.90%     0.40ms  1.92%  internal/poll.runtime_pollSetDeadline /home/kirr/src/tools/go/go/src/runtime/netpoll.go
    0.30ms  1.44% 41.35%     0.60ms  2.88%  net/http.readTransfer /home/kirr/src/tools/go/go/src/net/http/transfer.go
    0.30ms  1.44% 42.79%     0.30ms  1.44%  runtime.epollwait /home/kirr/src/tools/go/go/src/runtime/sys_linux_amd64.s
    0.30ms  1.44% 44.23%     0.30ms  1.44%  runtime.heapBitsSetType /home/kirr/src/tools/go/go/src/runtime/mbitmap.go
    0.30ms  1.44% 45.67%     0.30ms  1.44%  runtime.mapassign /home/kirr/src/tools/go/go/src/runtime/hashmap.go
    0.30ms  1.44% 47.12%     0.30ms  1.44%  runtime.memeqbody /home/kirr/src/tools/go/go/src/runtime/asm_amd64.s
    0.30ms  1.44% 48.56%     0.30ms  1.44%  runtime.nextFreeFast /home/kirr/src/tools/go/go/src/runtime/malloc.go
    0.30ms  1.44% 50.00%     0.40ms  1.92%  runtime.selectgo /home/kirr/src/tools/go/go/src/runtime/select.go
    0.30ms  1.44% 51.44%     0.30ms  1.44%  sync.(*Pool).pin /home/kirr/src/tools/go/go/src/sync/pool.go
    0.20ms  0.96% 52.40%     1.60ms  7.69%  bufio.(*Reader).ReadLine /home/kirr/src/tools/go/go/src/bufio/bufio.go
    0.20ms  0.96% 53.37%     0.60ms  2.88%  net/http.(*Transport).tryPutIdleConn /home/kirr/src/tools/go/go/src/net/http/transport.go
    0.20ms  0.96% 54.33%     4.40ms 21.15%  net/http.(*conn).readRequest /home/kirr/src/tools/go/go/src/net/http/server.go
    0.20ms  0.96% 55.29%     2.80ms 13.46%  net/http.(*persistConn).readLoop /home/kirr/src/tools/go/go/src/net/http/transport.go
    0.20ms  0.96% 56.25%     3.80ms 18.27%  net/http.(*persistConn).writeLoop /home/kirr/src/tools/go/go/src/net/http/transport.go
    0.20ms  0.96% 57.21%     0.20ms  0.96%  net/http.(*transferWriter).shouldSendContentLength /home/kirr/src/tools/go/go/src/net/http/transfer.go

(g.env) kirr@deco:~/src/tools/go/go/test/bench/go1$ go test -bench Client -cpuprofile cpu.out -benchtime 10s
goos: linux
goarch: amd64
BenchmarkHTTPClientServer-4       300000             41310 ns/op
PASS
ok      _/home/kirr/src/tools/go/go/test/bench/go1      14.265s
(g.env) kirr@deco:~/src/tools/go/go/test/bench/go1$ go tool pprof go1.test cpu.out
File: go1.test
Type: cpu
Time: Mar 31, 2017 at 5:40pm (MSK)
Duration: 13.01s, Total samples = 158.60ms ( 1.22%)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top20
Showing nodes accounting for 79.70ms, 50.25% of 158.60ms total
Dropped 282 nodes (cum <= 0.79ms)
Showing top 20 nodes out of 169
      flat  flat%   sum%        cum   cum%
   29.70ms 18.73% 18.73%    31.50ms 19.86%  syscall.Syscall /home/kirr/src/tools/go/go/src/syscall/asm_linux_amd64.s
    6.20ms  3.91% 22.64%     6.20ms  3.91%  runtime.futex /home/kirr/src/tools/go/go/src/runtime/sys_linux_amd64.s
    4.10ms  2.59% 25.22%     4.10ms  2.59%  runtime.epollwait /home/kirr/src/tools/go/go/src/runtime/sys_linux_amd64.s
    4.10ms  2.59% 27.81%     4.80ms  3.03%  runtime.lock /home/kirr/src/tools/go/go/src/runtime/lock_futex.go
    3.80ms  2.40% 30.20%        8ms  5.04%  runtime.selectgo /home/kirr/src/tools/go/go/src/runtime/select.go
    3.50ms  2.21% 32.41%    17.90ms 11.29%  runtime.findrunnable /home/kirr/src/tools/go/go/src/runtime/proc.go
    3.20ms  2.02% 34.43%    10.10ms  6.37%  runtime.mallocgc /home/kirr/src/tools/go/go/src/runtime/malloc.go
    2.80ms  1.77% 36.19%     2.80ms  1.77%  runtime._ExternalCode /home/kirr/src/tools/go/go/src/runtime/proc.go
    2.70ms  1.70% 37.89%     2.70ms  1.70%  runtime.casgstatus /home/kirr/src/tools/go/go/src/runtime/proc.go
    2.60ms  1.64% 39.53%     2.60ms  1.64%  runtime.heapBitsSetType /home/kirr/src/tools/go/go/src/runtime/mbitmap.go
    2.30ms  1.45% 40.98%        3ms  1.89%  runtime.deferreturn /home/kirr/src/tools/go/go/src/runtime/panic.go
    2.10ms  1.32% 42.31%     2.10ms  1.32%  runtime.usleep /home/kirr/src/tools/go/go/src/runtime/sys_linux_amd64.s
       2ms  1.26% 43.57%        2ms  1.26%  runtime.unlock /home/kirr/src/tools/go/go/src/runtime/lock_futex.go
    1.90ms  1.20% 44.77%     1.90ms  1.20%  sync.(*Mutex).Unlock /home/kirr/src/tools/go/go/src/sync/mutex.go
    1.80ms  1.13% 45.90%     1.80ms  1.13%  runtime.nextFreeFast /home/kirr/src/tools/go/go/src/runtime/malloc.go
    1.40ms  0.88% 46.78%        2ms  1.26%  runtime.mapassign /home/kirr/src/tools/go/go/src/runtime/hashmap.go
    1.40ms  0.88% 47.67%     3.50ms  2.21%  runtime.mapassign_faststr /home/kirr/src/tools/go/go/src/runtime/hashmap_fast.go
    1.40ms  0.88% 48.55%     1.40ms  0.88%  runtime.memclrNoHeapPointers /home/kirr/src/tools/go/go/src/runtime/memclr_amd64.s
    1.40ms  0.88% 49.43%     1.40ms  0.88%  sync.(*Mutex).Lock /home/kirr/src/tools/go/go/src/sync/mutex.go
    1.30ms  0.82% 50.25%     3.40ms  2.14%  runtime.runqgrab /home/kirr/src/tools/go/go/src/runtime/proc.go

Notice in particular:

Syscall part of the time is quite different (18% vs 26%)
mallocgc + friends is visible in longer run (not in run for 1s)
casgstatus is visible in longer run (not in run for 1s)

That lead me to thinking that much less garbage collection is actually happenning when bench time is only 1s. That turned out to be true even if the benchmark is repeated several times as go benchmark driver explicitly calls GC in between runs but that time is not accounted for the actual benchmark in question:
https://github.com/golang/go/blob/5a45a157/src/testing/benchmark.go#L141
That means that for benchmarks which actually generate garbage, but not so many to systematically trigger statistically sound number of actual garbage collections in default benchtime=1s, the default benchtime=1s is not appropriate as it actually does not show the cost associated with garbage collections (and casgstatus is associated 100% with the GC) even when run with -test.count=N. And BenchmarkHTTPClientServer happens to be benchmark of such kind.
So with this in mind the correct procedure to test the effect of GC-related patches must run the benchmark with benchtime=Ns  (N is timeframe in which GC actually triggers statistically sound). This way I've tested once again today's tip (5a45a157) without and with the patch with N=50s on a Linux 4.9.0-2-amd64 with i7-6600U CPU:
.../go/test/bench/go1$ go test -bench Client -benchtime 50s -count 20 -timeout 50m

and here is what it gives:
.../go/test/bench/go1$ benchstat B3/tip+gc.txt.50 B3/tip+patch+gc.txt.50
name                old time/op  new time/op  delta
HTTPClientServer-4  41.3µs ± 1%  40.7µs ± 1%  -1.47%  (p=0.000 n=20+20)

I've also made 3 smaller runs with N=30s and got for them delta= -0.64% (B1), -3.48% (B2) and -1.42% (B4). It seems to get the real delta with higher probability it is required to run with bigger N and more count, but from the data present it is already clear the delta is oscilating around -1.5% or -2%.
Raw data for the benchmarks are available here: https://lab.nexedi.com/kirr/misc/tree/d13eb36c/bench/nocasgstatus/5a45a157
Hope this clarifies things.
Thanks again,
Kirill
P.S. And last time it seems I was lucky to observe the effect with default -benchtime=1s ...

That means that for benchmarks which actually generate garbage, but not so many to systematically trigger statistically sound number of actual garbage collections in default benchtime=1s, the default benchtime=1s is not appropriate as it actually does not show the cost associated with garbage collections (and casgstatus is associated 100% with the GC) even when run with -test.count=N. And BenchmarkHTTPClientServer happens to be benchmark of such kind.

@aclements could you please confirm -benchtime 50s makes a difference on your side, like it did for -1.5% - 2% for me?
Thanks beforehand,
Kirill

Atomics got a bad reputation from their early implementations, which were extremely expensive. But on a modern x86 an atomic operation on an uncontended cache line in the L1 is only ~2.5x more expensive than a plain write.

@aclements about this: I was doing something else but noted atomics are not that light, so I've benchmarked it myself. Over 20 runs it gives:
name                      time/op
EmptyBase-4               0.45ns ± 0%
StoreUncontended-4        0.50ns ± 0%
StoreContended-4          2.42ns ± 8%
AtomicStoreUncontended-4  3.37ns ±15%
AtomicStoreContended-4    16.3ns ± 0%
IncUncontended-4          0.61ns ± 6%
IncContended-4            3.41ns ± 5%
AtomicIncUncontended-4    3.41ns ±13%
AtomicIncContended-4      20.1ns ± 6%

To me it says T(StoreUncontended) = 0.05ns (= 0.50ns - 0.45ns) and T(AtomicStoreUncontended) ≈ 2.9ns which gives uncontended atomic store is ~ 58x slower than uncontended plain store. For uncontended increment T(IncUncontended) ≈ 0.16ns and T(AtomicIncUncontended) ≈ 2.96ns which gives uncontended atomic inc is ~ 18x slower than uncontended plain inc.
58x is much more than 2.5x so I decided to share. My processor is i7-6600U which is relatively recent:
http://cpubenchmark.net/cpu.php?cpu=Intel+Core+i7-6600U+%40+2.60GHz&id=2608
About noise: not sure why it is there but it does not go away after rechecking with second -test.count 20 run (CPU governer is set to performance and no other benchmarks run):
name                      time/op
EmptyBase-4               0.45ns ± 0%
StoreUncontended-4        0.51ns ± 0%
StoreContended-4          2.24ns ±14%
AtomicStoreUncontended-4  3.49ns ± 5%
AtomicStoreContended-4    16.4ns ±10%
IncUncontended-4          0.63ns ± 5%
IncContended-4            3.47ns ± 2%
AtomicIncUncontended-4    3.52ns ± 5%
AtomicIncContended-4      19.3ns ± 2%

Full benchmark source:
// Package qatomic shows how much overhead is there when using atomics
package qatomic

import (
	"sync/atomic"
	"testing"
)

// base showing how much overhead empty benchmarking loop takes itself
// the time per iteration an operation takes is thus can be estimated as:
//
//	T(BenchmarkOp) - T(BenchmarkEmptyBase)
func BenchmarkEmptyBase(b *testing.B) {
	b.RunParallel(func(pb *testing.PB) {
		for pb.Next() {
		}
	})
}

// ----------------------------------------

// plain store to uncontended memory
// made sure via compile -S the store is actually there inside loop
func BenchmarkStoreUncontended(b *testing.B) {
	b.RunParallel(func(pb *testing.PB) {
		var mem uint64
		func(pmem *uint64) {
			for i := uint64(0); pb.Next(); i++ {
				*pmem = i
			}
		}(&mem)
	})
}

// plain store to contended memory; races are ignored
// made sure via compile -S the store is actually there inside loop
func BenchmarkStoreContended(b *testing.B) {
	var mem uint64
	b.RunParallel(func(pb *testing.PB) {
		for i := uint64(0); pb.Next(); i++ {
			mem = i
		}
	})
}

// atomic store to uncontended memory
func BenchmarkAtomicStoreUncontended(b *testing.B) {
	b.RunParallel(func(pb *testing.PB) {
		var mem uint64
		for i := uint64(0); pb.Next(); i++ {
			atomic.StoreUint64(&mem, i)
		}
	})
}

// atomic store to contended memory
func BenchmarkAtomicStoreContended(b *testing.B) {
	var mem uint64
	b.RunParallel(func(pb *testing.PB) {
		for i := uint64(0); pb.Next(); i++ {
			atomic.StoreUint64(&mem, i)
		}
	})
}

// ----------------------------------------

// plain inc of uncontended memory
// made sure via compile -S it actually does `load; inc; store` inside loop
func BenchmarkIncUncontended(b *testing.B) {
	b.RunParallel(func(pb *testing.PB) {
		var mem uint64
		func(pmem *uint64) {
			for pb.Next() {
				*pmem++
			}
		}(&mem)
	})
}

// plain inc of contended memory
// made sure via compile -S it actually does `load; inc; store` inside loop
func BenchmarkIncContended(b *testing.B) {
	var mem uint64
	b.RunParallel(func(pb *testing.PB) {
		for pb.Next() {
			mem++
		}
	})
}

// atomic inc of uncontended memory
func BenchmarkAtomicIncUncontended(b *testing.B) {
	b.RunParallel(func(pb *testing.PB) {
		var mem uint64
		for pb.Next() {
			atomic.AddUint64(&mem, 1)
		}
	})
}

// atomic inc of contended memory
func BenchmarkAtomicIncContended(b *testing.B) {
	var mem uint64
	b.RunParallel(func(pb *testing.PB) {
		for pb.Next() {
			atomic.AddUint64(&mem, 1)
		}
	})
}

(https://play.golang.org/p/ujUaQUBMzO)

It's an interesting result.  In the inner loop, StoreUncontended does this:
mov    %rdx,(%rcx)
inc    %rdx

AtomicStoreUncontended does this:
 xchg   %rdx,(%rax)

So it seems to me that StoreUncontended really is going to be faster, since it doesn't have to wait for the memory load in the loop.
StoreContended and AtomicStoreContended are similarly different.
I do not know whether this means anything about real programs, which are unlikely to loop storing values to memory without ever reading them.

@ianlancetaylor thanks for feedback. Yes on amd64 atomic store and atomic swap are both implemented with XCHGQ (from sync/atomic/asm_amd64.s):
TEXT ·StoreUint64(SB),NOSPLIT,$0-16
        MOVQ    addr+0(FP), BP
        MOVQ    val+8(FP), AX
        XCHGQ   AX, 0(BP)
        RET
...
TEXT ·SwapUint64(SB),NOSPLIT,$0-24
        MOVQ    addr+0(FP), BP
        MOVQ    new+8(FP), AX
        XCHGQ   AX, 0(BP)
        MOVQ    AX, old+16(FP)
        RET

with similar code emitted by compiler with atomic intrinics.
However the more realistic IncUncontended and AtomicIncUncontended are compiled to this:
TEXT ·BenchmarkIncUncontended.func1.1(SB), NOSPLIT, $0-16 // qatomic_test.go:73
        NO_LOCAL_POINTERS
...
pc48:
        MOVQ       16(CX), DX
        DECQ       DX
        MOVQ       DX, 16(CX)
        MOVL       $1, DX
pc64:
        TESTB      DL, DL
        JEQ        pc88
        MOVQ       (AX), DX         // qatomic_test.go:75
        INCQ       DX               // <-- load; inc; store here
        MOVQ       DX, (AX)
pc77:
        MOVQ       16(CX), DX       // qatomic_test.go:74
        TESTQ      DX, DX
        JEQ        pc12
        JMP        pc48
...

TEXT ·BenchmarkAtomicIncUncontended.func1(SB), $24-8 // qatomic_test.go:94
...
pc97:
        MOVQ       16(CX), DX
        DECQ       DX
        MOVQ       DX, 16(CX)
        MOVL       $1, DX
pc113:
        TESTB      DL, DL
        JEQ        pc138
        MOVL       $1, DX
        LOCK                                // qatomic_test.go:97
        XADDQ      DX, (AX)                 // <-- atomic load; inc; store here
pc127:
        MOVQ       16(CX), DX               // qatomic_test.go:96
        TESTQ      DX, DX
        JEQ        pc61
        JMP        pc97
...

which makes the comparision fair and still it is more then an order of magnitude difference.

Also if we are talking about casgstatus which was introduced in 0a7c7ac8 ("runtime: changes to g->atomicstatus (nee status) to support concurrent GC") there the situation is more store-only like - some excerpt from the diff:
---------------------------- src/pkg/runtime/proc.c ----------------------------
@@ -1649,9 +1783,9 @@ runtime·entersyscallblock_m(void)
 	gp->syscallsp = gp->sched.sp;
 	gp->syscallpc = gp->sched.pc;
 	gp->syscallstack = gp->stackbase;
 	gp->syscallguard = gp->stackguard;
-	gp->status = Gsyscall;
+	runtime·casgstatus(gp, Grunning, Gsyscall);
 	if(gp->syscallsp < gp->syscallguard-StackGuard || gp->syscallstack < gp->syscallsp) {
 		// runtime·printf("entersyscall inconsistent %p [%p,%p]\n",
 		//	gp->syscallsp, gp->syscallguard-StackGuard, gp->syscallstack);
 		runtime·throw("entersyscall_m");
@@ -1673,9 +1807,11 @@ runtime·exitsyscall(void)
 	g->waitsince = 0;
 	if(exitsyscallfast()) {
 		// There's a cpu for us, so we can run.
 		g->m->p->syscalltick++;
-		g->status = Grunning;
+		// We need to cas the status and scan before resuming...
+		runtime·casgstatus(g, Gsyscall, Grunning);
+
 		// Garbage collector isn't running (since we are),
 		// so okay to clear gcstack and gcsp.
 		g->syscallstack = (uintptr)nil;
 		g->syscallsp = (uintptr)nil;
---------------------------- src/pkg/runtime/mgc0.c ----------------------------
@@ -539,9 +540,10 @@ markroot(ParFor *desc, uint32 i)
 			runtime·throw("markroot: bad index");
 		gp = runtime·allg[i - RootCount];
 		// remember when we've first observed the G blocked
 		// needed only to output in traceback
-		if((gp->status == Gwaiting || gp->status == Gsyscall) && gp->waitsince == 0)
+		status = runtime·readgstatus(gp);
+		if((status == Gwaiting || status == Gsyscall) && gp->waitsince == 0)
 			gp->waitsince = work.tstart;
 		// Shrink a stack if not much of it is being used.
 		runtime·shrinkstack(gp);
 		scanstack(gp);
@@ -736,15 +738,16 @@ scanstack(G *gp)
 	int32 n;
 	Stktop *stk;
 	uintptr sp, guard;

-	switch(gp->status){
+	switch(runtime·readgstatus(gp)) {
 	default:
@@ -1348,9 +1351,9 @@ runtime·gc(int32 force)
 		if(i > 0)
 			a.start_time = runtime·nanotime();
 		// switch to g0, call gc(&a), then switch back
 		g->param = &a;
-		g->status = Gwaiting;
+		runtime·casgstatus(g, Grunning, Gwaiting);
 		g->waitreason = runtime·gostringnocopy((byte*)"garbage collection");
 		runtime·mcall(mgc);
 	}

...

so to me it looks like in the hot path (e.g. many syscalls called) it was plain store and became atomic cas because slowpath (GC) likely seldomly needs it.
/cc @aclements, @RLH, @rsc, @dvyukov, @josharian once again because atomic stores were shown just above to be ~60x slower than plain store when destination is in L1 and is not contended.
See also https://github.com/golang/go/issues/19563#issuecomment-299112896 and https://github.com/golang/go/issues/19563#issuecomment-290821533 for -1.5% - -2% speedup for whole BenchmarkHTTPClientServer when casgstatus is effectively disabled.

Most of the CASes are unnecessary and should be removed. The spin waits are not necessary either.
https://codereview.appspot.com/132960044#msg17

@dvyukov thanks for feedback. It is a bit pity there is no reply from GC team.

GC team here. It isn't that we aren't monitoring this issue, it's just that we don't have any new numbers that add to the conversation.
The GC is core so changes undergo a risk to benefit analysis. Here is where this analysis now stands.
On the benefit side there are micro benchmarks but there is not a lot of evidence that replacing some atomics with normal stores show an improvement to a wide range of real world applications or a smaller set of important applications running on today's HW.
On the risk side the GC is clever and uses CASes instead of mutexes to implement a lock free state machine algorithm. This discussion suggests that the GC should be even more clever and implement a non-blocking algorithm that changes the state variable sometimes with normal stores and sometimes with atomic instructions. The bug tail on these kinds of changes risks being long and difficult.

GC team here. It isn't that we aren't monitoring this issue, it's just that we don't have any new numbers that add to the conversation.

kirr wrote about BenchmarkHTTPClientServer with nocasgstatus patch:  (https://github.com/golang/go/issues/19563#issuecomment-299112896)

@aclements could you please confirm -benchtime 50s makes a difference on your side, like it did for -1.5% - 2% for me?

ok, I assume it did not make a difference (though getting this confirmed would be more explicit)

On the benefit side there are micro benchmarks but there is not a lot of evidence that replacing some atomics with normal stores show an improvement to a wide range of real world applications or a smaller set of important applications running on today's HW.
On the risk side the GC is clever and uses CASes instead of mutexes to implement a lock free state machine algorithm. This discussion suggests that the GC should be even more clever and implement a non-blocking algorithm that changes the state variable sometimes with normal stores and sometimes with atomic instructions. The bug tail on these kinds of changes risks being long and difficult

ok GC team, thanks for feedback. Your position is clear.
Kirill

Closing, the suggested change's risks (and effort needed to ensure correctness) outweighs the potential benefits.

source: gharchive/issue

authors: RLH aclements bcmills davecheney dvyukov ianlancetaylor navytux

license_type: permissive

license_source: github-api


text:
gotype: type-checking external test files that rely on internal test files fails

I tried to use go/types to type check a package with internal test files (with package foo) and external test files (with package foo_test). The internal test files expose some symbols that the external test files consume. This works fine in go test. However,  the go/types code does not recognize the symbols exported by the internal test files.
I want to use the go/types package to write a refactoring tool that consumes external test files that rely on symbols exported by internal tests files, but I can't because of this error.
I think the issue is that the go/types import logic ignores the internal _test.go files when type-checking the external _test.go files. The go/types config https://golang.org/pkg/go/types/#Config does not seem to have a knob to import _test.go files.
What version of Go are you using (go version)?
go version go1.9 darwin/amd64

Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOOS="darwin"

What did you do?
I created a new package testgotypes with 3 files:
testgotypes.go, without this file gotype doesn't run:
package testgotypes

internal_test.go, which exports a variable for the external test:
package testgotypes

var Export = "hello"

external_test.go, which tries to access the variable exported by internal_test.go:
package testgotypes_test

import (
	"testing"

	"testgotypes"
)

func TestExport(t *testing.T) {
	if testgotypes.Export != "hello" {
		t.Error("uh oh")
	}
}

I ran tests, which worked just fine:
➜  testgotypes git:(master) go test -v
=== RUN   TestExport
--- PASS: TestExport (0.00s)
PASS
ok  	testgotypes	0.006s

Then I tried to run gotype, compiled from /usr/local/Cellar/go/1.9/libexec/src/go/types/gotype.go:
➜  testgotypes git:(master) gotype -x -t .
external_test.go:10:5: Export not declared by package testgotypes

It fails with the same error my refactoring code sees.
To sanity check gotype, I then renamed internal_test.go to internal.go to make it part of the non-test build. Then gotype is happy:
➜  testgotypes git:(master) mv internal_test.go internal.go
➜  testgotypes git:(master) gotype -x -t .

What did you expect to see?
gotype successfully type checking external_test.go.
What did you see instead?
gotype fails to type check the external_test.go file.

This is not a go/types issue, it's a gotype issue: go/types doesn't care how the files are organized, it simply accepts a set of files.
The documentation inside gotype.go says for the -t flag:
-t
	include local test files in a directory (ignored if -x is provided)

so this is at least working as internally documented. But it should be documented externally, and furthermore, this behavior seems incorrect. It appears that go test does something different.

This is not a trivial fix. An external test package imports the local package consisting of the local package and local test files. The go/command builds that package in a temporary directory and then imports it from that temporary directory to make it all work.
gotype relies on go/types to type-check the external test package. go/types simply uses an importer to get the information about the package to be tested, and that importer doesn't know that it needs that package built with its local test files. Furthermore, the existing top-level API (go/importer) doesn't have a mechanism to communicate that information. Also, if the importer is not a "source" importer, but say a "gc" importer, the importer relies on installed packages, and they are never built with the local test files.
One option might be to implement a new custom importer, only known to gotype.go, which is based on go/internal/srcimporter. The srcimporter's mode argument (currently unused) could be used to signal that local test files need to be included. But that would not work for gotype -c=gc which doesn't rely on the source importer.
Still thinking.

Punting to Go 1.11 since gotype is an unreleased program.

authors: griesemer jellevandenhooff rsc


text:
cmd/compile: CGO arm builds fail on Go 1.10beta1

What version of Go are you using (go version)?
go1.10beta1
What operating system and processor architecture are you using (go env)?
Travis CI, Linux
What did you do?
Tried to cross compile go-ethereum to ARM5/6/7/64, which builds fine on all previous versions of Go.
What did you expect to see?
Successful build.
What did you see instead?
arm-linux-gnueabi-gcc: error: unrecognized command line option '-m64'
Full build logs at https://travis-ci.org/ethereum/go-ethereum/jobs/323227026

You assume we know how to build (or how you built) go-ethereum. How can we reproduce this to debug? I'm sure it's in that travis log somewhere, but a concise repro in the bug would be nice.
/cc @ianlancetaylor

Fair enough, I'll try to cook up a repro for you.

Maybe your GOARM=5 CC=arm-linux-gnueabi-gcc go run build/ci.go install -arch arm should be CC_FOR_TARGET instead.

Hmm, the issue seems to come from go run build/ci.go (not from the cross compilation invoked within), which for some reason wants to build runtime/cgo, so my CC env var makes that fail:
$ CC=aarch64-linux-gnu-gcc go run -x -v build/ci.go install -arch arm64
WORK=/tmp/go-build716618928
runtime/cgo
mkdir -p $WORK/b034/
cd /opt/google/go/src/runtime/cgo
CGO_LDFLAGS='"-g" "-O2" "-lpthread"' /opt/google/go/pkg/tool/linux_amd64/cgo -objdir $WORK/b034/ -importpath runtime/cgo -import_runtime_cgo=false -import_syscall=false -- -I $WORK/b034/ -g -O2 -Wall -Werror ./cgo.go
# runtime/cgo
aarch64-linux-gnu-gcc: error: unrecognized command line option '-m64'

Now, I could understand why this would fail if ci.go would depend on cgo, but it does not, rather it (and all its dependency tree) is pure Go, so go run shouldn't try to build runtime/cgo, yet it does.

Note, I can fix my issue by not setting CC, rather perhaps passing the compiler as a flag to ci.go and have that set it as an env var internally before invoking the cross compilation. However, it would be nice to know why go run wants to touch CGO, without a dependency for it.

You can inspect dependencies with go list -json to see why there's a path to runtime/cgo.

(If ci.go uses os/user or net, it'll probably depend on cgo by default.)

Hmm, indeed, runtime/cgo is indeed listed as some dependency:
$ go list -json ./build/ci.go
{
	"Dir": "/work/src/github.com/ethereum/go-ethereum/build",
	"ImportPath": "command-line-arguments",
	"Name": "main",
	"Doc": "The ci command is called from Continuous Integration scripts.",
	"Root": "/work",
	"Stale": true,
	"StaleReason": "stale dependency: github.com/ethereum/go-ethereum/vendor/github.com/Azure/go-autorest/autorest",
	"GoFiles": [
		"ci.go"
	],
	"Imports": [
		"bufio",
		"bytes",
		"encoding/base64",
		"flag",
		"fmt",
		"github.com/ethereum/go-ethereum/internal/build",
		"go/parser",
		"go/token",
		"io/ioutil",
		"log",
		"os",
		"os/exec",
		"path/filepath",
		"regexp",
		"runtime",
		"strings",
		"time"
	],
	"Deps": [
		"archive/tar",
		"archive/zip",
		"bufio",
		"bytes",
		"compress/bzip2",
		"compress/flate",
		"compress/gzip",
		"compress/zlib",
		"container/list",
		"context",
		"crypto",
		"crypto/aes",
		"crypto/cipher",
		"crypto/des",
		"crypto/dsa",
		"crypto/ecdsa",
		"crypto/elliptic",
		"crypto/hmac",
		"crypto/internal/cipherhw",
		"crypto/md5",
		"crypto/rand",
		"crypto/rc4",
		"crypto/rsa",
		"crypto/sha1",
		"crypto/sha256",
		"crypto/sha512",
		"crypto/subtle",
		"crypto/tls",
		"crypto/x509",
		"crypto/x509/pkix",
		"encoding",
		"encoding/asn1",
		"encoding/base64",
		"encoding/binary",
		"encoding/hex",
		"encoding/json",
		"encoding/pem",
		"encoding/xml",
		"errors",
		"flag",
		"fmt",
		"github.com/ethereum/go-ethereum/internal/build",
		"github.com/ethereum/go-ethereum/vendor/github.com/Azure/azure-storage-go",
		"github.com/ethereum/go-ethereum/vendor/github.com/Azure/go-autorest/autorest",
		"github.com/ethereum/go-ethereum/vendor/github.com/Azure/go-autorest/autorest/azure",
		"github.com/ethereum/go-ethereum/vendor/github.com/Azure/go-autorest/autorest/date",
		"github.com/ethereum/go-ethereum/vendor/github.com/dgrijalva/jwt-go",
		"github.com/ethereum/go-ethereum/vendor/golang.org/x/crypto/cast5",
		"github.com/ethereum/go-ethereum/vendor/golang.org/x/crypto/openpgp",
		"github.com/ethereum/go-ethereum/vendor/golang.org/x/crypto/openpgp/armor",
		"github.com/ethereum/go-ethereum/vendor/golang.org/x/crypto/openpgp/elgamal",
		"github.com/ethereum/go-ethereum/vendor/golang.org/x/crypto/openpgp/errors",
		"github.com/ethereum/go-ethereum/vendor/golang.org/x/crypto/openpgp/packet",
		"github.com/ethereum/go-ethereum/vendor/golang.org/x/crypto/openpgp/s2k",
		"go/ast",
		"go/parser",
		"go/scanner",
		"go/token",
		"hash",
		"hash/adler32",
		"hash/crc32",
		"image",
		"image/color",
		"image/internal/imageutil",
		"image/jpeg",
		"internal/cpu",
		"internal/nettrace",
		"internal/poll",
		"internal/race",
		"internal/singleflight",
		"internal/syscall/unix",
		"io",
		"io/ioutil",
		"log",
		"math",
		"math/big",
		"math/bits",
		"math/rand",
		"mime",
		"mime/multipart",
		"mime/quotedprintable",
		"net",
		"net/http",
		"net/http/cookiejar",
		"net/http/httptrace",
		"net/http/internal",
		"net/textproto",
		"net/url",
		"os",
		"os/exec",
		"os/user",
		"path",
		"path/filepath",
		"reflect",
		"regexp",
		"regexp/syntax",
		"runtime",
		"runtime/cgo",
		"runtime/internal/atomic",
		"runtime/internal/sys",
		"sort",
		"strconv",
		"strings",
		"sync",
		"sync/atomic",
		"syscall",
		"text/template",
		"text/template/parse",
		"time",
		"unicode",
		"unicode/utf16",
		"unicode/utf8",
		"unsafe",
		"vendor/golang_org/x/crypto/chacha20poly1305",
		"vendor/golang_org/x/crypto/chacha20poly1305/internal/chacha20",
		"vendor/golang_org/x/crypto/cryptobyte",
		"vendor/golang_org/x/crypto/cryptobyte/asn1",
		"vendor/golang_org/x/crypto/curve25519",
		"vendor/golang_org/x/crypto/poly1305",
		"vendor/golang_org/x/net/http2/hpack",
		"vendor/golang_org/x/net/idna",
		"vendor/golang_org/x/net/lex/httplex",
		"vendor/golang_org/x/net/proxy",
		"vendor/golang_org/x/text/secure/bidirule",
		"vendor/golang_org/x/text/transform",
		"vendor/golang_org/x/text/unicode/bidi",
		"vendor/golang_org/x/text/unicode/norm"
	]
}

But why does Go 1.10beta1 want to rebuild runtime/cgo, but previous versions of Go didn't want to do that?

But why does Go 1.10beta1 want to rebuild runtime/cgo, but previous versions of Go didn't want to do that?

Because you got lucky before. Go 1.10 has proper build caching based on file contents and all relevant environment variables. Go 1.9 and earlier depended heavily on modtimes of files only (not contents) and ignored many important environment variables.

That is, in Go 1.9 and before, your runtime/cgo had already been built, so your CC=foo go run ci.go wasn't rebuilding it. But in Go 1.10, it assumes the new $CC value invalidates the cached results (whose cache key includes a different $CC) and rebuilds it.

Sweet, that's what I was also guessing based on Florin's Advent post, just wanted to confirm. Thank you very much for your time on this, and sorry for the false alarm!

authors: bradfitz karalabe


text:
x/vgo: go-import mod meta tag is not backwards compatible with old go

Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
go version go1.10.1 linux/amd64 vgo:2018-02-20.1

Specifically vgo is as of 5db81b5cd6ab5ee1dd1b1b0ee0843f194ab7b908
Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/myitcv/.mountpoints/x"
GORACE=""
GOROOT="/home/myitcv/gos"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build749051986=/tmp/go-build -gno-record-gcc-switches"
VGOMODROOT=""

What did you do?
cd `mktemp -d`
export GOPATH=$PWD
go get myitcv.io/blah2

What did you expect to see?
Success.
What did you see instead?
package myitcv.io/blah2: unrecognized import path "myitcv.io/blah2" (parse https://myitcv.io/blah2?go-get=1: multiple meta tags match import path "myitcv.io/blah2")

It was my understanding that the mod VCS meta go-import should be able to co-exist with the current git (or equivalent) ones in order that a package can be go get-able as well as vgo-compatible. E.g.:
$ curl https://myitcv.io/blah2?go-get=1

<!DOCTYPE html>

        <meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
        <meta name="go-import" content="myitcv.io git https://github.com/myitcv/x">
        <meta name="go-import" content="myitcv.io/blah2 mod https://raw.githubusercontent.com/myitcv/pubx/master">
        <meta name="go-source" content="myitcv.io https://github.com/myitcv/x/wiki https://github.com/myitcv/x/tree/master{/dir} https://github.com/myitcv/x/blob/master{/dir}/{file}#L{line}">
        <meta http-equiv="refresh" content="0; url=https://godoc.org/myitcv.io/blah2">

Redirecting to docs at godoc.org/myitcv.io/blah2...

But old go get doesn't ignore unknown VCS's, and so the two meta tags "clash".
Or have I got this wrong?
If I have got this wrong, how can a repo support both?
As a side note, the corresponding test for matchGoImport looks a bit suspicious; it doesn't actually assert that the actual error (when there is one) matches the error we want; there are a number of mismatches in the expectations vs the actual result. And there is also no expectation for this scenario of multiple matches.

Since raising this issue (that report a problem with myitcv.io/gogenerate) I've had to revert that package into a working state (so that I don't break things).
I've therefore updated this issue to use a new dummy package (myitcv.io/blah2), specific to this issue, so that the problem remains static.


text:
cmd/gofmt: format regression

Here is snippet of code:
package foo

var linuxOopses = []*oops{
	&oops{
		[]*regexp.Regexp{
			compile("INFO: lockdep is turned off"),
			compile("INFO: Stall ended before state dump start"),
			compile("INFO: NMI handler .* took too long to run"),
			compile("_INFO::"),                                       // Android can print this during boot.
			compile("INFO: sys_.* is not present in /proc/kallsyms"), // pkg/host output in debug mode
			compile("INFO: no syscalls can create resource"),         // pkg/host output in debug mode
		},
	},
}

With 1.10 formatting produces no diff, however with tip I am getting diff:
$ gofmt -d /tmp/format.go
diff -u /tmp/format.go.orig /tmp/format.go
--- /tmp/format.go.orig	2018-04-29 12:01:23.844573882 +0200
+++ /tmp/format.go	2018-04-29 12:01:23.844573882 +0200
@@ -6,7 +6,7 @@
 			compile("INFO: lockdep is turned off"),
 			compile("INFO: Stall ended before state dump start"),
 			compile("INFO: NMI handler .* took too long to run"),
-			compile("_INFO::"),                                       // Android can print this during boot.
+			compile("_INFO::"), // Android can print this during boot.
 			compile("INFO: sys_.* is not present in /proc/kallsyms"), // pkg/host output in debug mode
 			compile("INFO: no syscalls can create resource"),         // pkg/host output in debug mode
 		},

This breaks our CI.
go version devel +e0d37a33ab Sun Apr 29 09:38:32 2018 +0000 linux/amd64

/cc @griesemer

To clarify the problem: we have several Go versions on CI and users using a mix of Go versions. CI checks that all code is formatted.

bisected to https://github.com/golang/go/commit/542ea5ad91367d2b40589942cc5757a4d5f43f97

@dvyukov see also the discussion in #22695

Specific to this example: The "_INFO::" entry is just one char below the threshold. If it were "__INFO::" (for instance), the comment would be aligned. I'm not saying you should change it; but there's a threshold and this code simply is below (or above) it. This is simply "unfortunate" in this case.
Regarding the layout algorithm: It could be better, and perhaps in the future we will have a better approach (which is also why we don't want to freeze gofmt). We could trivially change the threshold but that simply moves the problem from this example to another example.
More generally: We do not want to freeze gofmt. See https://github.com/golang/go/issues/22695#issuecomment-344483632 .
I'm going to close this as "unfortunate".

authors: agnivade dvyukov griesemer jimmyfrasche


text:
cmd/compile: internal/ssa test TestNexting failing

Change 0e9f8a21f8b6534931f1ab50909161a289a0da3c (runtime,cmd/compile: pass strings and slices to convT2{E,I} by value) broke the TestNexting/gdb-opt-hist test in the cmd/compile/internal/ssa package:
$ cd src/cmd/compile/internal/ssa

$ gotip test -v -run=TestNexting/gdb-opt-hist
=== RUN   TestNexting
=== RUN   TestNexting/gdb-opt-hist
--- FAIL: TestNexting (0.45s)
    --- FAIL: TestNexting/gdb-opt-hist (0.45s)
        debug_test.go:241: step/next histories differ, diff=
            --- testdata/hist.gdb-opt.nexts	2018-10-03 18:58:59.969506742 +0200
            +++ /tmp/debug_test518566154/test-hist.gdb-opt.nexts	2018-10-14 17:18:31.424976480 +0200
            @@ -120,6 +120,7 @@
             a = 3
             n = 0
             t = 0
            +92:			fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
             91:			n += a
             90:			t += i * a
             92:			fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
            @@ -128,6 +129,7 @@
             a = 3
             n = 3
             t = 3
            +92:			fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
             91:			n += a
             90:			t += i * a
             92:			fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
            @@ -141,6 +143,7 @@
             a = 2
             n = 6
             t = 9
            +92:			fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
             91:			n += a
             90:			t += i * a
             92:			fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
            @@ -149,6 +152,7 @@
             a = 1
             n = 8
             t = 17
            +92:			fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
             91:			n += a
             90:			t += i * a
             92:			fmt.Fprintf(os.Stderr, "%d\t%d\t%d\t%d\t%d\n", i, a, n, i*a, t) //gdb-dbg=(n,i,t)
FAIL
exit status 1
FAIL	cmd/compile/internal/ssa	0.450s

Spotted on the longtest builder.
cc @randall77

Wha??? That seems to be the exact opposite of the previous fix I just did.


text:
NOT SUPPORT send SIGINT  to other process on windows?

i has a bug:
code:
p,_ :=os.FindProcess(**)
p.Signal(syscall.SIGINT)
error:
not supported by windows
go version 1.11.1 32bit on windows.

Please check the documentation - https://golang.org/pkg/os/#Process.Signal.

Signal sends a signal to the Process. Sending Interrupt on Windows is not implemented.

I am going to close this as WAI. If you would like this to happen, kindly open a new issue. Thank you.

@mattn thanks，it‘s ok！


text:
crypto/sha1: on 32-bit platforms, panic trying to Sum after a large amount of data hashed

(Note: this is the same problem as in #29517, but in the crypto/sha1 package. Fix will be similar)
What version of Go are you using (go version)?

$ go version 1.11.1

Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
go env

$ go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.1/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.1/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/w6/9bjdntr539z1v138l_8mvmqshs5f5p/T/go-build219458045=/tmp/go-build -gno-record-gcc-switches -fno-common"

Note: the issue only occurs targeting architectures for which an int is 32 bits. I can easily trigger the problem by building with GOARCH=386.
What did you do?
Hash a large amount of data (length large than a platform int), save the hash using binary.Marshal, restore it using binary.Unmarshal, and then try to Sum it.
This play.golang.org link contains a simple example that triggers the issue:
https://play.golang.org/p/-OELF0xAvll
When I run it there, the arch I see is amd64p32, so the issue occurs.
What did you expect to see?
I expect to be able to successfully get a checksum for the unmarshaled sha1 without panic.
What did you see instead?
Upon calling h.Sum(nil), I get a panic similar to:
panic: d.nx != 0

goroutine 1 [running]:
crypto/sha1.(*digest).checkSum(0x41c6e8, 0x6e12, 0x0, 0x0, 0x0, 0x0, 0x0, 0x6e12)
	/usr/local/go/src/crypto/sha1/sha1.go:177 +0x240
crypto/sha1.(*digest).Sum(0x452000, 0x0, 0x0, 0x0, 0x0, 0x0, 0x60, 0x4441c0)
	/usr/local/go/src/crypto/sha1/sha1.go:156 +0x60
main.main()
	/tmp/sandbox916984865/main.go:22 +0x1e0

Possible resolution
Fix cast in unmarshal code (as in #29517)

@jblebrun since this is closely related to #29545, could we combine them into a parent issue which tracks both? Or do you forsee two very separate fixes for the issue?
/cc @FiloSottile

I originally discovered the issue in the crypto/sha256(#29517) library. After fixing that, it occurred to me to check for the same issue in the other crypto packages that introduced the Marshal/Unmarhsal functionality, and I discovered the same issue in sha512 (#29541), sha1 (#29543), and md5(#29545), but they should all link back to the original issue (#29517)
I mostly opened these tickets to be overly conservative on the level of tracking for each issue. As a fresh new contributor, I'm happy if a more regular contributor to the project wants to combine them in a way that makes more sense, and fits in with project conventions.

authors: jblebrun katiehockman


text:
cmd/ld: gdb on windows cannot find source code

What steps will reproduce the problem?

gdb ./hello.exe

Which compiler are you using (5g, 6g, 8g, gccgo)?

go build

Which operating system are you using?

win7 64 bit

Which revision are you using?  (hg identify)

hg id
52ba9506bd99 weekly/weekly.2012-02-07

Please provide any additional information below.

$ go build -x hello
WORK=C:\Users\user1\AppData\Local\Temp\go-build887497746
mkdir -p $WORK\hello\_obj\
cd $GOROOT\gocode\src\hello
$GOBIN\tool\8g.exe -o $WORK\hello\_obj\_go_.8 -p hello -I $WORK .\helloworld.go
$GOBIN\tool\pack.exe grc $WORK\hello.a $WORK\hello\_obj\_go_.8
$GOBIN\tool\8l.exe -o $WORK\hello\_obj\a.out.exe -L $WORK $WORK\hello.a
cp $WORK\hello\_obj\a.out.exe hello.exe

USER1@user1-00 /c/goweekly/gocode/src
$ ./hello.exe
Hello, World!

USER1@user1-00 /c/goweekly/gocode/src
$ go version
go version weekly.2012-02-07 +52ba9506bd99

USER1@user1-00 /c/goweekly/gocode/src
$

What is the expected output?

Listing helloworld source codes.

What do you see instead?

USER1@user1-00 /c/goweekly/gocode/src
$ gdb hello.exe
GNU gdb (GDB) 7.2
Copyright (C) 2010 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>;
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "mingw32".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>;...
Reading symbols from c:\goweekly\gocode\src/hello.exe...done.
(gdb) l
1       : No such file or directory.
        in
(gdb) quit

Trying this in latest go.

authors: mattn tjyang


text:
internal compiler error: binaryOp: bad operation

What version of Go are you using (go version)?

$ go version

go version go1.12.2 windows/amd64

Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\hf\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\code\go
set GOPROXY=
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\hf\AppData\Local\Temp\go-build639447286=/tmp/go-build -gno-record-gcc-switches

What did you do?
https://play.golang.org/p/CAilBT0YfzY
What did you expect to see?
An error message indicating the operation is not supported on floating point values.
What did you see instead?

std\lib.go:80:10: internal compiler error: binaryOp: bad operation: 2.3 ^ 2
Please file a bug report including a short program that triggers the error.
https://golang.org/issue/new

Duplicate of https://github.com/golang/go/issues/31060, already fixed in tip

authors: cuonglm hexaflex


text:
cmd/go: /root/go as GOROOT results in "missing $GOPATH"

What version of Go are you using (go version)?

$ go version
go version go1.13 linux/amd64

Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/root/mod/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build658210724=/tmp/go-build -gno-record-gcc-switches"

What did you do?
Simply extracted latest go version into root and tried to set my GOROOT to /root/go and run go env.
What did you expect to see?
For the go commands to work as usual.
What did you see instead?
missing $gopath as an error. Moving it to /root/foo/go worked fined (that's where go env output is from).

Somehow not seeing this now :)


text:
pkg.go.dev doesn't like package names that are just a domain

What is the URL of the page with the issue?
https://pkg.go.dev/retort.dev
What is your user agent?
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:77.0) Gecko/20100101 Firefox/77.0

Screenshot

What did you do?
I’m working on a package retort.dev (https://retort.dev).
I can pull the version info of here: https://proxy.golang.org/retort.dev/@v/v0.1.5.info
But the docs don't load at all https://pkg.go.dev/retort.dev
Downloading via the proxy works:
GOPROXY="https://proxy.golang.org" GO111MODULE=on go get -u -v retort.dev

What did you expect to see?
The package documentation.
What did you see instead?
A 404 page.
Comments
I’m wondering if pkg.go.dev isn’t expecting a package at the domain level?
You can see the package repo here https://github.com/retort-dev/retort

This appears to be working now 😀


text:
x/pkgsite: add tests for frontend.LatestVersion

Add tests for frontend.latestVersion, which is used to return the latest version of a path.

@julieqiu I would like to work on this issue. can I pick this?

Please do!

@julieqiu I have gone through TestServer test from server_test.go file. I am thinking to write the test cases for latestVersion in similar way.
1. testCases := []serverTestCase{
		{
			name:           "module packages tab latest version",
			urlPath:        fmt.Sprintf("/mod/%s?tab=packages", sample.ModulePath),
			wantStatusCode: http.StatusOK,
			// Fall back to the latest version.
			want: in("",
				in("div.DetailsHeader-version", text("v1.0.0"))),
		},
	}

insertTestModules()
create a server i.e s = NewServer()
create a middeware middleware.LatestVersion(s.LatestVersion) as s.LatestVersion is intended to be a parameter of middleware.LatestVersion
call latestVersion method i.e s.latestVersion(context.Background(), packagePath, modulePath, pageType)

6. for _, tc := range testCases {
		a. create a httptest.NewRecorder()
		b. create a new httptest request to the urlPath
		c. parse the res.Body
		d. here in res.Body we can check compare the `requested` and `want` version
	}

Before moving forward with this solution, can you please let me know that is this the right approach?

Hi @julieqiu, Can I pickup this issue? If, @vikramcse is okay with it?

authors: julieqiu kirandasika98 vikramcse