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 8e94207
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("the payload is an unsupported JSON 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 8e94207

@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: 8e94207 Previous: 0b261f0 Ratio
BenchmarkDocument/constructor_test - ns/op 2138 ns/op 1787 ns/op 1.20
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 1346 ns/op 1594 ns/op 0.84
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 15156 ns/op 9519 ns/op 1.59
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 26255 ns/op 21918 ns/op 1.20
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 35985 ns/op 29668 ns/op 1.21
BenchmarkDocument/delete_test - B/op 15219 B/op 15218 B/op 1.00
BenchmarkDocument/delete_test - allocs/op 332 allocs/op 332 allocs/op 1
BenchmarkDocument/object_test - ns/op 12784 ns/op 10685 ns/op 1.20
BenchmarkDocument/object_test - B/op 6752 B/op 6753 B/op 1.00
BenchmarkDocument/object_test - allocs/op 115 allocs/op 115 allocs/op 1
BenchmarkDocument/array_test - ns/op 42756 ns/op 36022 ns/op 1.19
BenchmarkDocument/array_test - B/op 11849 B/op 11850 B/op 1.00
BenchmarkDocument/array_test - allocs/op 269 allocs/op 269 allocs/op 1
BenchmarkDocument/text_test - ns/op 50602 ns/op 39389 ns/op 1.28
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 50560 ns/op 39841 ns/op 1.27
BenchmarkDocument/text_composition_test - B/op 18307 B/op 18306 B/op 1.00
BenchmarkDocument/text_composition_test - allocs/op 476 allocs/op 476 allocs/op 1
BenchmarkDocument/rich_text_test - ns/op 143495 ns/op 108443 ns/op 1.32
BenchmarkDocument/rich_text_test - B/op 38705 B/op 38696 B/op 1.00
BenchmarkDocument/rich_text_test - allocs/op 1154 allocs/op 1154 allocs/op 1
BenchmarkDocument/counter_test - ns/op 29090 ns/op 22165 ns/op 1.31
BenchmarkDocument/counter_test - B/op 10273 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 5090122 ns/op 4065603 ns/op 1.25
BenchmarkDocument/text_edit_gc_100 - B/op 1553570 B/op 1553817 B/op 1.00
BenchmarkDocument/text_edit_gc_100 - allocs/op 17167 allocs/op 17170 allocs/op 1.00
BenchmarkDocument/text_edit_gc_1000 - ns/op 428736414 ns/op 317983203 ns/op 1.35
BenchmarkDocument/text_edit_gc_1000 - B/op 136626818 B/op 136650136 B/op 1.00
BenchmarkDocument/text_edit_gc_1000 - allocs/op 210661 allocs/op 210792 allocs/op 1.00
BenchmarkDocument/text_split_gc_100 - ns/op 5636524 ns/op 4600044 ns/op 1.23
BenchmarkDocument/text_split_gc_100 - B/op 2217922 B/op 2217705 B/op 1.00
BenchmarkDocument/text_split_gc_100 - allocs/op 16595 allocs/op 16595 allocs/op 1
BenchmarkDocument/text_split_gc_1000 - ns/op 488811734 ns/op 361316168 ns/op 1.35
BenchmarkDocument/text_split_gc_1000 - B/op 214859970 B/op 214857474 B/op 1.00
BenchmarkDocument/text_split_gc_1000 - allocs/op 211435 allocs/op 211435 allocs/op 1
BenchmarkDocument/text_delete_all_10000 - ns/op 24256345 ns/op 19158247 ns/op 1.27
BenchmarkDocument/text_delete_all_10000 - B/op 5904930 B/op 5904398 B/op 1.00
BenchmarkDocument/text_delete_all_10000 - allocs/op 41128 allocs/op 41126 allocs/op 1.00
BenchmarkDocument/text_delete_all_100000 - ns/op 300425522 ns/op 252544482 ns/op 1.19
BenchmarkDocument/text_delete_all_100000 - B/op 53837352 B/op 53840580 B/op 1.00
BenchmarkDocument/text_delete_all_100000 - allocs/op 415952 allocs/op 415987 allocs/op 1.00
BenchmarkDocument/text_100 - ns/op 380335 ns/op 319861 ns/op 1.19
BenchmarkDocument/text_100 - B/op 118516 B/op 118564 B/op 1.00
BenchmarkDocument/text_100 - allocs/op 5079 allocs/op 5079 allocs/op 1
BenchmarkDocument/text_1000 - ns/op 3941110 ns/op 3440801 ns/op 1.15
BenchmarkDocument/text_1000 - B/op 1153127 B/op 1153121 B/op 1.00
BenchmarkDocument/text_1000 - allocs/op 50083 allocs/op 50083 allocs/op 1
BenchmarkDocument/array_1000 - ns/op 1947748 ns/op 1726323 ns/op 1.13
BenchmarkDocument/array_1000 - B/op 1103165 B/op 1103035 B/op 1.00
BenchmarkDocument/array_1000 - allocs/op 11872 allocs/op 11872 allocs/op 1
BenchmarkDocument/array_10000 - ns/op 23290352 ns/op 20333378 ns/op 1.15
BenchmarkDocument/array_10000 - B/op 9904965 B/op 9906738 B/op 1.00
BenchmarkDocument/array_10000 - allocs/op 120714 allocs/op 120722 allocs/op 1.00
BenchmarkDocument/array_gc_100 - ns/op 205223 ns/op 180638 ns/op 1.14
BenchmarkDocument/array_gc_100 - B/op 98485 B/op 98435 B/op 1.00
BenchmarkDocument/array_gc_100 - allocs/op 1249 allocs/op 1248 allocs/op 1.00
BenchmarkDocument/array_gc_1000 - ns/op 2244860 ns/op 1968319 ns/op 1.14
BenchmarkDocument/array_gc_1000 - B/op 1170880 B/op 1170782 B/op 1.00
BenchmarkDocument/array_gc_1000 - allocs/op 12912 allocs/op 12911 allocs/op 1.00
BenchmarkDocument/counter_1000 - ns/op 320602 ns/op 294776 ns/op 1.09
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 3865831 ns/op 3161573 ns/op 1.22
BenchmarkDocument/counter_10000 - B/op 2165768 B/op 2165762 B/op 1.00
BenchmarkDocument/counter_10000 - allocs/op 69515 allocs/op 69515 allocs/op 1
BenchmarkDocument/object_1000 - ns/op 2492067 ns/op 1956226 ns/op 1.27
BenchmarkDocument/object_1000 - B/op 1451517 B/op 1451634 B/op 1.00
BenchmarkDocument/object_1000 - allocs/op 9920 allocs/op 9920 allocs/op 1
BenchmarkDocument/object_10000 - ns/op 28179975 ns/op 23547087 ns/op 1.20
BenchmarkDocument/object_10000 - B/op 12367882 B/op 12367395 B/op 1.00
BenchmarkDocument/object_10000 - allocs/op 101220 allocs/op 101220 allocs/op 1
BenchmarkDocument/tree_100 - ns/op 1181032 ns/op 979888 ns/op 1.21
BenchmarkDocument/tree_100 - B/op 442930 B/op 442919 B/op 1.00
BenchmarkDocument/tree_100 - allocs/op 4505 allocs/op 4505 allocs/op 1
BenchmarkDocument/tree_1000 - ns/op 92590276 ns/op 66488436 ns/op 1.39
BenchmarkDocument/tree_1000 - B/op 35223005 B/op 35222255 B/op 1.00
BenchmarkDocument/tree_1000 - allocs/op 44117 allocs/op 44117 allocs/op 1
BenchmarkDocument/tree_10000 - ns/op 11460884862 ns/op 9065261525 ns/op 1.26
BenchmarkDocument/tree_10000 - B/op 3438890624 B/op 3438889488 B/op 1.00
BenchmarkDocument/tree_10000 - allocs/op 440157 allocs/op 440148 allocs/op 1.00
BenchmarkDocument/tree_delete_all_1000 - ns/op 91852413 ns/op 67881292 ns/op 1.35
BenchmarkDocument/tree_delete_all_1000 - B/op 35694008 B/op 35692397 B/op 1.00
BenchmarkDocument/tree_delete_all_1000 - allocs/op 51771 allocs/op 51770 allocs/op 1.00
BenchmarkDocument/tree_edit_gc_100 - ns/op 4316304 ns/op 3535492 ns/op 1.22
BenchmarkDocument/tree_edit_gc_100 - B/op 2077553 B/op 2077462 B/op 1.00
BenchmarkDocument/tree_edit_gc_100 - allocs/op 11165 allocs/op 11164 allocs/op 1.00
BenchmarkDocument/tree_edit_gc_1000 - ns/op 382111362 ns/op 261274073 ns/op 1.46
BenchmarkDocument/tree_edit_gc_1000 - B/op 180293578 B/op 180290240 B/op 1.00
BenchmarkDocument/tree_edit_gc_1000 - allocs/op 113383 allocs/op 113374 allocs/op 1.00
BenchmarkDocument/tree_split_gc_100 - ns/op 2865910 ns/op 2598057 ns/op 1.10
BenchmarkDocument/tree_split_gc_100 - B/op 1347898 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 219967028 ns/op 162687910 ns/op 1.35
BenchmarkDocument/tree_split_gc_1000 - B/op 113955568 B/op 113957029 B/op 1.00
BenchmarkDocument/tree_split_gc_1000 - allocs/op 93875 allocs/op 93873 allocs/op 1.00
BenchmarkRPC/client_to_server - ns/op 526077780 ns/op 415539104 ns/op 1.27
BenchmarkRPC/client_to_server - B/op 12500248 B/op 12311816 B/op 1.02
BenchmarkRPC/client_to_server - allocs/op 177441 allocs/op 177426 allocs/op 1.00
BenchmarkRPC/client_to_client_via_server - ns/op 942103972 ns/op 696998912 ns/op 1.35
BenchmarkRPC/client_to_client_via_server - B/op 22695296 B/op 22726112 B/op 1.00
BenchmarkRPC/client_to_client_via_server - allocs/op 331282 allocs/op 331476 allocs/op 1.00
BenchmarkRPC/attach_large_document - ns/op 1451529607 ns/op 1356891005 ns/op 1.07
BenchmarkRPC/attach_large_document - B/op 1799309968 B/op 1799596096 B/op 1.00
BenchmarkRPC/attach_large_document - allocs/op 9970 allocs/op 9687 allocs/op 1.03
BenchmarkRPC/adminCli_to_server - ns/op 792366198 ns/op 582185414 ns/op 1.36
BenchmarkRPC/adminCli_to_server - B/op 20186356 B/op 20183072 B/op 1.00
BenchmarkRPC/adminCli_to_server - allocs/op 324107 allocs/op 324118 allocs/op 1.00
BenchmarkLocker - ns/op 145.6 ns/op 133.6 ns/op 1.09
BenchmarkLocker - B/op 16 B/op 16 B/op 1
BenchmarkLocker - allocs/op 1 allocs/op 1 allocs/op 1
BenchmarkLockerParallel - ns/op 149.2 ns/op 162.3 ns/op 0.92
BenchmarkLockerParallel - B/op 0 B/op 0 B/op NaN
BenchmarkLockerParallel - allocs/op 0 allocs/op 0 allocs/op NaN
BenchmarkLockerMoreKeys - ns/op 352.4 ns/op 309.6 ns/op 1.14
BenchmarkLockerMoreKeys - B/op 15 B/op 14 B/op 1.07
BenchmarkLockerMoreKeys - allocs/op 0 allocs/op 0 allocs/op NaN
BenchmarkSync/memory_sync_10_test - ns/op 9113 ns/op 7860 ns/op 1.16
BenchmarkSync/memory_sync_10_test - B/op 1283 B/op 1280 B/op 1.00
BenchmarkSync/memory_sync_10_test - allocs/op 38 allocs/op 38 allocs/op 1
BenchmarkSync/memory_sync_100_test - ns/op 78746 ns/op 71304 ns/op 1.10
BenchmarkSync/memory_sync_100_test - B/op 8914 B/op 8646 B/op 1.03
BenchmarkSync/memory_sync_100_test - allocs/op 290 allocs/op 273 allocs/op 1.06
BenchmarkSync/memory_sync_1000_test - ns/op 759193 ns/op 674014 ns/op 1.13
BenchmarkSync/memory_sync_1000_test - B/op 82518 B/op 81527 B/op 1.01
BenchmarkSync/memory_sync_1000_test - allocs/op 2626 allocs/op 2568 allocs/op 1.02
BenchmarkSync/memory_sync_10000_test - ns/op 7577795 ns/op 7268292 ns/op 1.04
BenchmarkSync/memory_sync_10000_test - B/op 865502 B/op 870438 B/op 0.99
BenchmarkSync/memory_sync_10000_test - allocs/op 28225 allocs/op 26884 allocs/op 1.05
BenchmarkTextEditing - ns/op 32605007962 ns/op 27438268705 ns/op 1.19
BenchmarkTextEditing - B/op 8456934440 B/op 8457113568 B/op 1.00
BenchmarkTextEditing - allocs/op 20615758 allocs/op 20616850 allocs/op 1.00

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

Please sign in to comment.