Skip to content

add answers to exercise 1-7 chapter 3 #16

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

Merged
merged 13 commits into from
Sep 14, 2015
Prev Previous commit
Next Next commit
use atomic reference of a linked list insteadoff linked list of atomi…
…c references
  • Loading branch information
ryblovAV committed Sep 1, 2015
commit 51ec10c1a845aca7e42731efb19114c006b4441c
80 changes: 51 additions & 29 deletions src/main/scala/org/learningconcurrency/exercises/ch3/ex3_4.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@ package org.learningconcurrency
package exercises
package ch3

import java.util.ConcurrentModificationException
import java.util.concurrent.atomic.AtomicReference

import org.learningconcurrency.ch2._

import scala.annotation.tailrec

/**
* Implement a ConcurrentSortedList class, which implements a concurrent
*
* class ConcurrentSortedList[T](implicit val ord: Ordering[T]) {
* def add(x: T): Unit = ???
* def iterator: Iterator[T] = ???
* }
* def add(x: T): Unit = ???
* def iterator: Iterator[T] = ???
* }
*
* Under the hood, the ConcurrentSortedList class should use a linked list of atomic references.
* Ensure that your implementation is lock-free and avoids ABA problems.
Expand All @@ -30,40 +31,59 @@ object Ex3_4 extends App {

class ConcurrentSortedList[T](implicit val ord: Ordering[T]) {

val r = new AtomicReference[List[T]](List.empty[T])

def addWithSort(x:T,l:List[T]):List[T] = l match {
case h::_ if ord.compare(h,x) >= 0 => x::l
case h::t => h::addWithSort(x,t)
case _ => List(x)
private val h: AtomicReference[Option[T]] = new AtomicReference[Option[T]](None)
private val t: AtomicReference[Option[ConcurrentSortedList[T]]] = new AtomicReference[Option[ConcurrentSortedList[T]]](None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would model the list with the following inner class:

class Node(val head: T) {
  val tail = new AtomicReference[Option[Node]](None)
}

The ConcurrentSortedList would then contain a single reference:

val root = AtomicReference[Option[Node]](None)

This would make the implementation of the add method simpler, and easier to reason about. There would be no need to call set in addToTail, since you set it only once anyway.


@tailrec
private def addToTail(x: T): Unit = {
val v = t.get
v match {
case Some(p) => p.add(x)
case None => {
val l = new ConcurrentSortedList[T]
l.h.set(Some(x))
if (!t.compareAndSet(v, Some(l))) addToTail(x)
}
}
}

def add(x: T): Unit = {
val oldList = r.get
val newList = addWithSort(x,oldList)

if (!r.compareAndSet(oldList,newList)) add(x)
@tailrec
final def add(x: T): Unit = {
val optV = h.get

optV match {
case Some(v) => {
if (ord.compare(v, x) >= 0) {
if (h.compareAndSet(optV, Some(x))) addToTail(v)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this compareAndSet, you temporarily remove the value v out of the list. If some other thread now searches for the element in the list during this time, it won't be found (if you had to implement lookup operation, it would be problematic).

Also, adding like this will cause an unnecessary ripple effect, where the values are shifted to the end of the list, instead of just prepending one node to the beginning of the list (you need one CAS for the insert in the best solution, here you can have up to n CASes).

else add(x)
} else addToTail(x)
}
case None => {
if (!h.compareAndSet(optV, Some(x)))
add(x)
}
}
}

def iterator: Iterator[T] = new Iterator[T] {

val rIter = new AtomicReference(r.get)
var rIter: Option[ConcurrentSortedList[T]] = Some(ConcurrentSortedList.this)

override def hasNext: Boolean = {
!rIter.get.isEmpty
}
override def hasNext: Boolean = rIter.isEmpty == false

override def next(): T = {
val l = rIter.get

if (l.isEmpty) throw new NoSuchElementException("next on empty iterator")

if (!rIter.compareAndSet(l,l.tail))
throw new ConcurrentModificationException()

l.head
rIter match {
case Some(r) => {
rIter = r.t.get
r.h.get match {
case Some(v) => v
case None => throw new NoSuchElementException("next on empty iterator")
}
}
case None => throw new NoSuchElementException("next on empty iterator")
}
}

}

}
Expand All @@ -72,12 +92,14 @@ object Ex3_4 extends App {


(1 to 10).map((i) => thread {
Thread.sleep((Math.random() * 100).toInt)
for (i <- 1 to 10)
csl.add((math.random * 100 + i).toInt)
Thread.sleep(1)
}
}
).foreach(_.join)

log(s"length = ${csl.iterator.length}")

for (a <- csl.iterator) {
log(a.toString)
}
Expand Down