puzpuzpuz / xsync

Concurrent data structures for Go
Apache License 2.0
1.13k stars 40 forks source link

reduce mapof delete operation cost #130

Closed someview closed 5 months ago

someview commented 5 months ago

Now mapof delete a key using code like this:

                    if del {
                        // Deletion.
                        // First we update the hash, then the entry.
                        atomic.StoreUint64(&b.hashes[i], uint64(0))
                        atomic.StorePointer(&b.entries[i], nil)
                        leftEmpty := false
                        if hintNonEmpty == 0 {
                            leftEmpty = isEmptyBucketOf(b)
                        }
                        rootb.mu.Unlock()
                        table.addSize(bidx, -1)
                        // Might need to shrink the table.
                        if leftEmpty {
                            m.resize(table, mapShrinkHint)
                        }
                        return oldv, !computeOnly
                    }

This may cause greatly late when delete operation is frequent. How about consider this instead of the empty percentage of buckets as condition.

puzpuzpuz commented 5 months ago

Such an approach won't release memory if the map grows to a few million entries and then all but 100 entries are deleted. The current code will be shrinking the map occasionally.

someview commented 5 months ago

Such an approach won't release memory if the map grows to a few million entries and then all but 100 entries are deleted. The current code will be shrinking the map occasionally.

We have met the case: for server, a stateful client store all topics comes from the conn. a gobal concurrent topic-client hashmap When we shutdown a stateful client , we would clear its resouces(a lot of topic key would be deleted from the global map). Such a sudden deletion of a large amount of data may cause performance jitter.

puzpuzpuz commented 5 months ago

I see. For such use cases, the map could be marked grow-only, so it never shrinks. This would be enabled through a new constructor function, say, NewGrowOnlyMapOf. WDYT?

someview commented 5 months ago

Good idea. It seems we that should reuse the memory space when a kvpair would be updated.

puzpuzpuz commented 5 months ago

Good to know. I'll add these constructors - hopefully this weekend.

puzpuzpuz commented 5 months ago

I've published v3.2.0, so you can now configure a map to be grow-only:

m := xsync.NewMapOf[int, int](WithGrowOnly())
puzpuzpuz commented 5 months ago

Closing this issue for now as the feature is shipped. Let me know if you face any problems.