Repository: Tochemey/goakt
Pull request #611 (2025-01-27 10:19:49)

perf: fix bad performance of sharded map
- This has no effect on messages processing
- This will enhance actors map on a given node

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/Tochemey/goakt/pull/611?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Arsene) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 87.07%. Comparing base [(`202ae9e`)](https://app.codecov.io/gh/Tochemey/goakt/commit/202ae9e524f2a41a0b115b263d2dbddcb1b0dceb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Arsene) to head [(`8336659`)](https://app.codecov.io/gh/Tochemey/goakt/commit/8336659f70298d86b57e7c3c5f2ea032794e9cef?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Arsene).

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #611      +/-   ##
==========================================
+ Coverage   87.02%   87.07%   +0.04%     
==========================================
  Files          60       60              
  Lines        5713     5716       +3     
==========================================
+ Hits         4972     4977       +5     
+ Misses        596      594       -2     
  Partials      145      145              
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/Tochemey/goakt/pull/611?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Arsene).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Arsene).


Patch:
diff --git a/actors/actor_system.go b/actors/actor_system.go
index 28f40df..0b54c40 100644
--- a/actors/actor_system.go
+++ b/actors/actor_system.go
@@ -742,7 +742,7 @@ func (x *actorSystem) ReSpawn(ctx context.Context, name string) (*PID, error) {
 		pid := pidNode.GetValue()
 
 		parent := NoSender
-		if parentNode, ok := x.actors.Parent(pid); ok {
+		if parentNode, ok := x.actors.ParentAt(pid, 0); ok {
 			parent = parentNode.GetValue()
 		}
 
@@ -1116,7 +1116,7 @@ func (x *actorSystem) RemoteReSpawn(ctx context.Context, request *connect.Reques
 
 	pid := pidNode.GetValue()
 	parent := NoSender
-	if parentNode, ok := x.actors.Parent(pid); ok {
+	if parentNode, ok := x.actors.ParentAt(pid, 0); ok {
 		parent = parentNode.GetValue()
 	}
 
diff --git a/actors/pid.go b/actors/pid.go
index a984a4a..9fb4767 100644
--- a/actors/pid.go
+++ b/actors/pid.go
@@ -302,7 +302,7 @@ func (pid *PID) Child(name string) (*PID, error) {
 // Parent returns the parent of this PID
 func (pid *PID) Parent() *PID {
 	tree := pid.ActorSystem().tree()
-	parentNode, ok := tree.Parent(pid)
+	parentNode, ok := tree.ParentAt(pid, 0)
 	if !ok {
 		return nil
 	}
@@ -415,7 +415,7 @@ func (pid *PID) Restart(ctx context.Context) error {
 	children := pid.Children()
 	// get the parent node of the actor
 	parent := NoSender
-	if pnode, ok := tree.Parent(pid); ok {
+	if pnode, ok := tree.ParentAt(pid, 0); ok {
 		parent = pnode.GetValue()
 	}
 
diff --git a/actors/pid_node.go b/actors/pid_node.go
index 0beb02b..b2201ae 100644
--- a/actors/pid_node.go
+++ b/actors/pid_node.go
@@ -36,11 +36,6 @@ type pidValue struct {
 	data *PID
 }
 
-// newPidValue creates an instance of pidValue
-func newPidValue(data *PID) *pidValue {
-	return &pidValue{data: data}
-}
-
 // Value returns the actual pidValue value
 func (v *pidValue) Value() *PID {
 	return v.data
diff --git a/actors/pid_tree.go b/actors/pid_tree.go
index fc7e7bd..05d58cc 100644
--- a/actors/pid_tree.go
+++ b/actors/pid_tree.go
@@ -34,8 +34,8 @@ import (
 
 // pidTree represents the entire actors Tree structure
 type pidTree struct {
-	nodes     *shardedMap
-	parents   *shardedMap
+	nodes     shardedMap
+	parents   shardedMap
 	nodePool  *sync.Pool
 	valuePool *sync.Pool
 	size      atomic.Int64
@@ -117,7 +117,7 @@ func (t *pidTree) AddWatcher(node, watcher *PID) {
 
 // Ancestors retrieves all ancestors nodes of a given node
 func (t *pidTree) Ancestors(pid *PID) ([]*pidNode, bool) {
-	ancestorIDs, ok := t.getAncestors(pid.ID())
+	ancestorIDs, ok := t.ancestors(pid.ID())
 	if !ok {
 		return nil, false
 	}
@@ -131,19 +131,9 @@ func (t *pidTree) Ancestors(pid *PID) ([]*pidNode, bool) {
 	return ancestors, true
 }
 
-// Parent returns a given PID direct parent
-func (t *pidTree) Parent(pid *PID) (*pidNode, bool) {
-	return t.ancestorAt(pid, 0)
-}
-
-// GrandParent retrieves the grandparent of a node
-func (t *pidTree) GrandParent(pid *PID) (*pidNode, bool) {
-	return t.ancestorAt(pid, 1)
-}
-
-// GreatGrandParent retrieves the great-grand-parent of a node
-func (t *pidTree) GreatGrandParent(pid *PID) (*pidNode, bool) {
-	return t.ancestorAt(pid, 2)
+// ParentAt returns a given PID direct parent
+func (t *pidTree) ParentAt(pid *PID, level int) (*pidNode, bool) {
+	return t.ancestorAt(pid, level)
 }
 
 // Descendants retrieves all descendants of the node with the given ID.
@@ -203,10 +193,9 @@ func (t *pidTree) GetNode(id string) (*pidNode, bool) {
 // Nodes retrieves all nodes in the tree efficiently
 func (t *pidTree) Nodes() []*pidNode {
 	var nodes []*pidNode
-	t.nodes.Range(func(_, value any) bool {
+	t.nodes.Range(func(_, value any) {
 		node := value.(*pidNode)
 		nodes = append(nodes, node)
-		return true
 	})
 	return nodes
 }
@@ -223,8 +212,8 @@ func (t *pidTree) Reset() {
 	t.size.Store(0)
 }
 
-// getAncestors returns the list of ancestor nodes
-func (t *pidTree) getAncestors(id string) ([]string, bool) {
+// ancestors returns the list of ancestor nodes
+func (t *pidTree) ancestors(id string) ([]string, bool) {
 	if value, ok := t.parents.Load(id); ok {
 		return value.([]string), true
 	}
@@ -240,7 +229,7 @@ func (t *pidTree) addChild(parent *pidNode, child *pidNode) {
 
 // updateAncestors updates the parent/ancestor relationships.
 func (t *pidTree) updateAncestors(parentID, childID string) {
-	if ancestors, ok := t.getAncestors(parentID); ok {
+	if ancestors, ok := t.ancestors(parentID); ok {
 		t.parents.Store(childID, append([]string{parentID}, ancestors...))
 	} else {
 		t.parents.Store(childID, []string{parentID})
@@ -276,7 +265,7 @@ func collectDescendants(node *pidNode) []*pidNode {
 
 // ancestorAt retrieves the ancestor at the specified level (0 for parent, 1 for grandparent, etc.)
 func (t *pidTree) ancestorAt(pid *PID, level int) (*pidNode, bool) {
-	ancestors, ok := t.getAncestors(pid.ID())
+	ancestors, ok := t.ancestors(pid.ID())
 	if ok && len(ancestors) > level {
 		return t.GetNode(ancestors[level])
 	}
diff --git a/actors/sharded_map.go b/actors/sharded_map.go
index 402bbea..5dc77e2 100644
--- a/actors/sharded_map.go
+++ b/actors/sharded_map.go
@@ -31,60 +31,81 @@ import (
 
 const numShards = 64
 
-// shardedMap is a concurrent map with sharding for scalability
-type shardedMap struct {
-	shards []*sync.Map
+// Shard defines a Shard
+type shard struct {
+	sync.RWMutex
+	m map[string]any
 }
 
-// newShardedMap creates an instance of shardedMap
-func newShardedMap() *shardedMap {
-	shards := make([]*sync.Map, numShards)
-	for i := range shards {
-		shards[i] = &sync.Map{}
-	}
-	return &shardedMap{
-		shards: shards,
-	}
-}
+// ShardedMap defines a concurrent map with sharding for
+// scalability
+type shardedMap []*shard
 
-func (s *shardedMap) getShard(key string) *sync.Map {
-	hash := fnv64(key) % numShards
-	return s.shards[hash]
+// newShardedMap creates an instance of ShardedMap
+func newShardedMap() shardedMap {
+	shards := make([]*shard, numShards)
+	for i := range numShards {
+		shards[i] = &shard{
+			m: make(map[string]any),
+		}
+	}
+	return shards
 }
 
-func (s *shardedMap) Load(key string) (any, bool) {
+// Load returns the value of a given key
+func (s shardedMap) Load(key string) (any, bool) {
 	shard := s.getShard(key)
-	return shard.Load(key)
+	shard.RLock()
+	val, ok := shard.m[key]
+	shard.RUnlock()
+	return val, ok
 }
 
-func (s *shardedMap) Store(key string, value any) {
+// Store adds a key/value pair to the sharded map
+func (s shardedMap) Store(key string, value any) {
 	shard := s.getShard(key)
-	shard.Store(key, value)
+	shard.Lock()
+	shard.m[key] = value
+	shard.Unlock()
 }
 
-func (s *shardedMap) Delete(key string) {
+// Delete removes a given key from the sharded map
+func (s shardedMap) Delete(key string) {
 	shard := s.getShard(key)
-	shard.Delete(key)
+	shard.Lock()
+	delete(shard.m, key)
+	shard.Unlock()
 }
 
-func (s *shardedMap) Range(f func(key, value any) bool) {
-	for i := 0; i < numShards; i++ {
-		shard := s.shards[i]
-		shard.Range(f)
+// Range given a function iterate over the sharded map
+func (s shardedMap) Range(f func(key, value any)) {
+	for i := 0; i < len(s); i++ {
+		shard := s[i]
+		shard.RLock()
+		for k, v := range shard.m {
+			f(k, v)
+		}
+		shard.RUnlock()
 	}
 }
 
-func (s *shardedMap) Reset() {
-	// Reset each shard's map
-	for i := 0; i < numShards; i++ {
-		shard := s.shards[i]
-		shard.Range(func(key, _ any) bool {
-			shard.Delete(key) // Clear the entry
-			return true
-		})
+// Reset resets the sharded map
+func (s shardedMap) Reset() {
+	// Reset each Shard's map
+	for i := 0; i < len(s); i++ {
+		shard := s[i]
+		shard.Lock()
+		shard.m = make(map[string]any)
+		shard.Unlock()
 	}
 }
 
+// getShard returns the given Shard for a given key
+func (s shardedMap) getShard(key string) *shard {
+	hash := fnv64(key) % uint64(len(s))
+	return s[int(hash)]
+}
+
 func fnv64(key string) uint64 {
 	hash := fnv.New64()
 	_, _ = hash.Write([]byte(key))


Test patch:
diff --git a/actors/pid_tree_test.go b/actors/pid_tree_test.go
index a7f4087..21312c5 100644
--- a/actors/pid_tree_test.go
+++ b/actors/pid_tree_test.go
@@ -60,14 +60,14 @@ func TestTree(t *testing.T) {
 	require.True(t, ok)
 	require.NotEmpty(t, pid4Ancestors)
 	require.Len(t, pid4Ancestors, 2)
-	pid4Parent, ok := tree.Parent(pid4)
+	pid4Parent, ok := tree.ParentAt(pid4, 0)
 	require.True(t, ok)
 	require.NotNil(t, pid4Parent)
 	require.True(t, pid4Parent.GetValue().Equals(pid3))
-	pid4GParent, ok := tree.GrandParent(pid4)
+	pid4GParent, ok := tree.ParentAt(pid4, 1)
 	require.True(t, ok)
 	require.True(t, pid4GParent.GetValue().Equals(pid1))
-	pid4GGParent, ok := tree.GreatGrandParent(pid4)
+	pid4GGParent, ok := tree.ParentAt(pid4, 2)
 	require.False(t, ok)
 	require.Nil(t, pid4GGParent)