Skip to content

Commit 130fdc1

Browse files
committed
refactor: bump to go1.20, use atomic.Pointer, unsafe.Slice/String, generics
- Update go.mod to go 1.20 - descmap: use per-slot atomic.Pointer[T] instead of single atomic pointer to entire buckets array, only realloc the affected slot on Set - hack: simplify newMapIter using rv.MapRange() (go1.19), remove stringHeader, use unsafe.SliceData for zerobase (go1.20) - decoder: replace stringHeader/sliceHeader with unsafe.String and unsafe.Slice for string/binary decoding (go1.20) - utils: inline copyn with unsafe.Slice, replace 7 checkUniqueness functions with single generic checkUnique[T comparable] - unknownfields: use unsafe.Slice instead of sliceHeader manipulation - Add descmap unit tests
1 parent 62b18e0 commit 130fdc1

File tree

8 files changed

+170
-173
lines changed

8 files changed

+170
-173
lines changed

.github/workflows/tests.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ jobs:
66
unittest-amd64:
77
strategy:
88
matrix:
9-
go: [ "1.18", oldstable, stable ]
9+
go: [ "1.20", oldstable, stable ]
1010
os: [ ubuntu-latest, macos-latest ]
1111
runs-on: ${{ matrix.os }}
1212
steps:

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/cloudwego/frugal
22

3-
go 1.18
3+
go 1.20
44

55
require (
66
github.com/cloudwego/gopkg v0.1.2

internal/reflect/decoder.go

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -182,14 +182,9 @@ func decodeStringNoCopy(t *tType, b []byte, p unsafe.Pointer) (i int, err error)
182182
}
183183

184184
if t.Tag == defs.T_binary {
185-
h := (*sliceHeader)(p)
186-
h.Data = unsafe.Pointer(&b[i])
187-
h.Len = l
188-
h.Cap = l
189-
} else { // convert to str
190-
h := (*stringHeader)(p)
191-
h.Data = unsafe.Pointer(&b[i])
192-
h.Len = l
185+
*(*[]byte)(p) = unsafe.Slice(&b[i], l)
186+
} else {
187+
*(*string)(p) = unsafe.String(&b[i], l)
193188
}
194189
i += l
195190
return
@@ -224,16 +219,11 @@ func (d *tDecoder) decodeType(t *tType, b []byte, p unsafe.Pointer, maxdepth int
224219

225220
x := d.Malloc(l, 1, 0)
226221
if t.Tag == defs.T_binary {
227-
h := (*sliceHeader)(p)
228-
h.Data = x
229-
h.Len = l
230-
h.Cap = l
231-
} else { // convert to str
232-
h := (*stringHeader)(p)
233-
h.Data = x
234-
h.Len = l
222+
*(*[]byte)(p) = unsafe.Slice((*byte)(x), l)
223+
} else {
224+
*(*string)(p) = unsafe.String((*byte)(x), l)
235225
}
236-
copyn(x, b[i:], l)
226+
copy(unsafe.Slice((*byte)(x), l), b[i:])
237227
i += l
238228
return i, nil
239229

internal/reflect/descmap.go

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,13 @@
1616

1717
package reflect
1818

19-
import (
20-
"sync/atomic"
21-
"unsafe"
22-
)
19+
import "sync/atomic"
2320

2421
// mapStructDesc represents a read-lock-free hashmap for *structDesc like sync.Map.
2522
// it's NOT designed for writes.
23+
// Each slot is an atomic.Pointer so that Set only reallocs the target slot.
2624
type mapStructDesc struct {
27-
p unsafe.Pointer // for atomic, point to hashtable
25+
slots [mapStructDescBuckets + 1]atomic.Pointer[[]mapStructDescItem]
2826
}
2927

3028
// XXX: fixed size to make it simple,
@@ -37,19 +35,18 @@ type mapStructDescItem struct {
3735
}
3836

3937
func newMapStructDesc() *mapStructDesc {
40-
m := &mapStructDesc{}
41-
buckets := make([][]mapStructDescItem, mapStructDescBuckets+1) // [0] - [0xffff]
42-
atomic.StorePointer(&m.p, unsafe.Pointer(&buckets))
43-
return m
38+
return &mapStructDesc{}
4439
}
4540

4641
// Get ...
4742
func (m *mapStructDesc) Get(abiType uintptr) *structDesc {
48-
buckets := *(*[][]mapStructDescItem)(atomic.LoadPointer(&m.p))
49-
dd := buckets[abiType&mapStructDescBuckets]
50-
for i := range dd {
51-
if dd[i].abiType == abiType {
52-
return dd[i].sd
43+
slot := m.slots[abiType&mapStructDescBuckets].Load()
44+
if slot == nil {
45+
return nil
46+
}
47+
for i := range *slot {
48+
if (*slot)[i].abiType == abiType {
49+
return (*slot)[i].sd
5350
}
5451
}
5552
return nil
@@ -61,10 +58,21 @@ func (m *mapStructDesc) Set(abiType uintptr, sd *structDesc) {
6158
if m.Get(abiType) == sd {
6259
return
6360
}
64-
oldBuckets := *(*[][]mapStructDescItem)(atomic.LoadPointer(&m.p))
65-
newBuckets := make([][]mapStructDescItem, mapStructDescBuckets+1)
66-
copy(newBuckets, oldBuckets)
6761
bk := abiType & mapStructDescBuckets
68-
newBuckets[bk] = append(newBuckets[bk], mapStructDescItem{abiType: abiType, sd: sd})
69-
atomic.StorePointer(&m.p, unsafe.Pointer(&newBuckets))
62+
var old []mapStructDescItem
63+
if p := m.slots[bk].Load(); p != nil {
64+
old = *p
65+
}
66+
// alloc cap=len+1 upfront so append below won't realloc.
67+
items := make([]mapStructDescItem, len(old), len(old)+1)
68+
copy(items, old)
69+
for i := range items {
70+
if items[i].abiType == abiType {
71+
items[i].sd = sd
72+
m.slots[bk].Store(&items)
73+
return
74+
}
75+
}
76+
items = append(items, mapStructDescItem{abiType: abiType, sd: sd})
77+
m.slots[bk].Store(&items)
7078
}

internal/reflect/descmap_test.go

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* Copyright 2024 CloudWeGo Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package reflect
18+
19+
import (
20+
"testing"
21+
22+
"github.com/stretchr/testify/assert"
23+
)
24+
25+
func TestMapStructDesc_GetEmpty(t *testing.T) {
26+
m := newMapStructDesc()
27+
assert.Nil(t, m.Get(1))
28+
assert.Nil(t, m.Get(0))
29+
assert.Nil(t, m.Get(0xffff))
30+
}
31+
32+
func TestMapStructDesc_SetGet(t *testing.T) {
33+
m := newMapStructDesc()
34+
sd1 := &structDesc{}
35+
sd2 := &structDesc{}
36+
37+
m.Set(1, sd1)
38+
assert.Equal(t, sd1, m.Get(1))
39+
assert.Nil(t, m.Get(2))
40+
41+
m.Set(2, sd2)
42+
assert.Equal(t, sd1, m.Get(1))
43+
assert.Equal(t, sd2, m.Get(2))
44+
}
45+
46+
func TestMapStructDesc_Update(t *testing.T) {
47+
m := newMapStructDesc()
48+
sd1 := &structDesc{}
49+
sd2 := &structDesc{}
50+
51+
m.Set(1, sd1)
52+
assert.Equal(t, sd1, m.Get(1))
53+
54+
// update same key with new value
55+
m.Set(1, sd2)
56+
assert.Equal(t, sd2, m.Get(1))
57+
58+
// slot should have exactly 1 item, not 2
59+
bk := uintptr(1) & mapStructDescBuckets
60+
slot := m.slots[bk].Load()
61+
assert.Equal(t, 1, len(*slot))
62+
}
63+
64+
func TestMapStructDesc_SetNoop(t *testing.T) {
65+
m := newMapStructDesc()
66+
sd := &structDesc{}
67+
68+
m.Set(1, sd)
69+
// set same key+value again should be a noop
70+
m.Set(1, sd)
71+
72+
bk := uintptr(1) & mapStructDescBuckets
73+
slot := m.slots[bk].Load()
74+
assert.Equal(t, 1, len(*slot))
75+
}
76+
77+
func TestMapStructDesc_HashCollision(t *testing.T) {
78+
m := newMapStructDesc()
79+
sd1 := &structDesc{}
80+
sd2 := &structDesc{}
81+
82+
// keys that map to the same bucket
83+
k1 := uintptr(1)
84+
k2 := k1 + mapStructDescBuckets + 1 // same bucket as k1
85+
86+
m.Set(k1, sd1)
87+
m.Set(k2, sd2)
88+
89+
assert.Equal(t, sd1, m.Get(k1))
90+
assert.Equal(t, sd2, m.Get(k2))
91+
92+
// both in the same slot
93+
bk := k1 & mapStructDescBuckets
94+
slot := m.slots[bk].Load()
95+
assert.Equal(t, 2, len(*slot))
96+
}
97+
98+
func TestMapStructDesc_UpdateWithCollision(t *testing.T) {
99+
m := newMapStructDesc()
100+
sd1 := &structDesc{}
101+
sd2 := &structDesc{}
102+
sd3 := &structDesc{}
103+
104+
k1 := uintptr(1)
105+
k2 := k1 + mapStructDescBuckets + 1
106+
107+
m.Set(k1, sd1)
108+
m.Set(k2, sd2)
109+
110+
// update k1, should not affect k2
111+
m.Set(k1, sd3)
112+
assert.Equal(t, sd3, m.Get(k1))
113+
assert.Equal(t, sd2, m.Get(k2))
114+
115+
bk := k1 & mapStructDescBuckets
116+
slot := m.slots[bk].Load()
117+
assert.Equal(t, 2, len(*slot))
118+
}

internal/reflect/hack.go

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -150,14 +150,8 @@ type mapIter struct {
150150
}
151151

152152
// newMapIter creates reflect.MapIter for reflect.Value.
153-
// for go1.17, go1.18, rv.MapRange() will cause one more allocation
154-
// for >=go1.19, can use rv.MapRange() directly.
155-
// see: https://github.com/golang/go/commit/c5edd5f616b4ee4bbaefdb1579c6078e7ed7e84e
156-
// TODO: remove this func, and use mapIter{rv.MapRange()} when >=go1.19
157153
func newMapIter(rv reflect.Value) mapIter {
158-
ret := mapIter{}
159-
(*hackMapIter)(unsafe.Pointer(&ret.MapIter)).m = rv
160-
return ret
154+
return mapIter{*rv.MapRange()}
161155
}
162156

163157
func (m *mapIter) Next() (unsafe.Pointer, unsafe.Pointer) {
@@ -220,26 +214,16 @@ func rtTypePtr(rt reflect.Type) uintptr {
220214
return (*iface)(unsafe.Pointer(&rt)).data
221215
}
222216

223-
// same as reflect.StringHeader
224-
type stringHeader struct {
225-
Data unsafe.Pointer
226-
Len int
227-
}
228-
229-
// same as reflect.SliceHeader
230217
type sliceHeader struct {
231218
Data unsafe.Pointer
232219
Len int
233220
Cap int
234221
}
235222

236-
var (
237-
emptyslice = make([]byte, 0)
238-
239-
// for slice, Data should points to zerobase var in `runtime`
240-
// so that it can represent as []type{} instead of []type(nil)
241-
zerobase = ((*sliceHeader)(unsafe.Pointer(&emptyslice))).Data
242-
)
223+
// zerobase is the Data pointer of an empty slice.
224+
// For slice, Data should point to the zerobase var in `runtime`
225+
// so that it can represent as []type{} instead of []type(nil).
226+
var zerobase = unsafe.Pointer(unsafe.SliceData(make([]byte, 0)))
243227

244228
func (h *sliceHeader) Zero() {
245229
h.Len = 0

internal/reflect/unknownfields.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,7 @@ func (p *unknownFields) Size() int {
4848
func (p *unknownFields) Copy(b []byte) []byte {
4949
sz := p.Size()
5050
data := mallocgc(uintptr(sz), 0, false) // without zeroing
51-
ret := []byte{}
52-
h := (*sliceHeader)(unsafe.Pointer(&ret))
53-
h.Data = data
54-
h.Len = sz
55-
h.Cap = sz
51+
ret := unsafe.Slice((*byte)(data), sz)
5652
off := 0
5753
for _, x := range p.offs {
5854
copy(ret[off:], b[x.off:x.off+x.sz])

0 commit comments

Comments
 (0)