From e19fb93e71b5b0d379cf868799c4b94bd0a4905b Mon Sep 17 00:00:00 2001 From: HyungrakJo Date: Thu, 11 Aug 2022 17:41:46 +0900 Subject: [PATCH 1/4] remove mutex lock in a goroutine lifecycle --- context.go | 60 +++++++++++++++++++++++++++++++++++---------------- gid.go | 10 +++++++-- id_pool.go | 30 +++++++++++--------------- stack_tags.go | 50 +++++++++++++++++++++--------------------- 4 files changed, 88 insertions(+), 62 deletions(-) diff --git a/context.go b/context.go index 618a171..f0d46de 100644 --- a/context.go +++ b/context.go @@ -5,6 +5,11 @@ import ( "sync" ) +const ( + initialMaxGoroutineCount = 1024 + extendUnit = 128 +) + var ( mgrRegistry = make(map[*ContextManager]bool) mgrRegistryMtx sync.RWMutex @@ -20,15 +25,17 @@ type Values map[interface{}]interface{} // class of context variables. You should use NewContextManager for // construction. type ContextManager struct { - mtx sync.Mutex - values map[uint]Values + extendLock sync.RWMutex + values []Values + currentMaxGoroutineCount int } // NewContextManager returns a brand new ContextManager. It also registers the // new ContextManager in the ContextManager registry which is used by the Go // method. ContextManagers are typically defined globally at package scope. func NewContextManager() *ContextManager { - mgr := &ContextManager{values: make(map[uint]Values)} + mgr := &ContextManager{values: make([]Values, initialMaxGoroutineCount)} + mgr.currentMaxGoroutineCount = len(mgr.values) mgrRegistryMtx.Lock() defer mgrRegistryMtx.Unlock() mgrRegistry[mgr] = true @@ -60,14 +67,17 @@ func (m *ContextManager) SetValues(new_values Values, context_call func()) { mutated_keys := make([]interface{}, 0, len(new_values)) mutated_vals := make(Values, len(new_values)) - EnsureGoroutineId(func(gid uint) { - m.mtx.Lock() - state, found := m.values[gid] - if !found { + EnsureGoroutineId(func(gid uint32) { + var found bool + m.extendIfNeeded(gid) + + state := m.values[gid] + if state != nil { + found = true + } else { state = make(Values, len(new_values)) m.values[gid] = state } - m.mtx.Unlock() for key, new_val := range new_values { mutated_keys = append(mutated_keys, key) @@ -79,9 +89,7 @@ func (m *ContextManager) SetValues(new_values Values, context_call func()) { defer func() { if !found { - m.mtx.Lock() - delete(m.values, gid) - m.mtx.Unlock() + m.values[gid] = nil return } @@ -108,11 +116,9 @@ func (m *ContextManager) GetValue(key interface{}) ( return nil, false } - m.mtx.Lock() - state, found := m.values[gid] - m.mtx.Unlock() + state := m.values[gid] - if !found { + if state == nil { return nil, false } value, ok = state[key] @@ -124,9 +130,7 @@ func (m *ContextManager) getValues() Values { if !ok { return nil } - m.mtx.Lock() - state, _ := m.values[gid] - m.mtx.Unlock() + state := m.values[gid] return state } @@ -151,3 +155,23 @@ func Go(cb func()) { go cb() } + +func (m *ContextManager) extend(gid uint32) { + m.extendLock.Lock() + defer m.extendLock.Unlock() + if gid >= uint32(m.currentMaxGoroutineCount) { + unit := ((gid-uint32(m.currentMaxGoroutineCount))/extendUnit + 1) * extendUnit + m.values = append(m.values, make([]Values, unit)...) + m.currentMaxGoroutineCount += int(unit) + } +} + +func (m *ContextManager) extendIfNeeded(gid uint32) { + m.extendLock.RLock() + if gid >= uint32(m.currentMaxGoroutineCount) { + m.extendLock.RUnlock() + m.extend(gid) + } else { + m.extendLock.RUnlock() + } +} diff --git a/gid.go b/gid.go index c16bf3a..1d42523 100644 --- a/gid.go +++ b/gid.go @@ -4,17 +4,23 @@ var ( stackTagPool = &idPool{} ) +func initIdPool() { + stackTagPool.Pool.New = func() interface{} { + return stackTagPool.newID() + } +} + // Will return this goroutine's identifier if set. If you always need a // goroutine identifier, you should use EnsureGoroutineId which will make one // if there isn't one already. -func GetGoroutineId() (gid uint, ok bool) { +func GetGoroutineId() (gid uint32, ok bool) { return readStackTag() } // Will call cb with the current goroutine identifier. If one hasn't already // been generated, one will be created and set first. The goroutine identifier // might be invalid after cb returns. -func EnsureGoroutineId(cb func(gid uint)) { +func EnsureGoroutineId(cb func(gid uint32)) { if gid, ok := readStackTag(); ok { cb(gid) return diff --git a/id_pool.go b/id_pool.go index b7974ae..9d46fb4 100644 --- a/id_pool.go +++ b/id_pool.go @@ -6,29 +6,23 @@ package gls import ( "sync" + "sync/atomic" ) type idPool struct { - mtx sync.Mutex - released []uint - max_id uint + sync.Pool + curID uint32 } -func (p *idPool) Acquire() (id uint) { - p.mtx.Lock() - defer p.mtx.Unlock() - if len(p.released) > 0 { - id = p.released[len(p.released)-1] - p.released = p.released[:len(p.released)-1] - return id - } - id = p.max_id - p.max_id++ - return id +func (p *idPool) newID() uint32 { + curID := atomic.AddUint32(&p.curID, 1) + return curID - 1 } -func (p *idPool) Release(id uint) { - p.mtx.Lock() - defer p.mtx.Unlock() - p.released = append(p.released, id) +func (p *idPool) Acquire() (id uint32) { + return p.Get().(uint32) +} + +func (p *idPool) Release(id uint32) { + p.Put(id) } diff --git a/stack_tags.go b/stack_tags.go index 37bbd33..39519ff 100644 --- a/stack_tags.go +++ b/stack_tags.go @@ -9,11 +9,11 @@ const ( var ( pc_lookup = make(map[uintptr]int8, 17) - mark_lookup [16]func(uint, func()) + mark_lookup [16]func(uint32, func()) ) func init() { - setEntries := func(f func(uint, func()), v int8) { + setEntries := func(f func(uint32, func()), v int8) { var ptr uintptr f(0, func() { ptr = findPtr() @@ -40,9 +40,11 @@ func init() { setEntries(github_com_jtolds_gls_markD, 0xd) setEntries(github_com_jtolds_gls_markE, 0xe) setEntries(github_com_jtolds_gls_markF, 0xf) + + initIdPool() } -func addStackTag(tag uint, context_call func()) { +func addStackTag(tag uint32, context_call func()) { if context_call == nil { return } @@ -53,57 +55,57 @@ func addStackTag(tag uint, context_call func()) { // is easier. it shouldn't add any runtime cost in non-js builds. //go:noinline -func github_com_jtolds_gls_markS(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_markS(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_mark0(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_mark0(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_mark1(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_mark1(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_mark2(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_mark2(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_mark3(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_mark3(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_mark4(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_mark4(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_mark5(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_mark5(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_mark6(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_mark6(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_mark7(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_mark7(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_mark8(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_mark8(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_mark9(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_mark9(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_markA(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_markA(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_markB(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_markB(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_markC(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_markC(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_markD(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_markD(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_markE(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_markE(tag uint32, cb func()) { _m(tag, cb) } //go:noinline -func github_com_jtolds_gls_markF(tag uint, cb func()) { _m(tag, cb) } +func github_com_jtolds_gls_markF(tag uint32, cb func()) { _m(tag, cb) } -func _m(tag_remainder uint, cb func()) { +func _m(tag_remainder uint32, cb func()) { if tag_remainder == 0 { cb() } else { @@ -111,8 +113,8 @@ func _m(tag_remainder uint, cb func()) { } } -func readStackTag() (tag uint, ok bool) { - var current_tag uint +func readStackTag() (tag uint32, ok bool) { + var current_tag uint32 offset := 0 for { batch, next_offset := getStack(offset, stackBatchSize) @@ -125,7 +127,7 @@ func readStackTag() (tag uint, ok bool) { return current_tag, true } current_tag <<= bitWidth - current_tag += uint(val) + current_tag += uint32(val) } if next_offset == 0 { break From 2a87cc3bb0cec3c97e0492fd2e578d1d63962c76 Mon Sep 17 00:00:00 2001 From: HyungrakJo Date: Thu, 11 Aug 2022 18:47:51 +0900 Subject: [PATCH 2/4] modify benchmark test to scenario of competing value access between goroutines --- context_test.go | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/context_test.go b/context_test.go index 2fa426c..af000d0 100644 --- a/context_test.go +++ b/context_test.go @@ -124,22 +124,34 @@ func ExampleGo() { func BenchmarkGetValue(b *testing.B) { mgr := gls.NewContextManager() + wg := sync.WaitGroup{} mgr.SetValues(gls.Values{"test_key": "test_val"}, func() { b.ResetTimer() for i := 0; i < b.N; i++ { - val, ok := mgr.GetValue("test_key") - if !ok || val != "test_val" { - b.FailNow() - } + wg.Add(1) + gls.Go(func() { + defer wg.Done() + val, ok := mgr.GetValue("test_key") + if !ok || val != "test_val" { + b.FailNow() + } + }) } + wg.Wait() }) } func BenchmarkSetValues(b *testing.B) { mgr := gls.NewContextManager() + wg := sync.WaitGroup{} for i := 0; i < b.N/2; i++ { - mgr.SetValues(gls.Values{"test_key": "test_val"}, func() { - mgr.SetValues(gls.Values{"test_key2": "test_val2"}, func() {}) - }) + wg.Add(1) + go func() { + defer wg.Done() + mgr.SetValues(gls.Values{"test_key": "test_val"}, func() { + mgr.SetValues(gls.Values{"test_key2": "test_val2"}, func() {}) + }) + }() } + wg.Wait() } From dcb40e9836073571bcc6f5dc0e16e53f99037815 Mon Sep 17 00:00:00 2001 From: HyungrakJo Date: Fri, 12 Aug 2022 10:04:07 +0900 Subject: [PATCH 3/4] rewrite benchmark testcase --- context_test.go | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/context_test.go b/context_test.go index af000d0..4ddfcd0 100644 --- a/context_test.go +++ b/context_test.go @@ -128,16 +128,18 @@ func BenchmarkGetValue(b *testing.B) { mgr.SetValues(gls.Values{"test_key": "test_val"}, func() { b.ResetTimer() for i := 0; i < b.N; i++ { - wg.Add(1) - gls.Go(func() { - defer wg.Done() - val, ok := mgr.GetValue("test_key") - if !ok || val != "test_val" { - b.FailNow() - } - }) + for j := 0; j < 100; j++ { + wg.Add(1) + gls.Go(func() { + defer wg.Done() + val, ok := mgr.GetValue("test_key") + if !ok || val != "test_val" { + b.FailNow() + } + }) + } + wg.Wait() } - wg.Wait() }) } @@ -145,13 +147,15 @@ func BenchmarkSetValues(b *testing.B) { mgr := gls.NewContextManager() wg := sync.WaitGroup{} for i := 0; i < b.N/2; i++ { - wg.Add(1) - go func() { - defer wg.Done() - mgr.SetValues(gls.Values{"test_key": "test_val"}, func() { - mgr.SetValues(gls.Values{"test_key2": "test_val2"}, func() {}) - }) - }() + for j := 0; j < 100; j++ { + wg.Add(1) + go func() { + defer wg.Done() + mgr.SetValues(gls.Values{"test_key": "test_val"}, func() { + mgr.SetValues(gls.Values{"test_key2": "test_val2"}, func() {}) + }) + }() + } + wg.Wait() } - wg.Wait() } From 57d86e1e4f0ecb5c6e93b644863f1aee408279f1 Mon Sep 17 00:00:00 2001 From: HyungrakJo Date: Fri, 12 Aug 2022 11:31:49 +0900 Subject: [PATCH 4/4] make initial_max_go_routine_count, extend_unit configurable --- context.go | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/context.go b/context.go index f0d46de..d21aed6 100644 --- a/context.go +++ b/context.go @@ -26,22 +26,44 @@ type Values map[interface{}]interface{} // construction. type ContextManager struct { extendLock sync.RWMutex + extendUnit uint32 values []Values currentMaxGoroutineCount int } -// NewContextManager returns a brand new ContextManager. It also registers the -// new ContextManager in the ContextManager registry which is used by the Go -// method. ContextManagers are typically defined globally at package scope. -func NewContextManager() *ContextManager { - mgr := &ContextManager{values: make([]Values, initialMaxGoroutineCount)} +type Option struct { + InitialMaxGoroutineCount int + ExtendUnit int +} + +func newContextManager(opt Option) *ContextManager { + mgr := &ContextManager{values: make([]Values, opt.InitialMaxGoroutineCount)} mgr.currentMaxGoroutineCount = len(mgr.values) + mgr.extendUnit = uint32(opt.ExtendUnit) mgrRegistryMtx.Lock() defer mgrRegistryMtx.Unlock() mgrRegistry[mgr] = true return mgr } +func NewContextManagerWithOption(opt Option) *ContextManager { + if opt.InitialMaxGoroutineCount == 0 { + opt.InitialMaxGoroutineCount = initialMaxGoroutineCount + } + if opt.ExtendUnit == 0 { + opt.ExtendUnit = extendUnit + } + + return newContextManager(opt) +} + +// NewContextManager returns a brand new ContextManager. It also registers the +// new ContextManager in the ContextManager registry which is used by the Go +// method. ContextManagers are typically defined globally at package scope. +func NewContextManager() *ContextManager { + return newContextManager(Option{InitialMaxGoroutineCount: initialMaxGoroutineCount, ExtendUnit: extendUnit}) +} + // Unregister removes a ContextManager from the global registry, used by the // Go method. Only intended for use when you're completely done with a // ContextManager. Use of Unregister at all is rare.