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 bde74a4
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 43 deletions.
14 changes: 8 additions & 6 deletions pkg/document/document.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ import (

var (
// ErrUnsupportedPayloadType is returned when the payload is unserializable to JSON.
ErrUnsupportedPayloadType = errors.New(
"the payload is an unsupported JSON type")
ErrUnsupportedPayloadType = errors.New("unsupported payload type")
)

// DocEvent represents the event that occurred in the document.
Expand Down Expand Up @@ -390,7 +389,7 @@ func (d *Document) Broadcast(topic string, payload any) error {
}

// SubscribeBroadcastEvent subscribes to the given topic and registers
// an event handler
// an event handler.
func (d *Document) SubscribeBroadcastEvent(
topic string,
handler func(topic, publisher string, payload []byte) error,
Expand All @@ -399,16 +398,19 @@ func (d *Document) SubscribeBroadcastEvent(
}

// UnsubscribeBroadcastEvent unsubscribes to the given topic and deregisters
// the event handler
// the event handler.
func (d *Document) UnsubscribeBroadcastEvent(
topic string,
) {
delete(d.broadcastEventHandlers, topic)
}

// BroadcastEventHandlers returns the registered handlers for broadcast events.
func (d *Document) BroadcastEventHandlers() map[string](func(topic string,
publisher string, payload []byte) error) {
func (d *Document) BroadcastEventHandlers() map[string]func(
topic string,
publisher string,
payload []byte,
) error {
return d.broadcastEventHandlers
}

Expand Down
27 changes: 5 additions & 22 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,14 @@ 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
cachedString string
}

// ActorIDFromHex returns the bytes represented by the hexadecimal string str.
Expand Down Expand Up @@ -106,23 +106,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
3 changes: 0 additions & 3 deletions server/rpc/yorkie_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -584,9 +584,6 @@ func (s *yorkieServer) Broadcast(
return nil, err
}

if err != nil {
return nil, err
}
s.backend.Coordinator.Publish(
ctx,
clientID,
Expand Down

1 comment on commit bde74a4

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go Benchmark

Benchmark suite Current: bde74a4 Previous: 0b261f0 Ratio
BenchmarkDocument/constructor_test - ns/op 1757 ns/op 1787 ns/op 0.98
BenchmarkDocument/constructor_test - B/op 1240 B/op 1240 B/op 1
BenchmarkDocument/constructor_test - allocs/op 20 allocs/op 20 allocs/op 1
BenchmarkDocument/status_test - ns/op 1669 ns/op 1594 ns/op 1.05
BenchmarkDocument/status_test - B/op 1208 B/op 1208 B/op 1
BenchmarkDocument/status_test - allocs/op 18 allocs/op 18 allocs/op 1
BenchmarkDocument/equals_test - ns/op 9429 ns/op 9519 ns/op 0.99
BenchmarkDocument/equals_test - B/op 7009 B/op 7009 B/op 1
BenchmarkDocument/equals_test - allocs/op 119 allocs/op 119 allocs/op 1
BenchmarkDocument/nested_update_test - ns/op 22179 ns/op 21918 ns/op 1.01
BenchmarkDocument/nested_update_test - B/op 11993 B/op 11993 B/op 1
BenchmarkDocument/nested_update_test - allocs/op 253 allocs/op 253 allocs/op 1
BenchmarkDocument/delete_test - ns/op 29917 ns/op 29668 ns/op 1.01
BenchmarkDocument/delete_test - B/op 15218 B/op 15218 B/op 1
BenchmarkDocument/delete_test - allocs/op 332 allocs/op 332 allocs/op 1
BenchmarkDocument/object_test - ns/op 11859 ns/op 10685 ns/op 1.11
BenchmarkDocument/object_test - B/op 6753 B/op 6753 B/op 1
BenchmarkDocument/object_test - allocs/op 115 allocs/op 115 allocs/op 1
BenchmarkDocument/array_test - ns/op 37001 ns/op 36022 ns/op 1.03
BenchmarkDocument/array_test - B/op 11850 B/op 11850 B/op 1
BenchmarkDocument/array_test - allocs/op 269 allocs/op 269 allocs/op 1
BenchmarkDocument/text_test - ns/op 40477 ns/op 39389 ns/op 1.03
BenchmarkDocument/text_test - B/op 14922 B/op 14922 B/op 1
BenchmarkDocument/text_test - allocs/op 475 allocs/op 475 allocs/op 1
BenchmarkDocument/text_composition_test - ns/op 40428 ns/op 39841 ns/op 1.01
BenchmarkDocument/text_composition_test - B/op 18306 B/op 18306 B/op 1
BenchmarkDocument/text_composition_test - allocs/op 476 allocs/op 476 allocs/op 1
BenchmarkDocument/rich_text_test - ns/op 108526 ns/op 108443 ns/op 1.00
BenchmarkDocument/rich_text_test - B/op 38696 B/op 38696 B/op 1
BenchmarkDocument/rich_text_test - allocs/op 1154 allocs/op 1154 allocs/op 1
BenchmarkDocument/counter_test - ns/op 22143 ns/op 22165 ns/op 1.00
BenchmarkDocument/counter_test - B/op 10275 B/op 10274 B/op 1.00
BenchmarkDocument/counter_test - allocs/op 240 allocs/op 240 allocs/op 1
BenchmarkDocument/text_edit_gc_100 - ns/op 4127766 ns/op 4065603 ns/op 1.02
BenchmarkDocument/text_edit_gc_100 - B/op 1553357 B/op 1553817 B/op 1.00
BenchmarkDocument/text_edit_gc_100 - allocs/op 17166 allocs/op 17170 allocs/op 1.00
BenchmarkDocument/text_edit_gc_1000 - ns/op 323704037 ns/op 317983203 ns/op 1.02
BenchmarkDocument/text_edit_gc_1000 - B/op 136634364 B/op 136650136 B/op 1.00
BenchmarkDocument/text_edit_gc_1000 - allocs/op 210713 allocs/op 210792 allocs/op 1.00
BenchmarkDocument/text_split_gc_100 - ns/op 4589626 ns/op 4600044 ns/op 1.00
BenchmarkDocument/text_split_gc_100 - B/op 2217944 B/op 2217705 B/op 1.00
BenchmarkDocument/text_split_gc_100 - allocs/op 16593 allocs/op 16595 allocs/op 1.00
BenchmarkDocument/text_split_gc_1000 - ns/op 377663539 ns/op 361316168 ns/op 1.05
BenchmarkDocument/text_split_gc_1000 - B/op 214862866 B/op 214857474 B/op 1.00
BenchmarkDocument/text_split_gc_1000 - allocs/op 211477 allocs/op 211435 allocs/op 1.00
BenchmarkDocument/text_delete_all_10000 - ns/op 19327536 ns/op 19158247 ns/op 1.01
BenchmarkDocument/text_delete_all_10000 - B/op 5904317 B/op 5904398 B/op 1.00
BenchmarkDocument/text_delete_all_10000 - allocs/op 41125 allocs/op 41126 allocs/op 1.00
BenchmarkDocument/text_delete_all_100000 - ns/op 262672398 ns/op 252544482 ns/op 1.04
BenchmarkDocument/text_delete_all_100000 - B/op 53845232 B/op 53840580 B/op 1.00
BenchmarkDocument/text_delete_all_100000 - allocs/op 416011 allocs/op 415987 allocs/op 1.00
BenchmarkDocument/text_100 - ns/op 333782 ns/op 319861 ns/op 1.04
BenchmarkDocument/text_100 - B/op 118515 B/op 118564 B/op 1.00
BenchmarkDocument/text_100 - allocs/op 5079 allocs/op 5079 allocs/op 1
BenchmarkDocument/text_1000 - ns/op 3638937 ns/op 3440801 ns/op 1.06
BenchmarkDocument/text_1000 - B/op 1153122 B/op 1153121 B/op 1.00
BenchmarkDocument/text_1000 - allocs/op 50083 allocs/op 50083 allocs/op 1
BenchmarkDocument/array_1000 - ns/op 1818022 ns/op 1726323 ns/op 1.05
BenchmarkDocument/array_1000 - B/op 1103159 B/op 1103035 B/op 1.00
BenchmarkDocument/array_1000 - allocs/op 11873 allocs/op 11872 allocs/op 1.00
BenchmarkDocument/array_10000 - ns/op 20936302 ns/op 20333378 ns/op 1.03
BenchmarkDocument/array_10000 - B/op 9909220 B/op 9906738 B/op 1.00
BenchmarkDocument/array_10000 - allocs/op 120732 allocs/op 120722 allocs/op 1.00
BenchmarkDocument/array_gc_100 - ns/op 187011 ns/op 180638 ns/op 1.04
BenchmarkDocument/array_gc_100 - B/op 98476 B/op 98435 B/op 1.00
BenchmarkDocument/array_gc_100 - allocs/op 1248 allocs/op 1248 allocs/op 1
BenchmarkDocument/array_gc_1000 - ns/op 2011540 ns/op 1968319 ns/op 1.02
BenchmarkDocument/array_gc_1000 - B/op 1170683 B/op 1170782 B/op 1.00
BenchmarkDocument/array_gc_1000 - allocs/op 12911 allocs/op 12911 allocs/op 1
BenchmarkDocument/counter_1000 - ns/op 298127 ns/op 294776 ns/op 1.01
BenchmarkDocument/counter_1000 - B/op 198837 B/op 198836 B/op 1.00
BenchmarkDocument/counter_1000 - allocs/op 6508 allocs/op 6508 allocs/op 1
BenchmarkDocument/counter_10000 - ns/op 3231303 ns/op 3161573 ns/op 1.02
BenchmarkDocument/counter_10000 - B/op 2165761 B/op 2165762 B/op 1.00
BenchmarkDocument/counter_10000 - allocs/op 69515 allocs/op 69515 allocs/op 1
BenchmarkDocument/object_1000 - ns/op 1962833 ns/op 1956226 ns/op 1.00
BenchmarkDocument/object_1000 - B/op 1451348 B/op 1451634 B/op 1.00
BenchmarkDocument/object_1000 - allocs/op 9919 allocs/op 9920 allocs/op 1.00
BenchmarkDocument/object_10000 - ns/op 23935693 ns/op 23547087 ns/op 1.02
BenchmarkDocument/object_10000 - B/op 12370261 B/op 12367395 B/op 1.00
BenchmarkDocument/object_10000 - allocs/op 101225 allocs/op 101220 allocs/op 1.00
BenchmarkDocument/tree_100 - ns/op 983508 ns/op 979888 ns/op 1.00
BenchmarkDocument/tree_100 - B/op 442926 B/op 442919 B/op 1.00
BenchmarkDocument/tree_100 - allocs/op 4505 allocs/op 4505 allocs/op 1
BenchmarkDocument/tree_1000 - ns/op 68096423 ns/op 66488436 ns/op 1.02
BenchmarkDocument/tree_1000 - B/op 35223273 B/op 35222255 B/op 1.00
BenchmarkDocument/tree_1000 - allocs/op 44117 allocs/op 44117 allocs/op 1
BenchmarkDocument/tree_10000 - ns/op 9064844965 ns/op 9065261525 ns/op 1.00
BenchmarkDocument/tree_10000 - B/op 3438881712 B/op 3438889488 B/op 1.00
BenchmarkDocument/tree_10000 - allocs/op 440151 allocs/op 440148 allocs/op 1.00
BenchmarkDocument/tree_delete_all_1000 - ns/op 70053676 ns/op 67881292 ns/op 1.03
BenchmarkDocument/tree_delete_all_1000 - B/op 35692743 B/op 35692397 B/op 1.00
BenchmarkDocument/tree_delete_all_1000 - allocs/op 51770 allocs/op 51770 allocs/op 1
BenchmarkDocument/tree_edit_gc_100 - ns/op 3546042 ns/op 3535492 ns/op 1.00
BenchmarkDocument/tree_edit_gc_100 - B/op 2077467 B/op 2077462 B/op 1.00
BenchmarkDocument/tree_edit_gc_100 - allocs/op 11164 allocs/op 11164 allocs/op 1
BenchmarkDocument/tree_edit_gc_1000 - ns/op 267949104 ns/op 261274073 ns/op 1.03
BenchmarkDocument/tree_edit_gc_1000 - B/op 180293242 B/op 180290240 B/op 1.00
BenchmarkDocument/tree_edit_gc_1000 - allocs/op 113378 allocs/op 113374 allocs/op 1.00
BenchmarkDocument/tree_split_gc_100 - ns/op 2625611 ns/op 2598057 ns/op 1.01
BenchmarkDocument/tree_split_gc_100 - B/op 1347919 B/op 1347874 B/op 1.00
BenchmarkDocument/tree_split_gc_100 - allocs/op 9060 allocs/op 9060 allocs/op 1
BenchmarkDocument/tree_split_gc_1000 - ns/op 168941376 ns/op 162687910 ns/op 1.04
BenchmarkDocument/tree_split_gc_1000 - B/op 113957137 B/op 113957029 B/op 1.00
BenchmarkDocument/tree_split_gc_1000 - allocs/op 93872 allocs/op 93873 allocs/op 1.00
BenchmarkRPC/client_to_server - ns/op 404954870 ns/op 415539104 ns/op 0.97
BenchmarkRPC/client_to_server - B/op 12297773 B/op 12311816 B/op 1.00
BenchmarkRPC/client_to_server - allocs/op 177337 allocs/op 177426 allocs/op 1.00
BenchmarkRPC/client_to_client_via_server - ns/op 690568745 ns/op 696998912 ns/op 0.99
BenchmarkRPC/client_to_client_via_server - B/op 22746720 B/op 22726112 B/op 1.00
BenchmarkRPC/client_to_client_via_server - allocs/op 331787 allocs/op 331476 allocs/op 1.00
BenchmarkRPC/attach_large_document - ns/op 1371580067 ns/op 1356891005 ns/op 1.01
BenchmarkRPC/attach_large_document - B/op 1799398024 B/op 1799596096 B/op 1.00
BenchmarkRPC/attach_large_document - allocs/op 9603 allocs/op 9687 allocs/op 0.99
BenchmarkRPC/adminCli_to_server - ns/op 598002752 ns/op 582185414 ns/op 1.03
BenchmarkRPC/adminCli_to_server - B/op 20191076 B/op 20183072 B/op 1.00
BenchmarkRPC/adminCli_to_server - allocs/op 324141 allocs/op 324118 allocs/op 1.00
BenchmarkLocker - ns/op 129.8 ns/op 133.6 ns/op 0.97
BenchmarkLocker - B/op 16 B/op 16 B/op 1
BenchmarkLocker - allocs/op 1 allocs/op 1 allocs/op 1
BenchmarkLockerParallel - ns/op 158.7 ns/op 162.3 ns/op 0.98
BenchmarkLockerParallel - B/op 0 B/op 0 B/op NaN
BenchmarkLockerParallel - allocs/op 0 allocs/op 0 allocs/op NaN
BenchmarkLockerMoreKeys - ns/op 419.2 ns/op 309.6 ns/op 1.35
BenchmarkLockerMoreKeys - B/op 13 B/op 14 B/op 0.93
BenchmarkLockerMoreKeys - allocs/op 0 allocs/op 0 allocs/op NaN
BenchmarkSync/memory_sync_10_test - ns/op 7976 ns/op 7860 ns/op 1.01
BenchmarkSync/memory_sync_10_test - B/op 1280 B/op 1280 B/op 1
BenchmarkSync/memory_sync_10_test - allocs/op 38 allocs/op 38 allocs/op 1
BenchmarkSync/memory_sync_100_test - ns/op 71906 ns/op 71304 ns/op 1.01
BenchmarkSync/memory_sync_100_test - B/op 8631 B/op 8646 B/op 1.00
BenchmarkSync/memory_sync_100_test - allocs/op 272 allocs/op 273 allocs/op 1.00
BenchmarkSync/memory_sync_1000_test - ns/op 684589 ns/op 674014 ns/op 1.02
BenchmarkSync/memory_sync_1000_test - B/op 81607 B/op 81527 B/op 1.00
BenchmarkSync/memory_sync_1000_test - allocs/op 2571 allocs/op 2568 allocs/op 1.00
BenchmarkSync/memory_sync_10000_test - ns/op 7164707 ns/op 7268292 ns/op 0.99
BenchmarkSync/memory_sync_10000_test - B/op 846286 B/op 870438 B/op 0.97
BenchmarkSync/memory_sync_10000_test - allocs/op 26614 allocs/op 26884 allocs/op 0.99
BenchmarkTextEditing - ns/op 27965399354 ns/op 27438268705 ns/op 1.02
BenchmarkTextEditing - B/op 8456798176 B/op 8457113568 B/op 1.00
BenchmarkTextEditing - allocs/op 20615256 allocs/op 20616850 allocs/op 1.00

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.