Skip to content

Commit 2caf638

Browse files
prattmicgopherbot
authored andcommitted
runtime: don't use maps in js note implementation
notes are used in sensitive locations in the runtime, such as those with write barriers forbidden. Maps aren't designed for this sort of internal use. Notably, newm -> notewakeup doesn't allow write barriers, but mapaccess1 -> panic contains write barriers. The js runtime only builds right now because the map access is optimized to mapaccess1_fast64, which happens to not have a panic call. The initial swisstable map implementation doesn't have a fast64 variant. While we could add one, it is a bad idea in general to use a map in such a fragile location. Simplify the implementation by storing the metadata directly in the note, and using a linked list for checkTimeouts. For #54766. Cq-Include-Trybots: luci.golang.try:gotip-js-wasm Change-Id: Ib9d39f064ae4ad32dcc873f799428717eb6c2d5a Reviewed-on: https://go-review.googlesource.com/c/go/+/595558 LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Michael Pratt <[email protected]> Reviewed-by: Cherry Mui <[email protected]>
1 parent 38f0a82 commit 2caf638

File tree

4 files changed

+106
-66
lines changed

4 files changed

+106
-66
lines changed

src/runtime/lock_js.go

Lines changed: 33 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -63,29 +63,21 @@ func unlock2(l *mutex) {
6363

6464
// One-time notifications.
6565

66-
type noteWithTimeout struct {
67-
gp *g
68-
deadline int64
69-
}
70-
71-
var (
72-
notes = make(map[*note]*g)
73-
notesWithTimeout = make(map[*note]noteWithTimeout)
74-
)
66+
// Linked list of notes with a deadline.
67+
var allDeadlineNotes *note
7568

7669
func noteclear(n *note) {
77-
n.key = note_cleared
70+
n.status = note_cleared
7871
}
7972

8073
func notewakeup(n *note) {
81-
// gp := getg()
82-
if n.key == note_woken {
74+
if n.status == note_woken {
8375
throw("notewakeup - double wakeup")
8476
}
85-
cleared := n.key == note_cleared
86-
n.key = note_woken
77+
cleared := n.status == note_cleared
78+
n.status = note_woken
8779
if cleared {
88-
goready(notes[n], 1)
80+
goready(n.gp, 1)
8981
}
9082
}
9183

@@ -113,48 +105,50 @@ func notetsleepg(n *note, ns int64) bool {
113105
}
114106

115107
id := scheduleTimeoutEvent(delay)
116-
mp := acquirem()
117-
notes[n] = gp
118-
notesWithTimeout[n] = noteWithTimeout{gp: gp, deadline: deadline}
119-
releasem(mp)
108+
109+
n.gp = gp
110+
n.deadline = deadline
111+
if allDeadlineNotes != nil {
112+
allDeadlineNotes.allprev = n
113+
}
114+
n.allnext = allDeadlineNotes
115+
allDeadlineNotes = n
120116

121117
gopark(nil, nil, waitReasonSleep, traceBlockSleep, 1)
122118

123119
clearTimeoutEvent(id) // note might have woken early, clear timeout
124120

125-
mp = acquirem()
126-
delete(notes, n)
127-
delete(notesWithTimeout, n)
128-
releasem(mp)
121+
n.gp = nil
122+
n.deadline = 0
123+
if n.allprev != nil {
124+
n.allprev.allnext = n.allnext
125+
}
126+
if allDeadlineNotes == n {
127+
allDeadlineNotes = n.allnext
128+
}
129+
n.allprev = nil
130+
n.allnext = nil
129131

130-
return n.key == note_woken
132+
return n.status == note_woken
131133
}
132134

133-
for n.key != note_woken {
134-
mp := acquirem()
135-
notes[n] = gp
136-
releasem(mp)
135+
for n.status != note_woken {
136+
n.gp = gp
137137

138138
gopark(nil, nil, waitReasonZero, traceBlockGeneric, 1)
139139

140-
mp = acquirem()
141-
delete(notes, n)
142-
releasem(mp)
140+
n.gp = nil
143141
}
144142
return true
145143
}
146144

147145
// checkTimeouts resumes goroutines that are waiting on a note which has reached its deadline.
148-
// TODO(drchase): need to understand if write barriers are really okay in this context.
149-
//
150-
//go:yeswritebarrierrec
151146
func checkTimeouts() {
152147
now := nanotime()
153-
// TODO: map iteration has the write barriers in it; is that okay?
154-
for n, nt := range notesWithTimeout {
155-
if n.key == note_cleared && now >= nt.deadline {
156-
n.key = note_timeout
157-
goready(nt.gp, 1)
148+
for n := allDeadlineNotes; n != nil; n = n.allnext {
149+
if n.status == note_cleared && n.deadline != 0 && now >= n.deadline {
150+
n.status = note_timeout
151+
goready(n.gp, 1)
158152
}
159153
}
160154
}

src/runtime/note_js.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package runtime
6+
7+
// sleep and wakeup on one-time events.
8+
// before any calls to notesleep or notewakeup,
9+
// must call noteclear to initialize the Note.
10+
// then, exactly one thread can call notesleep
11+
// and exactly one thread can call notewakeup (once).
12+
// once notewakeup has been called, the notesleep
13+
// will return. future notesleep will return immediately.
14+
// subsequent noteclear must be called only after
15+
// previous notesleep has returned, e.g. it's disallowed
16+
// to call noteclear straight after notewakeup.
17+
//
18+
// notetsleep is like notesleep but wakes up after
19+
// a given number of nanoseconds even if the event
20+
// has not yet happened. if a goroutine uses notetsleep to
21+
// wake up early, it must wait to call noteclear until it
22+
// can be sure that no other goroutine is calling
23+
// notewakeup.
24+
//
25+
// notesleep/notetsleep are generally called on g0,
26+
// notetsleepg is similar to notetsleep but is called on user g.
27+
type note struct {
28+
status int32
29+
30+
// The G waiting on this note.
31+
gp *g
32+
33+
// Deadline, if any. 0 indicates no timeout.
34+
deadline int64
35+
36+
// allprev and allnext are used to form the allDeadlineNotes linked
37+
// list. These are unused if there is no deadline.
38+
allprev *note
39+
allnext *note
40+
}

src/runtime/note_other.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build !js
6+
7+
package runtime
8+
9+
// sleep and wakeup on one-time events.
10+
// before any calls to notesleep or notewakeup,
11+
// must call noteclear to initialize the Note.
12+
// then, exactly one thread can call notesleep
13+
// and exactly one thread can call notewakeup (once).
14+
// once notewakeup has been called, the notesleep
15+
// will return. future notesleep will return immediately.
16+
// subsequent noteclear must be called only after
17+
// previous notesleep has returned, e.g. it's disallowed
18+
// to call noteclear straight after notewakeup.
19+
//
20+
// notetsleep is like notesleep but wakes up after
21+
// a given number of nanoseconds even if the event
22+
// has not yet happened. if a goroutine uses notetsleep to
23+
// wake up early, it must wait to call noteclear until it
24+
// can be sure that no other goroutine is calling
25+
// notewakeup.
26+
//
27+
// notesleep/notetsleep are generally called on g0,
28+
// notetsleepg is similar to notetsleep but is called on user g.
29+
type note struct {
30+
// Futex-based impl treats it as uint32 key,
31+
// while sema-based impl as M* waitm.
32+
key uintptr
33+
}

src/runtime/runtime2.go

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -170,33 +170,6 @@ type mutex struct {
170170
key uintptr
171171
}
172172

173-
// sleep and wakeup on one-time events.
174-
// before any calls to notesleep or notewakeup,
175-
// must call noteclear to initialize the Note.
176-
// then, exactly one thread can call notesleep
177-
// and exactly one thread can call notewakeup (once).
178-
// once notewakeup has been called, the notesleep
179-
// will return. future notesleep will return immediately.
180-
// subsequent noteclear must be called only after
181-
// previous notesleep has returned, e.g. it's disallowed
182-
// to call noteclear straight after notewakeup.
183-
//
184-
// notetsleep is like notesleep but wakes up after
185-
// a given number of nanoseconds even if the event
186-
// has not yet happened. if a goroutine uses notetsleep to
187-
// wake up early, it must wait to call noteclear until it
188-
// can be sure that no other goroutine is calling
189-
// notewakeup.
190-
//
191-
// notesleep/notetsleep are generally called on g0,
192-
// notetsleepg is similar to notetsleep but is called on user g.
193-
type note struct {
194-
// Futex-based impl treats it as uint32 key,
195-
// while sema-based impl as M* waitm.
196-
// Used to be a union, but unions break precise GC.
197-
key uintptr
198-
}
199-
200173
type funcval struct {
201174
fn uintptr
202175
// variable-size, fn-specific data here

0 commit comments

Comments
 (0)