La méthode HashSet <T> .removeAll est étonnamment lente

92

Jon Skeet a récemment soulevé un sujet de programmation intéressant sur son blog: "Il y a un trou dans mon abstraction, chère Liza, chère Liza" (italiques ajoutés):

J'ai un ensemble - un HashSet, en fait. Je souhaite supprimer certains éléments de celui-ci… et de nombreux éléments peuvent bien ne pas exister. En fait, dans notre cas de test, aucun des éléments de la collection «suppressions» ne sera dans l'ensemble d'origine. Cela semble - et est en effet - extrêmement facile à coder. Après tout, nous devons Set<T>.removeAllnous aider, non?

Nous spécifions la taille de l'ensemble «source» et la taille de la collection «suppressions» sur la ligne de commande, et construisons les deux. L'ensemble source contient uniquement des entiers non négatifs; l'ensemble des suppressions ne contient que des entiers négatifs. Nous mesurons le temps nécessaire pour supprimer tous les éléments à l'aide de System.currentTimeMillis(), ce qui n'est pas le chronomètre le plus précis au monde, mais qui est plus que suffisant dans ce cas, comme vous le verrez. Voici le code:

import java.util.*;
public class Test 
{ 
    public static void main(String[] args) 
    { 
       int sourceSize = Integer.parseInt(args[0]); 
       int removalsSize = Integer.parseInt(args[1]); 
        
       Set<Integer> source = new HashSet<Integer>(); 
       Collection<Integer> removals = new ArrayList<Integer>(); 
        
       for (int i = 0; i < sourceSize; i++) 
       { 
           source.add(i); 
       } 
       for (int i = 1; i <= removalsSize; i++) 
       { 
           removals.add(-i); 
       } 
        
       long start = System.currentTimeMillis(); 
       source.removeAll(removals); 
       long end = System.currentTimeMillis(); 
       System.out.println("Time taken: " + (end - start) + "ms"); 
    }
}

Commençons par lui donner une tâche facile: un ensemble source de 100 éléments et 100 à supprimer:

c:UsersJonTest>java Test 100 100
Time taken: 1ms

D'accord, nous ne nous attendions pas à ce que ce soit lent… clairement, nous pouvons accélérer les choses un peu. Que diriez-vous d'une source d'un million d'articles et de 300 000 articles à supprimer?

c:UsersJonTest>java Test 1000000 300000
Time taken: 38ms

Hmm. Cela semble encore assez rapide. Maintenant, je sens que j'ai été un peu cruel, lui demandant de faire tout ce retrait. Facilitons un peu la tâche - 300 000 éléments sources et 300 000 suppressions:

c:UsersJonTest>java Test 300000 300000
Time taken: 178131ms

Pardon? Près de trois minutes ? Yikes! Il devrait sûrement être plus facile de supprimer des éléments d'une collection plus petite que celle que nous avons gérée en 38 ms?

Quelqu'un peut-il expliquer pourquoi cela se produit? Pourquoi la HashSet<T>.removeAllméthode est-elle si lente?

Communauté
la source
2
J'ai testé votre code et cela a fonctionné rapidement. Pour votre cas, il a fallu environ 12 ms pour terminer. J'ai également augmenté les deux valeurs d'entrée de 10 et cela a pris 36 ms. Peut-être que votre PC effectue des tâches intensives sur le processeur pendant que vous exécutez les tests?
Slimu
4
Je l'ai testé, et j'ai eu le même résultat que l'OP (enfin, je l'ai arrêté avant la fin). Etrange en effet. Windows, JDK 1.7.0_55
JB Nizet
2
Il y a un billet ouvert sur ceci: JDK-6982173
Haozhun
44
Comme discuté sur Meta , cette question a été à l'origine plagiée du blog de Jon Skeet (maintenant directement citée et liée à la question, en raison de la modification d'un modérateur). Les futurs lecteurs doivent noter que l'article de blog dont il a été plagié explique en fait la cause du comportement, de la même manière que la réponse acceptée ici. En tant que tel, au lieu de lire les réponses ici, vous souhaiterez peut-être simplement cliquer et lire le blog complet .
Mark Amery
1
Le bogue sera corrigé dans Java 15: JDK-6394757
ZhekaKozlov le

Réponses:

138

Le comportement est (quelque peu) documenté dans le javadoc :

Cette implémentation détermine quel est le plus petit de cet ensemble et de la collection spécifiée, en appelant la méthode size sur chacun. Si cet ensemble comporte moins d'éléments , l'implémentation effectue une itération sur cet ensemble, vérifiant à son tour chaque élément retourné par l'itérateur pour voir s'il est contenu dans la collection spécifiée . S'il en est ainsi, il est supprimé de cet ensemble avec la méthode remove de l'itérateur. Si la collection spécifiée comporte moins d'éléments, l'implémentation effectue une itération sur la collection spécifiée, en supprimant de cet ensemble chaque élément renvoyé par l'itérateur, à l'aide de la méthode remove de cet ensemble.

Ce que cela signifie en pratique, lorsque vous appelez source.removeAll(removals);:

  • si la removalscollection est d'une taille inférieure à source, la removeméthode de HashSetest appelée, ce qui est rapide.

  • si la removalscollection est de taille égale ou supérieure à source, alors removals.containsest appelée, ce qui est lent pour une ArrayList.

Solution rapide:

Collection<Integer> removals = new HashSet<Integer>();

Notez qu'il existe un bogue ouvert très similaire à ce que vous décrivez. L'essentiel semble être que c'est probablement un mauvais choix mais qu'il ne peut pas être changé car il est documenté dans le javadoc.


Pour référence, voici le code de removeAll(dans Java 8 - je n'ai pas vérifié les autres versions):

public boolean removeAll(Collection<?> c) {
    Objects.requireNonNull(c);
    boolean modified = false;

    if (size() > c.size()) {
        for (Iterator<?> i = c.iterator(); i.hasNext(); )
            modified |= remove(i.next());
    } else {
        for (Iterator<?> i = iterator(); i.hasNext(); ) {
            if (c.contains(i.next())) {
                i.remove();
                modified = true;
            }
        }
    }
    return modified;
}
assylies
la source
15
Sensationnel. J'ai appris quelque chose aujourd'hui. Cela me semble être un mauvais choix d'implémentation. Ils ne doivent pas le faire si l'autre collection n'est pas un ensemble.
JB Nizet
2
@JBNizet Oui, c'est bizarre - cela a été discuté ici avec votre suggestion - je ne sais pas pourquoi cela n'a pas été fait ...
assylias
2
Merci beaucoup @assylias .. Mais je me demande vraiment comment vous l'avez compris .. :) Bien vraiment sympa .... Avez-vous fait face à ce problème ???
8
@show_stopper Je viens de lancer un profileur et j'ai vu que ArrayList#containsc'était le coupable. Un regard sur le code de a AbstractSet#removeAlldonné le reste de la réponse.
assylias