Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultiMap RemoveOperation iterate through backing collection causes performance degradation when using SET collection type #14145

Closed
gokhanoner opened this Issue Nov 19, 2018 · 0 comments

Comments

Projects
None yet
4 participants
@gokhanoner
Copy link
Contributor

gokhanoner commented Nov 19, 2018

Please see https://github.com/hazelcast/hazelcast/blob/master/hazelcast/src/main/java/com/hazelcast/multimap/impl/operations/RemoveOperation.java#L57

The only reason to iterate is to get recordId from removed record & use it to remove the backup entry. Instead, we can just use collection.remove() which gives O(1) remove performance when using SET collection:

    Collection<MultiMapRecord> coll = multiMapValue.getCollection(false);
    MultiMapRecord record = new MultiMapRecord(isBinary() ? value : toObject(value));
    //use collection.remove(record) to speed up SET
    response = coll.remove(record);
    if (coll.isEmpty()) {
        container.delete(dataKey);
    }

RemoveBakupOperation can also do the same, instead of passing the recordId, we simply pass value to remove.

The only thing to consider is compatibility during rolling upgrade I guess.

Some perf. numbers ( in milliseconds ):

2 Node Cluster, 1 Client, all local

    Multimap Remove Performance  
    # of values per key 100K
    # of remove operations 50K
    Value type Integer
 
Remove from index Backing Collection Current (3.11) Change RemoveOperation to use collection.remove() instead of iteration
0 SET 109689 4998
  LIST 4979 4896
20K SET 108299 4998
  LIST 29053 30636

test code:

    static long removeTest(int start, int cnt, Consumer<Integer> opr) {
        Instant begin = Instant.now();
        int limit = start + cnt;
        try {
            for (int i = start; i < limit; i++) {
                opr.accept(i);
            }
        } finally {
            Instant end = Instant.now();
            return Duration.between(begin, end).toMillis();
        }
    }

///...
removeTest(0, 50_000, i -> mmap.remove(1, i))
removeTest(20_000, 50_000, i -> mmap.remove(1, i))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.