From e93a502c7ff05d3f5b5fd37510e065f0f7ceb630 Mon Sep 17 00:00:00 2001 From: Nevkontakte Date: Sun, 7 Mar 2021 22:55:05 +0000 Subject: [PATCH 1/6] Support Go 1.13-1.16 changes in the sync package. Go 1.13 introduced two more low-level fields that would've been referenced by essentially dead code, so I decided to replace original pool.go entirely. As a byproduct, this might yield a tiny decrease in output size (assuming DCE hasn't been pruning the unexported methods away already). --- build/build.go | 4 ++++ compiler/natives/src/sync/pool.go | 26 ++++++++++++++++++-------- compiler/natives/src/sync/sync.go | 6 +++--- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/build/build.go b/build/build.go index ccd974504..256524a6b 100644 --- a/build/build.go +++ b/build/build.go @@ -183,6 +183,10 @@ func importWithSrcDir(bctx build.Context, path string, srcDir string, mode build pkg.GoFiles = nil case "internal/poll": pkg.GoFiles = exclude(pkg.GoFiles, "fd_poll_runtime.go") + case "sync": + // GopherJS completely replaces sync.Pool implementation with a simpler one, + // since it always executes in a single-threaded environment. + pkg.GoFiles = exclude(pkg.GoFiles, "pool.go") case "crypto/rand": pkg.GoFiles = []string{"rand.go", "util.go"} pkg.TestGoFiles = exclude(pkg.TestGoFiles, "rand_linux_test.go") // Don't want linux-specific tests (since linux-specific package files are excluded too). diff --git a/compiler/natives/src/sync/pool.go b/compiler/natives/src/sync/pool.go index 629010d9e..842bc3534 100644 --- a/compiler/natives/src/sync/pool.go +++ b/compiler/natives/src/sync/pool.go @@ -2,12 +2,25 @@ package sync -import "unsafe" - +// A Pool is a set of temporary objects that may be individually saved and +// retrieved. +// +// GopherJS provides a simpler, naive implementation with no synchronization at +// all. This is still correct for the GopherJS runtime because: +// +// 1. JavaScript is single-threaded, so it is impossible for two threads to be +// accessing the pool at the same moment in time. +// 2. GopherJS goroutine implementation uses cooperative multi-tasking model, +// which only allows passing control to other goroutines when the function +// might block. +// +// TODO(nevkontakte): Consider adding a mutex just to be safe if it doesn't +// create a large performance hit. +// +// Note: there is a special handling in the gopherjs/build package that filters +// out all original Pool implementation in order to avoid awkward unused fields +// referenced by dead code. type Pool struct { - local unsafe.Pointer - localSize uintptr - store []interface{} New func() interface{} } @@ -30,6 +43,3 @@ func (p *Pool) Put(x interface{}) { } p.store = append(p.store, x) } - -func runtime_registerPoolCleanup(cleanup func()) { -} diff --git a/compiler/natives/src/sync/sync.go b/compiler/natives/src/sync/sync.go index 2ae46e0a6..9d7f71a19 100644 --- a/compiler/natives/src/sync/sync.go +++ b/compiler/natives/src/sync/sync.go @@ -18,13 +18,13 @@ var semWaiters = make(map[*uint32][]chan bool) var semAwoken = make(map[*uint32]uint32) func runtime_Semacquire(s *uint32) { - runtime_SemacquireMutex(s, false) + runtime_SemacquireMutex(s, false, 1) } // SemacquireMutex is like Semacquire, but for profiling contended Mutexes. // Mutex profiling is not supported, so just use the same implementation as runtime_Semacquire. // TODO: Investigate this. If it's possible to implement, consider doing so, otherwise remove this comment. -func runtime_SemacquireMutex(s *uint32, lifo bool) { +func runtime_SemacquireMutex(s *uint32, lifo bool, skipframes int) { if (*s - semAwoken[s]) == 0 { ch := make(chan bool) if lifo { @@ -41,7 +41,7 @@ func runtime_SemacquireMutex(s *uint32, lifo bool) { *s-- } -func runtime_Semrelease(s *uint32, handoff bool) { +func runtime_Semrelease(s *uint32, handoff bool, skipframes int) { // TODO: Use handoff if needed/possible. *s++ From 2fd003e1ee88cbad0606710cc18d2f9defee5ac7 Mon Sep 17 00:00:00 2001 From: visualfc Date: Sat, 8 Feb 2020 17:40:43 +0800 Subject: [PATCH 2/6] Fix math package conflict with natives introduced in Go 1.14. math.FMA() implementation introduced a non-exported function zero(), which conflicted with out own variable. Resolved the conflict by renaming our variable. Original commit by @visualfc: https://github.com/goplusjs/gopherjs/commit/ad4d281280de0bfe947dcaadc4591fb620376813 --- compiler/natives/src/math/math.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/natives/src/math/math.go b/compiler/natives/src/math/math.go index ca09bfcd0..3b5cd5fc6 100644 --- a/compiler/natives/src/math/math.go +++ b/compiler/natives/src/math/math.go @@ -7,10 +7,10 @@ import ( ) var math = js.Global.Get("Math") -var zero float64 = 0 -var posInf = 1 / zero -var negInf = -1 / zero -var nan = 0 / zero +var _zero float64 = 0 +var posInf = 1 / _zero +var negInf = -1 / _zero +var nan = 0 / _zero func Acos(x float64) float64 { return math.Call("acos", x).Float() From b9a8c2e0b21759725d9d3f42c8ea5a67777be24a Mon Sep 17 00:00:00 2001 From: Nevkontakte Date: Mon, 8 Mar 2021 00:20:03 +0000 Subject: [PATCH 3/6] Fix `internal/syscall/unix` to compile with Go 1.16. Looks like a few new functions were added to the package, with a few new constants. Neither of them are really relevant in the JS context and arguably we shouldn't be compiling this code in the first place (see issue #991), but this lets the package compile for now. --- compiler/natives/src/internal/syscall/unix/unix.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/compiler/natives/src/internal/syscall/unix/unix.go b/compiler/natives/src/internal/syscall/unix/unix.go index 583259037..e63aaca04 100644 --- a/compiler/natives/src/internal/syscall/unix/unix.go +++ b/compiler/natives/src/internal/syscall/unix/unix.go @@ -4,8 +4,12 @@ package unix import "syscall" -const randomTrap = 0 -const fstatatTrap = 0 +const ( + randomTrap uintptr = 0 + fstatatTrap uintptr = 0 + getrandomTrap uintptr = 0 + copyFileRangeTrap uintptr = 0 +) func IsNonblock(fd int) (nonblocking bool, err error) { return false, nil From 9d3b114f490bb574427ac43c18a6aa102dfe948e Mon Sep 17 00:00:00 2001 From: Nevkontakte Date: Mon, 8 Mar 2021 00:57:16 +0000 Subject: [PATCH 4/6] Fix `time` package to compile with Go 1.16. I'm not sure of the role of the seq field plays outside of the time package, but I'll add it there. In the upstream code the runtimeTimer struct seems to replicate (and require to be compatible with) runtime.timer struct, however in GopherJS it is clearly not and it doesn't seem to be a problem. Similarly, in the upstream startTimer, stopTimer, resetTimer and modTimer are actually implemented in the `runtime` package, but since we already re-implemented two of them here, I'll just keep this package self-contained. --- compiler/natives/src/time/time.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/compiler/natives/src/time/time.go b/compiler/natives/src/time/time.go index e89889775..f99c12f30 100644 --- a/compiler/natives/src/time/time.go +++ b/compiler/natives/src/time/time.go @@ -21,6 +21,7 @@ type runtimeTimer struct { period int64 f func(interface{}, uintptr) arg interface{} + seq uintptr timeout *js.Object active bool } @@ -79,6 +80,22 @@ func stopTimer(t *runtimeTimer) bool { return wasActive } +func modTimer(t *runtimeTimer, when, period int64, f func(interface{}, uintptr), arg interface{}, seq uintptr) { + stopTimer(t) + t.when = when + t.period = period + t.f = f + t.arg = arg + t.seq = seq + startTimer(t) +} + +func resetTimer(t *runtimeTimer, when int64) bool { + wasActive := t.active + modTimer(t, when, t.period, t.f, t.arg, t.seq) + return wasActive +} + func forceZipFileForTesting(zipOnly bool) { } From 1a175f7a1fe8cbf59c0f07013f37f7f6fa8195e9 Mon Sep 17 00:00:00 2001 From: Nevkontakte Date: Mon, 8 Mar 2021 01:43:26 +0000 Subject: [PATCH 5/6] Fix `os` and `internal/poll` packages to build with Go 1.16. --- build/build.go | 4 ++++ compiler/natives/src/internal/poll/fd_poll.go | 2 +- compiler/natives/src/os/os.go | 2 ++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/build/build.go b/build/build.go index 256524a6b..3134690e2 100644 --- a/build/build.go +++ b/build/build.go @@ -175,6 +175,10 @@ func importWithSrcDir(bctx build.Context, path string, srcDir string, mode build switch path { case "os": pkg.GoFiles = excludeExecutable(pkg.GoFiles) // Need to exclude executable implementation files, because some of them contain package scope variables that perform (indirectly) syscalls on init. + // Prefer dirent_js.go version, since it targets a similar environment to + // ours. Arguably this file should be excluded by the build tags (see + // https://github.com/gopherjs/gopherjs/issues/991). + pkg.GoFiles = exclude(pkg.GoFiles, "dirent_linux.go") case "runtime": pkg.GoFiles = []string{} // Package sources are completely replaced in natives. case "runtime/internal/sys": diff --git a/compiler/natives/src/internal/poll/fd_poll.go b/compiler/natives/src/internal/poll/fd_poll.go index 5690f9fe8..afaf76413 100644 --- a/compiler/natives/src/internal/poll/fd_poll.go +++ b/compiler/natives/src/internal/poll/fd_poll.go @@ -33,7 +33,7 @@ func (pd *pollDesc) wait(mode int, isFile bool) error { if pd.closing { return errClosing(isFile) } - return ErrTimeout + return ErrDeadlineExceeded } func (pd *pollDesc) waitRead(isFile bool) error { return pd.wait('r', isFile) } diff --git a/compiler/natives/src/os/os.go b/compiler/natives/src/os/os.go index 96f36d28b..94ed0c581 100644 --- a/compiler/natives/src/os/os.go +++ b/compiler/natives/src/os/os.go @@ -8,6 +8,8 @@ import ( "github.com/gopherjs/gopherjs/js" ) +const isBigEndian = false + func runtime_args() []string { // not called on Windows return Args } From 1d627d5b256ed83106bbb34e4f113faa840ecc25 Mon Sep 17 00:00:00 2001 From: Nevkontakte Date: Tue, 9 Mar 2021 14:41:02 +0000 Subject: [PATCH 6/6] Update the comment to reference the correct issue. Co-authored-by: Jonathan Hall --- build/build.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/build.go b/build/build.go index 3134690e2..e9d5d5272 100644 --- a/build/build.go +++ b/build/build.go @@ -177,7 +177,7 @@ func importWithSrcDir(bctx build.Context, path string, srcDir string, mode build pkg.GoFiles = excludeExecutable(pkg.GoFiles) // Need to exclude executable implementation files, because some of them contain package scope variables that perform (indirectly) syscalls on init. // Prefer dirent_js.go version, since it targets a similar environment to // ours. Arguably this file should be excluded by the build tags (see - // https://github.com/gopherjs/gopherjs/issues/991). + // https://github.com/gopherjs/gopherjs/issues/693). pkg.GoFiles = exclude(pkg.GoFiles, "dirent_linux.go") case "runtime": pkg.GoFiles = []string{} // Package sources are completely replaced in natives.