Skip to content

Commit

Permalink
Remove lock from ActorID
Browse files Browse the repository at this point in the history
  • Loading branch information
hackerwins committed Oct 2, 2023
1 parent 5e50c94 commit 0b261f0
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 36 deletions.
30 changes: 6 additions & 24 deletions pkg/document/time/actor_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"errors"
"fmt"
"math"
"sync"
)

const actorIDSize = 12
Expand Down Expand Up @@ -57,13 +56,13 @@ var (
ErrInvalidActorID = errors.New("invalid actor id")
)

// ActorID is bytes represented by the hexadecimal string.
// It should be generated by unique value.
// ActorID represents the unique ID of the client. It is composed of 12 bytes.
// It caches the string representation of ActorID to reduce the number of calls
// to hex.EncodeToString. This causes a multi-routine problem, so it is
// recommended to use it in a single routine or to use it after locking.
type ActorID struct {
bytes [actorIDSize]byte

cachedString string
cachedStringMu sync.RWMutex
bytes [actorIDSize]byte
cachedString string
}

// ActorIDFromHex returns the bytes represented by the hexadecimal string str.
Expand Down Expand Up @@ -106,23 +105,6 @@ func ActorIDFromBytes(bytes []byte) (*ActorID, error) {
// String returns the hexadecimal encoding of ActorID.
// If the receiver is nil, it would return empty string.
func (id *ActorID) String() string {
id.cachedStringMu.RLock()
readLocked := true
defer func() {
if readLocked {
id.cachedStringMu.RUnlock()
}
}()

if id.cachedString != "" {
return id.cachedString
}

id.cachedStringMu.RUnlock()
readLocked = false
id.cachedStringMu.Lock()
defer id.cachedStringMu.Unlock()

if id.cachedString == "" {
id.cachedString = hex.EncodeToString(id.bytes[:])
}
Expand Down
3 changes: 0 additions & 3 deletions server/backend/sync/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ type Coordinator interface {
// Publish publishes the given event.
Publish(ctx context.Context, publisherID *time.ActorID, event DocEvent)

// PublishToLocal publishes the given event.
PublishToLocal(ctx context.Context, publisherID *time.ActorID, event DocEvent)

// Members returns the members of this cluster.
Members() map[string]*ServerInfo

Expand Down
12 changes: 3 additions & 9 deletions server/backend/sync/memory/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,9 @@ func (c *Coordinator) Publish(
publisherID *time.ActorID,
event sync.DocEvent,
) {
c.pubSub.Publish(ctx, publisherID, event)
}

// PublishToLocal publishes the given event.
func (c *Coordinator) PublishToLocal(
ctx context.Context,
publisherID *time.ActorID,
event sync.DocEvent,
) {
// NOTE(hackerwins): String() triggers the cache of ActorID to avoid
// race condition of concurrent access to the cache.
_ = event.Publisher.String()
c.pubSub.Publish(ctx, publisherID, event)
}

Expand Down

0 comments on commit 0b261f0

Please sign in to comment.