1

I'm using a program to run the Collatz Conjecture (http://en.wikipedia.org/wiki/Collatz_conjecture) from mathematics. I've implemented a class that runs the conjecture algorithm (and gives you back the output) and one that creates a fixed thread pool (with my number of processors: 8) and accepts Callables which are calls for the conjecture algorithm.

I created a HashSet<Callable> for all the numbers between 1 (the input type must be a positive integer) and 400,000. This hangs (seemingly) forever, but lower numbers work out just fine, which is strange. Stranger yet, running it appears to take longer to process these calls than it takes a single thread to process the same amount of information; it also bloats the memory significantly.

For instance, on my computer, the program takes less than a second to perform the algorithm (just one iteration) with 400,000 (the final value) and all the lower values take less time to compute (maybe with the exception of primes, which take longer) I'm running Windows 8.1 with 8GB ram, and 8 logical processors at 2.2Ghz.

Code:

private static void initThreads() throws InterruptedException {
    //Files.createDirectories(SEQUENCER_FOLDER_PATH);
    //Files.createFile(SEQUENCER_FILE_PATH);
    ExecutorService service = Executors.newFixedThreadPool(8, new ThreadFactory() {
        private BigInteger count = BigInteger.ZERO;

        @Override
        public Thread newThread(Runnable r) {
            count = count.add(BigInteger.ONE);
            return new Thread(r, "Collatz Sequencer Thread: " + count);
        }
    });
    int finalNumber = 400_000;
    final HashSet<Callable<Void>> tasks = new HashSet<>(finalNumber);
    for (long l = 1; l <= finalNumber; l++) {
        final BigInteger number = BigInteger.valueOf(l);
        tasks.add(() -> {
            CollatzSequencer sequencer = new CollatzSequencer(new BigInteger(number.toString()));
            synchronized (dataSet) {
                dataSet.put(number, sequencer.init());
            }
            return null;
        });
    }
    service.invokeAll(tasks);
    Thread dataThread = new Thread(() -> {
        while (true) {
            synchronized (dataSet) {
                if (dataSet.size() == finalNumber) {
                    System.err.println("Values: \n");
                    for (CollatzSequencer.FinalSequencerReport data : dataSet.values()) {
                        System.err.println("Entry: " + data.getInitialValue() + ", " + data.getIterations());
                    }
                    System.exit(0);
                }
            }
        }
    }, "Collatz Conjecture Data Set Thread");
    dataThread.start();
}

Collatz Conjecture Algorithm:

/*
 * To change this license header, choose License Headers in Project Properties.
 * To change this template file, choose Tools | Templates
 * and open the template in the editor.
 */
package com.collatzsequencer.core;

import java.math.BigInteger;

/**
 * A sequencer used for computing the collatz sequence.
 *
 * @author Sarah Szabo
 * @version 1.0
 */
public class CollatzSequencer {

private final BigInteger initialValue;

public CollatzSequencer(BigInteger currentValue) {
    if (currentValue == null) {
        throw new NullPointerException("Value passed can't be null");
    } else if (currentValue.compareTo(new BigInteger("1")) < 0) {
        throw new NumberFormatException("The value passed to the constructor must be a natural number.");
    }
    this.initialValue = currentValue;
}

public FinalSequencerReport init() {
    return new FinalSequencerReport(performOperation(new SequencerReport(this.initialValue)), this.initialValue);
}

private SequencerReport performOperation(SequencerReport report) {
    if (report.getResult().equals(new BigInteger("1"))) {
        return new SequencerReport(report.getResult(), report.getIterations(), report.getSequence().length() > 1
                ? report.getSequence().substring(0, report.getSequence().length() - 3) : "The sequence starts and ends at 1 <Nothing Done>");
    } else if (report.getResult().mod(new BigInteger("2")).equals(new BigInteger("0"))) {
        BigInteger value = report.getResult().divide(new BigInteger("2"));
        return performOperation(new SequencerReport(value, report.getIterations().add(new BigInteger("1")),
                report.getSequence() + " " + report.getResult() + "/2 -> " + value + " ->"));
    } else {
        BigInteger value = report.getResult().multiply(new BigInteger("3")).add(new BigInteger("1"));
        return performOperation(new SequencerReport(value, report.getIterations()
                .add(new BigInteger("1")), report.getSequence() + report.getResult() + " * 3 + 1 ->" + value + " ->"));
    }
}

public static final class FinalSequencerReport extends SequencerReport {

    private final BigInteger initialValue;
    private final String finalFormattedString;

    public FinalSequencerReport(SequencerReport finalReport, BigInteger initialValue) {
        super(finalReport.getResult(), finalReport.getIterations(), finalReport.getSequence());
        this.initialValue = initialValue;
        this.finalFormattedString = "Initial Value: "
                + getInitialValue() + "\nFinal Value: " + getResult() + "\nIterations:  "
                + getIterations() + "\nAlgebraic Sequence:\n" + getSequence();
    }

    public String getFinalFormattedString() {
        return finalFormattedString;
    }

    public BigInteger getInitialValue() {
        return initialValue;
    }
}

public static class SequencerReport {

    private final BigInteger result, iterations;
    private final String sequence;

    public SequencerReport(BigInteger result) {
        this(result, new BigInteger("0"), "");
    }

    public SequencerReport(BigInteger result, BigInteger iterations, String sequence) {
        this.result = result;
        this.iterations = iterations;
        this.sequence = sequence;
    }

    public BigInteger getResult() {
        return this.result;
    }

    public BigInteger getIterations() {
        return this.iterations;
    }

    public String getSequence() {
        return this.sequence;
    }

  }
}
Robert Harvey
  • 178,213
  • 47
  • 333
  • 501
Sarah Szabo
  • 10,345
  • 9
  • 37
  • 60
  • 1
    Do you need a `BigInteger` here, or will a `long` suffice? Max long is `2^63-1`, or `9.22 x 10^18` – durron597 Oct 17 '14 at 18:22
  • I don't see your ExecutorService nor your tasks being used anywhere, is there a part of the code you forgot to show? – Lolo Oct 17 '14 at 18:23

1 Answers1

1

As you said, your code works; the problem is probably just performance. Some things I would try:

  1. Use long instead of BigInteger. BigInteger is very slow.
  2. Instead of mod 2 (or % 2), use & 1. The binary AND will have effectively the same result and is much faster.
  3. You are doing way, way too much String manipulation. Override sequencerReport.toString() and have it do the toString calls all at the end when you're printing the data.
  4. Don't do new ThreadFactory(). Use Guava's ThreadFactoryBuilder.
    • You should never call new Thread() ever in your code unless you really know what you're doing, which means don't do it.
  5. Add a wait/notify mechanism for dataThread instead of a busy loop. Call dataSet.notify() when the work is done and dataSet.wait() inside the dataThread body.
durron597
  • 31,968
  • 17
  • 99
  • 158
  • I would add another performance tip. dataThread is doing a busy loop that consumes too much CPU while waiting for the termination condition. A wait/notify mechanism will probably be better. – Eyal Schneider Oct 17 '14 at 18:32
  • @EyalSchneider Thanks, missed that one – durron597 Oct 17 '14 at 18:34
  • I would be very disappointed if you told me that the Java compiler does not replace `a%2` with `a&1`. GCC has been doing that optimization since before some of y'all were born. – Solomon Slow Oct 17 '14 at 20:08
  • @jameslarge It might, but if `BigInteger` actually is necessary here, there must be a preferable way than `mod`. Unless `BigInteger` does that optimization too. – durron597 Oct 17 '14 at 20:12
  • @jameslarge It can't as it can't know, it's not negative. It uses a more complicated expression accounting for the sign, so it's better to do this trivial microoptimization manually. – maaartinus Oct 19 '14 at 22:33
  • @durron597 There's `BigInteger#shiftRight`. I'd bet there's some optimization for this case, but it goes through many hoops first. – maaartinus Oct 19 '14 at 22:38