0

This is for learning purposes.

Imagine I want to calculcate prime numbers and use a ThreadPoolExecutor to do so. Below you can see my current implementation, which is kind of silly. My structure:
I generate numbers in a certain range.
For each generated number, create a task to check whether the given number is a prime.
If it is a prime, the result of the operation is the number, else it is null.
A collector goes through the resultlist and checks if there is a number or null. In case it is a number, write that number down to a certain file (here: sorted by amount of digits)

What I would like to do instead: If the number to be checked in the task is not a prime, delete my future from the list/cancel it. As far as I know, only the Executor itself can cancel a Future. What I want is the task itself to say "Hey, I know my result is no use to you, so please ignore me while iterating throught the list". I do not know how to do so.

What I do right now (relevant part):

 final List<Future<Long>> resultList = new ArrayList<>();
        final BlockingQueue<Runnable> workingQueue = new ArrayBlockingQueue<>(CAPACITY);
        final ExecutorService exec = new ThreadPoolExecutor(
                Runtime.getRuntime().availableProcessors() - 2,
                Runtime.getRuntime().availableProcessors() - 1,
                5, TimeUnit.SECONDS,
                workingQueue,
                new ThreadPoolExecutor.CallerRunsPolicy()
        );


        for (long i = GENERATEFROM; i <= GENERATETO; i++) {
            Future<Long> result = exec.submit(new Worker(i));
            resultList.add(result);
        }
        Collector collector = new Collector(resultList,GENERATETO);
        collector.start();
        exec.shutdown();

A Worker is there to execute one task(is it a prime number?)

public class Worker implements Callable<Long> {

    private long number;

    public Worker(long number) {
        this.number = number;
    }


    //checks whether an int is prime or not.
    boolean isPrime(long n) {
        //check if n is a multiple of 2
        if (n % 2 == 0) return false;
        //if not, then just check the odds
        for (long i = 3; i * i <= n; i += 2) {
            if (n % i == 0)
                return false;
        }
        return true;
    }


    @Override
    public Long call() throws Exception {

        if (isPrime(number)) {
            return number;
        }
        return null;


    }
}

And, for the sake of completence, my collector:

public class Collector {

    private List<Future<Long>> primeNumbers;
    private long maxNumberGenerated;
    private HashMap<Integer, PrintWriter> digitMap;
    private final long maxWaitTime;
    private final TimeUnit timeUnit;


    public Collector(List<Future<Long>> primeNumbers, long maxNumberGenerated) {
        this.primeNumbers = primeNumbers;
        this.maxNumberGenerated = maxNumberGenerated;
        this.digitMap = new HashMap<>();
        this.maxWaitTime = 1000;
        this.timeUnit = TimeUnit.MILLISECONDS;
    }


    public void start() {

        try {
            //create Files
            int filesToCreate = getDigits(maxNumberGenerated);
            for (int i = 1; i <= filesToCreate; i++) {
                File f = new File(System.getProperty("user.dir") + "/src/solutionWithExecutor/PrimeNumsWith_" + i +
                        "_Digits.txt");
                PrintWriter pw = new PrintWriter(f, "UTF-8");
                digitMap.put(i, pw);
            }


            for (Future<Long> future : primeNumbers) {
                Object possibleNumber = future.get();
                if (possibleNumber != null) {
                    long numberToTest = (long) possibleNumber;
                    int numOfDigits = getDigits(numberToTest);
                    PrintWriter correspondingFileWriter = digitMap.get(numOfDigits);
                    correspondingFileWriter.println(possibleNumber.toString());
                    correspondingFileWriter.flush();

                }
            }
            for (PrintWriter fw : digitMap.values()) {
                fw.close();
            }


        } catch (InterruptedException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        } catch (ExecutionException e) {
            e.printStackTrace();
        }
    }

    private int getDigits(long maxNumberGenerated) {

        return String.valueOf(maxNumberGenerated).length();
    }


}
InDaPond
  • 574
  • 3
  • 6
  • 23
  • 2
    Instead of a `List>`, use a `Map>`. When you get a `Future` form a submitted task, add it to the map with the `i` it was created with. Then, when the task notices that it's `i` is no prime, remove it from the `Map`. Questin is, though: why do you even need to remove these `Future`s? Sounds like a task you could handle via `list.stream().filter(f -> f.get() != null).collect(Collectors.asList());` – Turing85 Jun 20 '18 at 23:02
  • FYI: `future.get()` returns a Long so you don't need to do the casting to `possibleNumber`. You are doing `possibleNumber.toString()` twice, once inside `getDigits(...)` and once directly. Careful of variables like `correspondingFileWriter` or `fw` that aren't a `FileWriter`. Also I'd create your `PrintWriter`s on demand so get them from the map, if they are null, create the `PrintWriter` and `put(...)` it into the map. Your loop above may leave `PrintWriter`s unclosed because they aren't in the `digitalMap`. – Gray Jun 22 '18 at 14:12
  • yes, thank you for that in put. this was my first concept and not refactored yet - most of what u said has already been done. Only thing I did not do is the PrintWriter creating on demand, I thought its nicer to do the creation all beforehand, for readability and for being faster during the actual operation – InDaPond Jun 23 '18 at 16:32
  • @Turing85 I checked your profile and you seem to know a lot about low level execution. Is it any faster to the the stream with != null instead of asking whether the number is null? I thought it is the same in the sense of checking whethere there is an actual address (or a value at that adress, dont know how null works) or maybe its even slower because it has to wait until all the futures are done – InDaPond Jun 23 '18 at 16:34
  • @InDaPond Let us discuss this in [chat](https://chat.stackoverflow.com/rooms/173681/room-for-turing85-and-indapond). – Turing85 Jun 23 '18 at 16:46

1 Answers1

1

What I would like to do instead: If the number to be checked in the task is not a prime, delete my future from the list/cancel it. As far as I know, only the Executor itself can cancel a Future.

To me this seems like an unnecessary optimization. The Future is there so that the task can return a value. Once the task figures out it's not a prime and returns null the "cost" to the program associated with the Future is negligible. There is nothing to "cancel". That task has completed and all that is left is the memory that allows the Future to pass back the null or the prime Long.

Since we are talking about learning, in many situations programmers worry too quickly about performance and we often spend time optimizing parts of our application which really aren't the problem. If I saw using some JVM monitor (maybe jconsole) that the application was running out of memory then I maybe would worry about the list of Futuress but otherwise I'd write clean and easily maintained code.

If you really are worried about the Future then don't save them in a list at all and just share a BlockingQueue<Long> between the prime checking tasks and the main thread. The prime checking jobs would add(...) to the queue and the main thread would take(). You should consider putting nulls in the list because otherwise you wouldn't know if the prime tasks were done unless you counted the results. You'd want to check X random numbers and then you'll know that it was done when X results (null or numbers) get taken from the BlockingQueue<Long>.

Hope this helps.

Gray
  • 115,027
  • 24
  • 293
  • 354