-2

This piece of code has a performance problem and I asked to fix it. doTransaction() and printTransactions() methods can not be modified.

I tried to do some changes in processTransactions() but it wasn't successful. any suggestion would be appropriated.

import java.util.EnumMap;
import java.util.HashMap;
import java.util.Map;
public class TransactionProcessor {
    /** The number of transactions */
    private static final int NUM_TRANSACTIONS = 1000;
    /** The status of a transaction */
    private static enum Status {
        RUNNING,
        OK,
        FAILURE
    }
    /** The status of transactions */
    private HashMap <Integer, Status> transactionStatus = new HashMap<>();
    /** 
    * Perform the complex transaction. This method must be called by the     exercise and cannot be changed 
    * @param input the input of the transaction 
    * @return the output of the transaction 
    */
    final protected double doTransaction(double input) throws InterruptedException {
        // --- You cannot modify this method --- 
        Thread.sleep(10000);
        return input * 100;
    }
    /** 
    * Print the number of transactions. This method must be called by the   exercise and cannot be changed 
    * @param transactions an object describing the transaction status 
    */
    final protected void printTransactions(Map < ?, Status > transactions) {
        // --- You cannot modify this method --- 
        EnumMap < Status,
        Integer > counts = new EnumMap<>(Status.class);
        for (Status s: Status.values()) {
            counts.put(s, 0);
        }
        for (Status s: transactions.values()) {
            counts.put(s, counts.get(s) + 1);
        }
        System.out.printf("- %d Ok transactions, %d Running transactions, " + "%d Failed transactions. Completed percentage: %s%%\n", counts.get(Status.OK), counts.get(Status.RUNNING), counts.get(Status.FAILURE), (counts.get(Status.OK) + counts.get(Status.FAILURE)) * 100.0 / NUM_TRANSACTIONS);
    }
    /** 
    * Process all transactions 
    * @return the output of all transactions 
    */
    public double processTransactions() {
        double result = 0.0;
        for (int i = 0; i < NUM_TRANSACTIONS; i++) {
            try {
                transactionStatus.put(i, Status.RUNNING);
                result += doTransaction(i);
                transactionStatus.put(i, Status.OK);
                printTransactions(transactionStatus);
            } catch(InterruptedException ex) {
                System.out.println("Transaction failed");
                transactionStatus.put(i, Status.FAILURE);
            }
        }
        return result;
    }
    /** 
    * Main method. Display the result and execution time. 
    * @param args (not used) 
    */
    public static void main(String[] args) {
        long startTime = System.currentTimeMillis();
        TransactionProcessor tp = new TransactionProcessor();
        double result = tp.processTransactions();
        System.out.printf("The result is: %f . " + "Elapsed time: %s seconds\n", result, (System.currentTimeMillis() - startTime) / 1000.0);
    }
}
Tom
  • 16,842
  • 17
  • 45
  • 54
Feedib Com
  • 33
  • 1
  • 1
  • 5
  • 6
    I don't see what performance problem you could be observing that would not be absolutely *swamped* by the ten-second sleep in `doTransaction()`. To make the elapsed time smaller, invoke `doTransaction()` fewer times. – John Bollinger Mar 09 '17 at 17:17
  • If the 10-second sleep is even remotely representative of the real process then nothing `processTransactions()` currently does moves the performance needle. Not that `processTransactions()` appears to be doing anything inherently inefficient in itself anyway. – John Bollinger Mar 09 '17 at 17:25
  • Possible duplicate of [Java HashMap performance optimization / alternative](http://stackoverflow.com/questions/1757363/java-hashmap-performance-optimization-alternative) – Feedib Com Mar 09 '17 at 20:39
  • (1) Not at all a dupe of the suggested target. (2) it is inappropriate to so drastically modify the question after receiving answers (rolled back). (3) None of the answers having yet been upvoted, you should be able to *delete* the question if you wish to do so. – John Bollinger Mar 09 '17 at 20:42

2 Answers2

1

If, as you say, the doTransaction() implementation you presented must be called 1000 times, and given that each call takes a minimum of 10 seconds, the total time consumed is at least 10000 seconds, which is nearly three hours. The only way to substantially reduce the time consumed would be to perform many doTransaction() calls in parallel. To get 10000 seconds down to under 1200 seconds, you'll need at least 9-fold concurrency. If that's the approach you're expected to take, then the structure of the original processTransactions() method suggests 10-fold concurrency.

With that said, this is an altogether unrealistic model. In the real world, you cannot expect linear speedup from parallelization, and it is anyway rarely the case that you can parallelize freely, without regard to the details of the workload. It will work here because doTransaction() does not actually perform any work, but I don't see what useful lesson you're actually supposed to take away.

In any event, do not expect me to offer any actual code. I am -- and we are -- happy to give you occasional help and guidance on your homework, but we will not do it for you.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
0

for loop is the biggest thing i see, but you need it; did you try a for each loop instead?, maybe try using AddExact (if you can use an integer instead of double) method where your adding result. If you have a very big number of transaction, try moving them in the for loop in groups, and make sure you are testing the program from cmd instead of the ide

BlooB
  • 955
  • 10
  • 23