puzzled about func ConcurrentMap.Key()

[bluesky1024]

func (m ConcurrentMap) Keys() []string {
	count := m.Count()

	time.Sleep(5*time.Second)

	ch := make(chan string, count)
	go func() {
		// Foreach shard.
		wg := sync.WaitGroup{}
		wg.Add(SHARD_COUNT)
		for _, shard := range m {
			go func(shard *ConcurrentMapShared) {
				// Foreach key, value pair.
				shard.RLock()
				for key := range shard.items {
					ch <- key
				}
				shard.RUnlock()
				wg.Done()
			}(shard)
		}
		wg.Wait()
		close(ch)
	}()

	// Generate keys
	keys := make([]string, 0, count)
	for k := range ch {
		keys = append(keys, k)
	}
	return keys
}

as the func shows, m.Count() is called before loop keys from shards.
However, there was no direct connect between Cont() res and the length of keys.
So why the func is designed to count first ?

[orcaman]

Count returns the number of items in the map (see this. It sums up the number of items in all shards (so going all shards and counting the number of elements on each).

[bluesky1024]

@orcaman
the realize of Count() is clear, but the result of count may have changed when the keys loop. that's the point i do not understand. Is the operation of count with much shard lock necessary ?
what's more, the result of count() is unreliable for not locking all the shard at the same time. is there any plan to optimize this function?

[orcaman]

@bluesky1024 oh, I see what you mean now. So I would say that there are two approaches to getting keys from maps that I'm familiar with. The first approach is the redis approach - the infamous "keys" command, which locks the entire DB (or map in our case) until the count is done / keys are returned. And then there's a dynamic approach which is a bit more reliable than not locking at all but not 100% bullet-proof (like what we went for here).

I think it's not a bad idea to add a function more similar to redis, something like "KeysWithLock" (to make sure the user understands the performance consequences of using this). If you feel like it, it would be cool if you could create this and send a PR for review.

Anyway - reopening this for now.

[bluesky1024]

sorry for not notice your replay...
I'm not free these days with much work. And I will try to finish "KeysWithLock" in the future.

[lvxiaorun]

I also noticed this problem when I first came into contact with Cmap. I think the original intention of Cmap design is to sharde data and reduce the granularity of lock to increase the efficiency of read and write. We normally don't need to obtain all keys so rigor, so I think KeyswithLock is probably not important.

[baizhiwen]

Why use channel to receive data and then use the for loop to assign it to the string array? Why not just accept it as a string array?Higher performance or safer?

[yzbmz5913]

Why use channel to receive data and then use the for loop to assign it to the string array? Why not just accept it as a string array?Higher performance or safer?

Appending to a slice concurrently is not safe.