From 2b3b81d7b91bfb0eff7904e5193fb537deba8d9f Mon Sep 17 00:00:00 2001 From: Florian Thienel Date: Tue, 28 May 2024 16:31:57 +0200 Subject: [PATCH] clean up the concurrency in Score --- core/bandmap/bandmap.go | 2 +- core/core.go | 30 ++++++++ core/logbook/qsolist.go | 10 +-- core/logbook/qsolist_test.go | 4 +- core/score/score.go | 135 ++++++++++++++++++++++------------- core/score/score_test.go | 14 ++-- 6 files changed, 133 insertions(+), 62 deletions(-) diff --git a/core/bandmap/bandmap.go b/core/bandmap/bandmap.go index 48e8bee..a1c86e9 100644 --- a/core/bandmap/bandmap.go +++ b/core/bandmap/bandmap.go @@ -181,8 +181,8 @@ func (m *Bandmap) ContestChanged(contest core.Contest) { } func (m *Bandmap) ScoreUpdated(score core.Score) { + totalScore := score.Result() m.do <- func() { - totalScore := score.Result() m.weights.TotalPoints = float64(totalScore.Points) m.weights.TotalMultis = float64(totalScore.Multis) m.update() diff --git a/core/core.go b/core/core.go index 13cc555..36d9cb6 100644 --- a/core/core.go +++ b/core/core.go @@ -483,6 +483,22 @@ func NewScore() Score { } } +func (s Score) Copy() Score { + result := Score{ + ScorePerBand: make(map[Band]BandScore), + GraphPerBand: make(map[Band]BandGraph), + } + + for band, bandScore := range s.ScorePerBand { + result.ScorePerBand[band] = bandScore + } + for band, bandGraph := range s.GraphPerBand { + result.GraphPerBand[band] = bandGraph.Copy() + } + + return result +} + func (s Score) String() string { buf := bytes.NewBufferString("") fmt.Fprintf(buf, "Band QSOs Dupe Pts P/Q Mult Q/M Result \n") @@ -564,6 +580,20 @@ func NewBandGraph(band Band, startTime time.Time, duration time.Duration) BandGr } } +func (g BandGraph) Copy() BandGraph { + result := BandGraph{ + Band: g.Band, + DataPoints: make([]BandScore, len(g.DataPoints)), + Max: g.Max, + startTime: g.startTime, + binSeconds: g.binSeconds, + } + + copy(result.DataPoints, g.DataPoints) + + return result +} + func (g BandGraph) String() string { points := make([]string, len(g.DataPoints)) multis := make([]string, len(g.DataPoints)) diff --git a/core/logbook/qsolist.go b/core/logbook/qsolist.go index a553005..22b0ade 100644 --- a/core/logbook/qsolist.go +++ b/core/logbook/qsolist.go @@ -62,7 +62,8 @@ func (f ExchangeFieldsChangedListenerFunc) ExchangeFieldsChanged(myExchangeField type QSOScorer interface { Clear() - Add(qso core.QSO) core.QSOScore + AddMuted(qso core.QSO) core.QSOScore + Unmute() } type QSOList struct { @@ -139,7 +140,6 @@ func (l *QSOList) Fill(qsos []core.QSO) { for _, qso := range qsos { l.put(qso) } - l.refreshScore() allQSOs := l.all() l.dataLock.Unlock() @@ -180,6 +180,7 @@ func (l *QSOList) put(qso core.QSO) func() { qsos := l.all() return func() { + l.scorer.Unmute() l.emitQSOsCleared() for _, qso := range qsos { l.emitQSOAdded(qso) @@ -213,7 +214,7 @@ func findIndex(list []core.QSO, number core.QSONumber) (int, bool) { } func (l *QSOList) append(qso core.QSO) func() { - score := l.scorer.Add(qso) + score := l.scorer.AddMuted(qso) qso.Points = score.Points qso.Multis = score.Multis qso.Duplicate = score.Duplicate @@ -225,6 +226,7 @@ func (l *QSOList) append(qso core.QSO) func() { l.list = append(l.list, qso) return func() { + l.scorer.Unmute() l.emitQSOAdded(qso) } } @@ -243,7 +245,7 @@ func (l *QSOList) refreshScore() { l.dupes = make(dupeIndex) l.worked = make(dupeIndex) for i, qso := range l.list { - score := l.scorer.Add(qso) + score := l.scorer.AddMuted(qso) qso.Points = score.Points qso.Multis = score.Multis qso.Duplicate = score.Duplicate diff --git a/core/logbook/qsolist_test.go b/core/logbook/qsolist_test.go index b7b2847..c4469fa 100644 --- a/core/logbook/qsolist_test.go +++ b/core/logbook/qsolist_test.go @@ -306,7 +306,7 @@ func (s *testScorer) Clear() { s.worked = make([]string, 1) } -func (s *testScorer) Add(qso core.QSO) core.QSOScore { +func (s *testScorer) AddMuted(qso core.QSO) core.QSOScore { if s.scores == nil { s.scores = make(map[string]core.QSOScore) } @@ -325,3 +325,5 @@ func (s *testScorer) Add(qso core.QSO) core.QSOScore { s.worked = append(s.worked, callsign) return result } + +func (s *testScorer) Unmute() {} diff --git a/core/score/score.go b/core/score/score.go index deb9974..567e4e2 100644 --- a/core/score/score.go +++ b/core/score/score.go @@ -50,9 +50,10 @@ var toConvalMode = map[core.Mode]conval.Mode{ } type Counter struct { - core.Score - counter convalCounter - counterMutex *sync.RWMutex + score core.Score + readScore core.Score + scoreLock *sync.Mutex + counter *safeConvalCounter view View prefixDatabase prefixDatabase invalid bool @@ -71,22 +72,27 @@ type Counter struct { func NewCounter(settings core.Settings, entities DXCCEntities) *Counter { result := &Counter{ - Score: core.NewScore(), - counter: new(nullCounter), - counterMutex: new(sync.RWMutex), + score: core.NewScore(), + scoreLock: &sync.Mutex{}, + counter: newSafeCounter(new(nullCounter)), view: new(nullView), prefixDatabase: prefixDatabase{entities}, } + result.copyScore() result.setStation(settings.Station()) result.setContest(settings.Contest()) - result.resetCounter() + result.resetCounter() // CONVAL WRITE LOCK return result } +func (c *Counter) copyScore() { + c.readScore = c.score.Copy() // READ +} + func (c *Counter) Result() int { - return c.Score.Result().Result() + return c.readScore.Result().Result() // READ } func (c *Counter) SetView(view View) { @@ -96,7 +102,7 @@ func (c *Counter) SetView(view View) { } c.view = view c.view.SetGoals(c.contestPointsGoal, c.contestMultisGoal) - c.view.ShowScore(c.Score) + c.view.ShowScore(c.readScore) // READ } func (c *Counter) StationChanged(station core.Station) { @@ -104,7 +110,7 @@ func (c *Counter) StationChanged(station core.Station) { c.setStation(station) c.invalid = (oldSetup.MyCountry != c.contestSetup.MyCountry) - c.resetCounter() + c.resetCounter() // CONVAL WRITE LOCK } func (c *Counter) setStation(station core.Station) { @@ -128,7 +134,7 @@ func (c *Counter) ContestChanged(contest core.Contest) { c.view.SetGoals(c.contestPointsGoal, c.contestMultisGoal) c.invalid = true - c.resetCounter() + c.resetCounter() // CONVAL WRITE LOCK } func (c *Counter) setContest(contest core.Contest) { @@ -140,18 +146,6 @@ func (c *Counter) setContest(contest core.Contest) { c.theirExchangeFields = toConvalExchangeFields(contest.TheirExchangeFields) } -func (c *Counter) resetCounter() { - c.counterMutex.Lock() - defer c.counterMutex.Unlock() - - if c.contestDefinition == nil { - c.counter = new(nullCounter) - return - } - - c.counter = conval.NewCounter(*c.contestDefinition, c.contestSetup, c.prefixDatabase) -} - func (c *Counter) Valid() bool { return !c.invalid && (c.contestSetup.MyCountry != "") && (c.contestSetup.MyContinent != "") } @@ -159,7 +153,7 @@ func (c *Counter) Valid() bool { func (c *Counter) Show() { c.view.Show() c.view.SetGoals(c.contestPointsGoal, c.contestMultisGoal) - c.view.ShowScore(c.Score) + c.view.ShowScore(c.readScore) // READ } func (c *Counter) Hide() { @@ -171,53 +165,57 @@ func (c *Counter) Notify(listener interface{}) { } func (c *Counter) Clear() { - c.Score = core.NewScore() - - c.resetCounter() + c.scoreLock.Lock() + c.score = core.NewScore() // WRITE + c.copyScore() + c.scoreLock.Unlock() + c.resetCounter() // CONVAL WRITE LOCK c.invalid = (c.contestSetup.MyCountry == "") - c.emitScoreUpdated(c.Score) -} -func (c *Counter) countQSO(qso core.QSO) conval.QSOScore { - c.counterMutex.Lock() - defer c.counterMutex.Unlock() - return c.counter.Add(c.toConvalQSO(qso)) + c.emitScoreUpdated(c.readScore) // READ } -func (c *Counter) Add(qso core.QSO) core.QSOScore { - qsoScore := c.countQSO(qso) +func (c *Counter) AddMuted(qso core.QSO) core.QSOScore { + qsoScore := c.counter.Add(c.toConvalQSO(qso)) // CONVAL WRITE LOCK result := core.QSOScore{ Points: qsoScore.Points, Multis: qsoScore.Multis, Duplicate: qsoScore.Duplicate, } - bandScore := c.ScorePerBand[qso.Band] + c.scoreLock.Lock() + + bandScore := c.score.ScorePerBand[qso.Band] // PREPARE WRITE bandScore.AddQSO(result) - c.ScorePerBand[qso.Band] = bandScore + c.score.ScorePerBand[qso.Band] = bandScore // WRITE if c.contestDefinition != nil { - graph, ok := c.GraphPerBand[qso.Band] + graph, ok := c.score.GraphPerBand[qso.Band] // PREPARE WRITE if !ok { graph = core.NewBandGraph(qso.Band, c.contestStartTime, c.contestDefinition.Duration) } graph.Add(qso.Time, result) - c.GraphPerBand[graph.Band] = graph + c.score.GraphPerBand[graph.Band] = graph // WRITE - sumGraph, ok := c.GraphPerBand[core.NoBand] + sumGraph, ok := c.score.GraphPerBand[core.NoBand] // PREPARE WRITE if !ok { sumGraph = core.NewBandGraph(core.NoBand, c.contestStartTime, c.contestDefinition.Duration) } sumGraph.Add(qso.Time, result) - c.GraphPerBand[core.NoBand] = sumGraph + c.score.GraphPerBand[core.NoBand] = sumGraph // WRITE } - c.emitScoreUpdated(c.Score) + c.copyScore() + c.scoreLock.Unlock() return result } +func (c *Counter) Unmute() { + c.emitScoreUpdated(c.readScore) +} + func (c *Counter) emitScoreUpdated(score core.Score) { c.view.ShowScore(score) for _, listener := range c.listeners { @@ -237,17 +235,11 @@ func (c *Counter) Value(callsign callsign.Callsign, entity dxcc.Prefix, band cor Mode: toConvalMode[mode], TheirExchange: c.toQSOExchange(c.theirExchangeFields, exchange), } - qsoScore := c.probeQSO(convalQSO) + qsoScore := c.counter.Probe(convalQSO) // CONVAL READ LOCK return qsoScore.Points, qsoScore.Multis, qsoScore.MultiValues } -func (c *Counter) probeQSO(qso conval.QSO) conval.QSOScore { - c.counterMutex.RLock() - defer c.counterMutex.RUnlock() - return c.counter.Probe(qso) -} - func (c *Counter) toConvalQSO(qso core.QSO) conval.QSO { result := conval.QSO{ TheirCall: qso.Callsign, @@ -269,6 +261,51 @@ func (c *Counter) toQSOExchange(fields []conval.ExchangeField, values []string) return conval.ParseExchange(fields, values, c.prefixDatabase, c.contestDefinition) } +func (c *Counter) resetCounter() { + var counter convalCounter + if c.contestDefinition == nil { + counter = new(nullCounter) + } else { + counter = conval.NewCounter(*c.contestDefinition, c.contestSetup, c.prefixDatabase) + } + c.counter.Reset(counter) // CONVAL WRITE LOCK +} + +type safeConvalCounter struct { + counter convalCounter + lock *sync.RWMutex +} + +func newSafeCounter(counter convalCounter) *safeConvalCounter { + return &safeConvalCounter{ + counter: counter, + lock: &sync.RWMutex{}, + } +} + +func (c *safeConvalCounter) Reset(counter convalCounter) { + c.lock.Lock() + defer c.lock.Unlock() + + if counter == nil { + c.counter = new(nullCounter) + } else { + c.counter = counter + } +} + +func (c *safeConvalCounter) Add(qso conval.QSO) conval.QSOScore { + c.lock.Lock() + defer c.lock.Unlock() + return c.counter.Add(qso) +} + +func (c *safeConvalCounter) Probe(qso conval.QSO) conval.QSOScore { + c.lock.RLock() + defer c.lock.RUnlock() + return c.counter.Probe(qso) +} + func toConvalExchangeFields(fields []core.ExchangeField) []conval.ExchangeField { result := make([]conval.ExchangeField, len(fields)) for i, field := range fields { diff --git a/core/score/score_test.go b/core/score/score_test.go index 9c763ef..3f20ee1 100644 --- a/core/score/score_test.go +++ b/core/score/score_test.go @@ -22,8 +22,8 @@ func TestNewCounter(t *testing.T) { assert.True(t, counter.Valid(), "valid") assert.Equal(t, strings.ToLower(myTestEntity.entity.PrimaryPrefix), string(counter.contestSetup.MyCountry), "station entity") - counter.ScorePerBand[core.Band80m] = core.BandScore{QSOs: 5} - assert.Equal(t, 5, counter.ScorePerBand[core.Band80m].QSOs) + counter.score.ScorePerBand[core.Band80m] = core.BandScore{QSOs: 5} + assert.Equal(t, 5, counter.score.ScorePerBand[core.Band80m].QSOs) } func TestConvalCounter(t *testing.T) { @@ -55,13 +55,13 @@ func TestConvalCounter(t *testing.T) { func TestAdd(t *testing.T) { counter := NewCounter(&testSettings{stationCallsign: "DL1AAA"}, &myTestEntity) - counter.Add(core.QSO{Callsign: callsign.MustParse("DL0ABC"), Band: core.Band80m, DXCC: dxcc.Prefix{Prefix: "DL", PrimaryPrefix: "DL", Continent: "EU", CQZone: 14, ITUZone: 28}}) - counter.Add(core.QSO{Callsign: callsign.MustParse("DF0ABC"), Band: core.Band80m, DXCC: dxcc.Prefix{Prefix: "DF", PrimaryPrefix: "DL", Continent: "EU", CQZone: 14, ITUZone: 28}}) + counter.AddMuted(core.QSO{Callsign: callsign.MustParse("DL0ABC"), Band: core.Band80m, DXCC: dxcc.Prefix{Prefix: "DL", PrimaryPrefix: "DL", Continent: "EU", CQZone: 14, ITUZone: 28}}) + counter.AddMuted(core.QSO{Callsign: callsign.MustParse("DF0ABC"), Band: core.Band80m, DXCC: dxcc.Prefix{Prefix: "DF", PrimaryPrefix: "DL", Continent: "EU", CQZone: 14, ITUZone: 28}}) - assert.Equal(t, 2, counter.Score.Result().QSOs, "total QSOs") + assert.Equal(t, 2, counter.score.Result().QSOs, "total QSOs") - assert.Equal(t, 1, len(counter.ScorePerBand)) - bandScore := counter.ScorePerBand[core.Band80m] + assert.Equal(t, 1, len(counter.score.ScorePerBand)) + bandScore := counter.score.ScorePerBand[core.Band80m] assert.Equal(t, 2, bandScore.QSOs, "band QSOs") }