My naive version now is too slow. I think setting/accessing concurrent atomic bit is way slower compared to access/modify an array of boolean. Second, the parallel execution only happens on the beginning of the process. For bigger numbers, thread-pool is not really active.
public class EratosthenesSieve {
private final ExecutorService executorService;
private final int n;
private final AtomicBitSet bitSet;
private final int squareRoot;
private final Node head;
private final Node trailer;
public EratosthenesSieve(int n) {
this.n = n;
this.squareRoot = (int) Math.sqrt(n);
bitSet = new AtomicBitSet(n + 1);
executorService = Executors.newCachedThreadPool();
Node h = new Node(0, null, null);
Node t = new Node(0, null, h);
h.setNext(t);
head = h;
trailer = t;
}
public Node addLast(int i) {
Node node;
while ((node = trailer.prepend(i)) == null)
;
return node;
}
class FlagIdeal
implements Runnable {
private final int candidate;
FlagIdeal(int candidate) {
this.candidate = candidate;
}
@Override
public void run() {
int j = bitSet.firstZeroAfter(candidate);
if (j >= n) {
executorService.shutdown();
return;
}
int prime = j;
for (Node<Integer> iterator = head.forward(); iterator != null; iterator = iterator.forward()) {
// with a good scheduler, it should never happen
if (prime > iterator.element) {
executorService.submit(this);
return;
}
}
// after the iteration over the list of restrictions, something may change
if (bitSet.get(prime)) {
executorService.submit(this);
return;
}
// work is over
if (prime > squareRoot) {
executorService.shutdown();
return;
}
int lastNumber = prime * prime;
// before adding work, add a new restriction
Node activeRestriction = addLast(lastNumber);
executorService.submit(new FlagIdeal(prime + 1));
// now, mark all multiples of this prime number starting from the square
bitSet.set(lastNumber);
lastNumber = lastNumber + prime;
while (lastNumber <= n) {
bitSet.set(lastNumber);
activeRestriction.element = lastNumber;
lastNumber = lastNumber + prime;
}
// remove the restriction
while (!activeRestriction.delete() && !activeRestriction.isDeleted())
;
}
private void surroundedSleep(int i) {
try {
Thread.sleep(i);
} catch (InterruptedException e) {
new RuntimeException(e);
}
}
}
public static void main(String[] args) throws InterruptedException {
EratosthenesSieve eratosthenesSieve = new EratosthenesSieve(100000000);
long initial = System.nanoTime();
eratosthenesSieve.execute();
System.out.println(System.nanoTime() - initial);
}
private void execute() throws InterruptedException {
executorService.submit(new FlagIdeal(2));
while (!executorService.isTerminated()) {
Thread.sleep(100);
}
int count = 0;
for (int i = 2; i < n; i++) {
if (!bitSet.get(i)) {
count++;
// System.out.print(i);
// System.out.print(' ');
}
}
System.out.println();
System.out.println(count);
}
}
Here, kind of the bottleneck
public class AtomicBitSet {
public static final int CHUNK_SIZE = 64;
public static final int CHUNK_SIZE_MINUS_ONE = 63;
public static final int CHUNK_BITS = 6;
// Adapted <From StackOverflow url="http://stackoverflow.com/a/12425007/1879686" author="Peter Lawrey">
private final AtomicLongArray array;
public AtomicBitSet(int length) {
int intLength = (length + CHUNK_SIZE_MINUS_ONE) / CHUNK_SIZE;
array = new AtomicLongArray(intLength);
}
public void set(long n) {
long bit = 1l << n;
int idx = (int) (n >>> CHUNK_BITS);
while (true) {
long num = array.get(idx);
long num2 = num | bit;
if (num == num2 || array.compareAndSet(idx, num, num2))
return;
}
}
public boolean get(long n) {
long bit = 1l << n;
int idx = (int) (n >>> CHUNK_BITS);
long num = array.get(idx);
return (num & bit) != 0;
}
// </From StackOverflow>
public int firstZeroAfter(long n){
int idx = (int) (n >>> CHUNK_BITS);
long bit = 1l << (n % CHUNK_SIZE);
long oneMask = bit - 1;
long num = array.get(idx);
int trailingOnes = Long.numberOfTrailingZeros(~num & ~oneMask);
while(trailingOnes == CHUNK_SIZE && idx < array.length() -1){
idx++;
num = array.get(idx);
trailingOnes = Long.numberOfTrailingZeros(~num);
}
return (idx << CHUNK_BITS) + trailingOnes;
}
}
Suggestions?
Trivia: Parallel execution safety
I am using a ConcurrentDoubleLinkedList
written by Doug Lea with assistance from members of JCP JSR-166 Expert Group to "lock" the bigger number that can be safely considered as a prime. When you read Node
in the code, it is a node of this list.
Below is the Node slightly modified:
import java.util.concurrent.atomic.AtomicReference;
/**
* Linked Nodes. As a minor efficiency hack, this class opportunistically inherits from AtomicReference, with the atomic
* ref used as the "next" link.
*
* Nodes are in doubly-linked lists. There are three kinds of special nodes, distinguished by: * The list header has a
* null prev link * The list trailer has a null next link * A deletion marker has a prev link pointing to itself. All
* three kinds of special nodes have null element fields.
*
* Regular nodes have non-null element, next, and prev fields. To avoid visible inconsistencies when deletions overlap
* element replacement, replacements are done by replacing the node, not just setting the element.
*
* Nodes can be traversed by read-only ConcurrentLinkedDeque class operations just by following raw next pointers, so
* long as they ignore any special nodes seen along the way. (This is automated in method forward.) However, traversal
* using prev pointers is not guaranteed to see all live nodes since a prev pointer of a deleted node can become
* unrecoverably stale.
*/
public class Node
extends AtomicReference<Node> {
private volatile Node prev;
int element;
/** Creates a node with given contents */
Node(int element, Node next, Node prev) {
super(next);
this.prev = prev;
this.element = element;
}
/** Creates a marker node with given successor */
Node(Node next) {
super(next);
this.prev = this;
this.element = 0;
}
/**
* Gets next link (which is actually the value held as atomic reference).
*/
private Node getNext() {
return get();
}
/**
* Sets next link
*
* @param n
* the next node
*/
void setNext(Node n) {
set(n);
}
/**
* compareAndSet next link
*/
private boolean casNext(Node cmp, Node val) {
return compareAndSet(cmp, val);
}
/**
* Gets prev link
*/
private Node getPrev() {
return prev;
}
/**
* Sets prev link
*
* @param b
* the previous node
*/
void setPrev(Node b) {
prev = b;
}
/**
* Returns true if this is a header, trailer, or marker node
*/
boolean isSpecial() {
return element == 0;
}
/**
* Returns true if this is a trailer node
*/
boolean isTrailer() {
return getNext() == null;
}
/**
* Returns true if this is a header node
*/
boolean isHeader() {
return getPrev() == null;
}
/**
* Returns true if this is a marker node
*/
boolean isMarker() {
return getPrev() == this;
}
/**
* Returns true if this node is followed by a marker, meaning that it is deleted.
*
* @return true if this node is deleted
*/
boolean isDeleted() {
Node f = getNext();
return f != null && f.isMarker();
}
/**
* Returns next node, ignoring deletion marker
*/
private Node nextNonmarker() {
Node f = getNext();
return (f == null || !f.isMarker()) ? f : f.getNext();
}
/**
* Returns the next non-deleted node, swinging next pointer around any encountered deleted nodes, and also patching
* up successor''s prev link to point back to this. Returns null if this node is trailer so has no successor.
*
* @return successor, or null if no such
*/
Node successor() {
Node f = nextNonmarker();
for (;;) {
if (f == null)
return null;
if (!f.isDeleted()) {
if (f.getPrev() != this && !isDeleted())
f.setPrev(this); // relink f's prev
return f;
}
Node s = f.nextNonmarker();
if (f == getNext())
casNext(f, s); // unlink f
f = s;
}
}
/**
* Returns the apparent predecessor of target by searching forward for it starting at this node, patching up
* pointers while traversing. Used by predecessor().
*
* @return target's predecessor, or null if not found
*/
private Node findPredecessorOf(Node target) {
Node n = this;
for (;;) {
Node f = n.successor();
if (f == target)
return n;
if (f == null)
return null;
n = f;
}
}
/**
* Returns the previous non-deleted node, patching up pointers as needed. Returns null if this node is header so has
* no successor. May also return null if this node is deleted, so doesn't have a distinct predecessor.
*
* @return predecessor or null if not found
*/
Node predecessor() {
Node n = this;
for (;;) {
Node b = n.getPrev();
if (b == null)
return n.findPredecessorOf(this);
Node s = b.getNext();
if (s == this)
return b;
if (s == null || !s.isMarker()) {
Node p = b.findPredecessorOf(this);
if (p != null)
return p;
}
n = b;
}
}
/**
* Returns the next node containing a nondeleted user element. Use for forward list traversal.
*
* @return successor, or null if no such
*/
Node forward() {
Node f = successor();
return (f == null || f.isSpecial()) ? null : f;
}
/**
* Returns previous node containing a nondeleted user element, if possible. Use for backward list traversal, but
* beware that if this method is called from a deleted node, it might not be able to determine a usable predecessor.
*
* @return predecessor, or null if no such could be found
*/
Node back() {
Node f = predecessor();
return (f == null || f.isSpecial()) ? null : f;
}
/**
* Tries to insert a node holding element as successor, failing if this node is deleted.
*
* @param element
* the element
* @return the new node, or null on failure.
*/
Node append(int element) {
for (;;) {
Node f = getNext();
if (f == null || f.isMarker())
return null;
Node x = new Node(element, f, this);
if (casNext(f, x)) {
f.setPrev(x); // optimistically link
return x;
}
}
}
/**
* Tries to insert a node holding element as predecessor, failing if no live predecessor can be found to link to.
*
* @param element
* the element
* @return the new node, or null on failure.
*/
Node prepend(int element) {
for (;;) {
Node b = predecessor();
if (b == null)
return null;
Node x = new Node(element, this, b);
if (b.casNext(this, x)) {
setPrev(x); // optimistically link
return x;
}
}
}
/**
* Tries to mark this node as deleted, failing if already deleted or if this node is header or trailer
*
* @return true if successful
*/
boolean delete() {
Node b = getPrev();
Node f = getNext();
if (b != null && f != null && !f.isMarker() && casNext(f, new Node(f))) {
if (b.casNext(this, f))
f.setPrev(b);
return true;
}
return false;
}
/**
* Tries to insert a node holding element to replace this node. failing if already deleted.
*
* @param newElement
* the new element
* @return the new node, or null on failure.
*/
Node replace(int newElement) {
for (;;) {
Node b = getPrev();
Node f = getNext();
if (b == null || f == null || f.isMarker())
return null;
Node x = new Node(newElement, f, b);
if (casNext(f, new Node(x))) {
b.successor(); // to relink b
x.successor(); // to relink f
return x;
}
}
}
@Override
public String toString() {
return Integer.toString(element);
}
}
Node
? Your code is incomplete without it. \$\endgroup\$Node
implementation you link, on account of attempting to assign a new value tofinal
fieldNode.element
. Evidently you have modified it; we cannot review code you do not provide. \$\endgroup\$<E>
and exchanged it toint
because I was trying another optimization but it was pointless. \$\endgroup\$