Traversal and removing elements from ConcurrentskipListSet

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Traversal and removing elements from ConcurrentskipListSet

Dileep Mandapam
Hi 

Environment details:-
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)



 Removing an element while traversal . Sometimes elements are not removing from concurrentskiplistset. 

for( Element e : concurrentskiplistsetcollection) {
 
     if ( e is to be removed) {
            concurrentskiplistset.remove(e);
    }

}

My question is safe to use above to pattern to remove elements ?
--
Regards
Dileep M

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Traversal and removing elements from ConcurrentskipListSet

Michael Kuhlmann
Hi Dileep,

please read the JavaDoc of ConcurrentSkipListSet. It refers to the
"weakly consistent" section of the package JavaDoc. This states:

***
[Iterators] are guaranteed to traverse elements as they existed upon
construction exactly once, and may (but are not guaranteed to) reflect
any modifications subsequent to construction.
***

So yes, this should work as expected as long as every e is unique.
However, it's always better to directly use the Iterator instead of the
short for loop, and call remove() on the iterator. This will work for
all modifiable Iterable implementations, and it will work for multiple
entries which are equal to each other.

-Michael


Am 16.05.2017 um 14:43 schrieb Dileep Mandapam:

> Hi
>
> Environment details:-
> java version "1.8.0_131"
> Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
> Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)
>
>
>
>  Removing an element while traversal . Sometimes elements are not
> removing from concurrentskiplistset.
>
> for( Element e : concurrentskiplistsetcollection) {
>  
>      if ( e is to be removed) {
>             concurrentskiplistset.remove(e);
>     }
>
> }
>
> My question is safe to use above to pattern to remove elements ?
> --
> Regards
> Dileep M
>
>
> _______________________________________________
> Concurrency-interest mailing list
> [hidden email]
> http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Traversal and removing elements from ConcurrentskipListSet

Martin Buchholz-3
In reply to this post by Dileep Mandapam
There's a new API for what you're trying to do - removeIf
(and probably there's work to be done on our part making it optimal)

On Tue, May 16, 2017 at 5:43 AM, Dileep Mandapam <[hidden email]> wrote:
Hi 

Environment details:-
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)



 Removing an element while traversal . Sometimes elements are not removing from concurrentskiplistset. 

for( Element e : concurrentskiplistsetcollection) {
 
     if ( e is to be removed) {
            concurrentskiplistset.remove(e);
    }

}

My question is safe to use above to pattern to remove elements ?
--
Regards
Dileep M

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest



_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Traversal and removing elements from ConcurrentskipListSet

Dileep Mandapam
Thanks for your answers..

I wrote a test program to verify. 

import java.util.Comparator;
import java.util.Iterator;
import java.util.concurrent.ConcurrentSkipListSet;
import java.util.concurrent.Executors;

/**
 * Created by dmandapam on 5/16/17.
 */
public class ConcurrentSj {
    static final ConcurrentSkipListSet<MyBug> concurrentSkipListSet = new ConcurrentSkipListSet<>(new Comparator<MyBug>() {
        @Override
        public int compare(MyBug o1, MyBug o2) {
            return 1;
        }
    });
    public static void main(String... rag) throws InterruptedException {
        Executors.newSingleThreadExecutor().execute(() -> {
            while (true) {
                try {
                    Thread.sleep(100);
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
                MyBug bug = new MyBug((int) (Math.random() * 10000));
                concurrentSkipListSet.add(bug);
                System.out.println("added bug" + bug);
            }
        });
        while (true) {
            boolean flag = false;
            System.out.println("weak iterators");
            for(MyBug bug : concurrentSkipListSet) {
                if (flag) {
                    boolean remove = concurrentSkipListSet.remove(bug);
                    System.out.println("removed = "+ bug + " result = "+ remove);
                }
                flag = !flag;
            }
            /*Iterator<MyBug> iterator = concurrentSkipListSet.iterator();
            while (iterator.hasNext()) {
                MyBug bug = iterator.next();
                if (flag) {
                    boolean remove = concurrentSkipListSet.remove(bug);
                    System.out.println("removed = "+ bug + " result = "+ remove);
                }
                flag = !flag;
            }*/
            Thread.sleep(1000);
        }
    }

    static class MyBug {
        int id;

        public MyBug(int id) {
            this.id = id;
        }

        @Override
        public String toString() {
            return "id=" + id ;
        }

        /*@Override
        public boolean equals(Object o) {
            if (this == o) return true;
            if (o == null || getClass() != o.getClass()) return false;

            MyBug bug = (MyBug) o;

            return id == bug.id;
        }

        @Override
        public int hashCode() {
            return id;
        }*/
    }
}


Looks like there is a bug in ConcurrentskipListSet.

On Wed, May 17, 2017 at 6:32 AM, Martin Buchholz <[hidden email]> wrote:
There's a new API for what you're trying to do - removeIf
(and probably there's work to be done on our part making it optimal)

On Tue, May 16, 2017 at 5:43 AM, Dileep Mandapam <[hidden email]> wrote:
Hi 

Environment details:-
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)



 Removing an element while traversal . Sometimes elements are not removing from concurrentskiplistset. 

for( Element e : concurrentskiplistsetcollection) {
 
     if ( e is to be removed) {
            concurrentskiplistset.remove(e);
    }

}

My question is safe to use above to pattern to remove elements ?
--
Regards
Dileep M

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest





--
Regards
Dileep M

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest

ConcurrentSj.java (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Traversal and removing elements from ConcurrentskipListSet

Martin Buchholz-3


On Tue, May 16, 2017 at 7:36 PM, Dileep Mandapam <[hidden email]> wrote:
       public int compare(MyBug o1, MyBug o2) {
            return 1;

That's not a valid compare method. 

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Traversal and removing elements from ConcurrentskipListSet

Michael Kuhlmann
In reply to this post by Dileep Mandapam
And in addition to Martin's comments, which already named the issue,
this is not what I meant with the iterator:

Am 17.05.2017 um 04:36 schrieb Dileep Mandapam:

>             /*Iterator<MyBug> iterator = concurrentSkipListSet.iterator();
>             while (iterator.hasNext()) {
>                 MyBug bug = iterator.next();
>                 if (flag) {
>                     boolean remove = concurrentSkipListSet.remove(bug);
>                     System.out.println("removed = "+ bug + " result = "+
> remove);
>                 }
>                 flag = !flag;
>             }*/

Of course you should call remove() on the iterator itself, otherwise
that part of your code would do nothing different than the previous.

Anyway, fix your comparator, and do something with your main() method,
whatever you're expecting - currently it's stuck in an endless loop,
totally independent of your list instance and whatever you do with it.

-Michael
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Traversal and removing elements from ConcurrentskipListSet

Dileep Mandapam
Thanks, I rewrote the program. I ran into a strange situation. I am mutating a field which is used in the comparator.  Now remove method is returning false for most of the times. 


import java.util.Comparator;
import java.util.concurrent.ConcurrentSkipListSet;

/**
* Created by dmandapam on 5/17/17.
*/
public class RemoveTest {
static final ConcurrentSkipListSet<MyBug> concurrentSkipListSet = new ConcurrentSkipListSet<>(new Comparator<MyBug>() {
@Override
public int compare(MyBug o1, MyBug o2) {
int result = Integer.compare(o1.getId(), o2.getId());
return result;
}
});
public static void main(String... args) {
MyBug bug1 = new MyBug(10);
concurrentSkipListSet.add(bug1);
concurrentSkipListSet.add(new MyBug(20));
concurrentSkipListSet.add(new MyBug(30));
bug1.setId(50);
boolean remove = concurrentSkipListSet.remove(new MyBug(30));
System.out.println("remove " + remove);

}
static class MyBug {
int id;

public MyBug(int id) {
this.id = id;
}

@Override
public String toString() {
return "id=" + id ;
}

public void setId(int id) {
this.id = id;
}

public int getId() {
return id;
}
}


But if i try  with TreeSet then remove method always returns true. Could you guys help me in uderstanding the problem.

On Wed, May 17, 2017 at 12:59 PM, Michael Kuhlmann <[hidden email]> wrote:
And in addition to Martin's comments, which already named the issue,
this is not what I meant with the iterator:

Am 17.05.2017 um 04:36 schrieb Dileep Mandapam:
>             /*Iterator<MyBug> iterator = concurrentSkipListSet.iterator();
>             while (iterator.hasNext()) {
>                 MyBug bug = iterator.next();
>                 if (flag) {
>                     boolean remove = concurrentSkipListSet.remove(bug);
>                     System.out.println("removed = "+ bug + " result = "+
> remove);
>                 }
>                 flag = !flag;
>             }*/

Of course you should call remove() on the iterator itself, otherwise
that part of your code would do nothing different than the previous.

Anyway, fix your comparator, and do something with your main() method,
whatever you're expecting - currently it's stuck in an endless loop,
totally independent of your list instance and whatever you do with it.

-Michael
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest



--
Regards
Dileep M

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Traversal and removing elements from ConcurrentskipListSet

Michael Kuhlmann
You mustn't change the internal state of already inserted elements so
that compare() (or hash() in other cases) changes. This is true for all
cases where such a state is used to balance a structure, like HashMap
and TreeMap and such. The internal ordering in this container will be
wrong then.

This is not really an issue with the ConcurrentSkipListSet, or with
concurrency in general. I recommend you to learn about the correct usage
of Java's basic collection classes before you start with this.

Good luck,
Michael


Am 17.05.2017 um 16:10 schrieb Dileep Mandapam:

> Thanks, I rewrote the program. I ran into a strange situation. I am
> mutating a field which is used in the comparator.  Now remove method is
> returning false for most of the times.
>
>
> import java.util.Comparator;
> import java.util.concurrent.ConcurrentSkipListSet;
>
> /**
> * Created by dmandapam on 5/17/17.
> */
> public class RemoveTest {
> static final ConcurrentSkipListSet<MyBug> concurrentSkipListSet = new ConcurrentSkipListSet<>(new Comparator<MyBug>() {
> @Override
> public int compare(MyBug o1, MyBug o2) {
> int result = Integer.compare(o1.getId(), o2.getId());
>             return result;
>         }
> });
>     public static void main(String... args) {
> MyBug bug1 = new MyBug(10);
>         concurrentSkipListSet.add(bug1);
>         concurrentSkipListSet.add(new MyBug(20));
>         concurrentSkipListSet.add(new MyBug(30));
>         bug1.setId(50);
>         boolean remove = concurrentSkipListSet.remove(new MyBug(30));
>         System.out.println("remove " + remove);
>
>     }
> static class MyBug {
> int id;
>
>         public MyBug(int id) {
> this.id = id;
>         }
>
> @Override
> public String toString() {
> return "id=" + id ;
>         }
>
> public void setId(int id) {
> this.id = id;
>         }
>
> public int getId() {
> return id;
>         }
> }
>
>
>
> *But if i try with TreeSet then remove method always returns true. Could
> you guys help me in uderstanding the problem.*
>
>
> On Wed, May 17, 2017 at 12:59 PM, Michael Kuhlmann <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     And in addition to Martin's comments, which already named the issue,
>     this is not what I meant with the iterator:
>
>     Am 17.05.2017 um 04:36 schrieb Dileep Mandapam:
>     >             /*Iterator<MyBug> iterator = concurrentSkipListSet.iterator();
>     >             while (iterator.hasNext()) {
>     >                 MyBug bug = iterator.next();
>     >                 if (flag) {
>     >                     boolean remove = concurrentSkipListSet.remove(bug);
>     >                     System.out.println("removed = "+ bug + " result = "+
>     > remove);
>     >                 }
>     >                 flag = !flag;
>     >             }*/
>
>     Of course you should call remove() on the iterator itself, otherwise
>     that part of your code would do nothing different than the previous.
>
>     Anyway, fix your comparator, and do something with your main() method,
>     whatever you're expecting - currently it's stuck in an endless loop,
>     totally independent of your list instance and whatever you do with it.
>
>     -Michael
>     _______________________________________________
>     Concurrency-interest mailing list
>     [hidden email]
>     <mailto:[hidden email]>
>     http://cs.oswego.edu/mailman/listinfo/concurrency-interest
>     <http://cs.oswego.edu/mailman/listinfo/concurrency-interest>
>
>
>
>
> --
> Regards
> Dileep M

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Traversal and removing elements from ConcurrentskipListSet

Brian S O'Neill-3
In reply to this post by Dileep Mandapam
What are you trying to do exactly? Identify how the set fails when the
behavior it depends is broken? When an entry is added to the set with a
specific comparator result, the result must not change for as long as
the entry exists in the set. In general, comparators must not depend on
mutable state. Your comparator is broken.

On 2017-05-17 07:10 AM, Dileep Mandapam wrote:

> Thanks, I rewrote the program. I ran into a strange situation. I am
> mutating a field which is used in the comparator.  Now remove method is
> returning false for most of the times.
>
>
> import java.util.Comparator;
> import java.util.concurrent.ConcurrentSkipListSet;
>
> /**
> * Created by dmandapam on 5/17/17.
> */
> public class RemoveTest {
> static final ConcurrentSkipListSet<MyBug> concurrentSkipListSet =new ConcurrentSkipListSet<>(new Comparator<MyBug>() {
> @Override
> public int compare(MyBug o1,MyBug o2) {
> int result =Integer.compare(o1.getId(),o2.getId());
>              return result;
>          }
> });
>      public static void main(String...args) {
> MyBug bug1 =new MyBug(10);
>          concurrentSkipListSet.add(bug1);
>          concurrentSkipListSet.add(new MyBug(20));
>          concurrentSkipListSet.add(new MyBug(30));
>          bug1.setId(50);
>          boolean remove =concurrentSkipListSet.remove(new MyBug(30));
>          System.out.println("remove " + remove);
>
>      }
> static class MyBug {
> int id;
>
>          public MyBug(int id) {
> this.id =id;
>          }
>
> @Override
> public String toString() {
> return "id=" +id ;
>          }
>
> public void setId(int id) {
> this.id =id;
>          }
>
> public int getId() {
> return id;
>          }
> }
>
>
>
> *But if i try with TreeSet then remove method always returns true. Could
> you guys help me in uderstanding the problem.*
>
>
_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Traversal and removing elements from ConcurrentskipListSet

Dileep Mandapam
Ok thanks

On 17-May-2017 8:01 PM, "Brian S O'Neill" <[hidden email]> wrote:
What are you trying to do exactly? Identify how the set fails when the behavior it depends is broken? When an entry is added to the set with a specific comparator result, the result must not change for as long as the entry exists in the set. In general, comparators must not depend on mutable state. Your comparator is broken.

On 2017-05-17 07:10 AM, Dileep Mandapam wrote:
Thanks, I rewrote the program. I ran into a strange situation. I am mutating a field which is used in the comparator.  Now remove method is returning false for most of the times.


import java.util.Comparator;
import java.util.concurrent.ConcurrentSkipListSet;

/**
* Created by dmandapam on 5/17/17.
*/
public class RemoveTest {
static final ConcurrentSkipListSet<MyBug> concurrentSkipListSet =new ConcurrentSkipListSet<>(new Comparator<MyBug>() {
@Override
public int compare(MyBug o1,MyBug o2) {
int result =Integer.compare(o1.getId(),o2.getId());
             return result;
         }
});
     public static void main(String...args) {
MyBug bug1 =new MyBug(10);
         concurrentSkipListSet.add(bug1);
         concurrentSkipListSet.add(new MyBug(20));
         concurrentSkipListSet.add(new MyBug(30));
         bug1.setId(50);
         boolean remove =concurrentSkipListSet.remove(new MyBug(30));
         System.out.println("remove " + remove);

     }
static class MyBug {
int id;

         public MyBug(int id) {
this.id =id;
         }

@Override
public String toString() {
return "id=" +id ;
         }

public void setId(int id) {
this.id =id;
         }

public int getId() {
return id;
         }
}



*But if i try with TreeSet then remove method always returns true. Could you guys help me in uderstanding the problem.*


_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest

_______________________________________________
Concurrency-interest mailing list
[hidden email]
http://cs.oswego.edu/mailman/listinfo/concurrency-interest
Loading...