# Streamlining a method to run faster

I did some guesswork tweaking and let the heap reach a maximum of ~450 megabytes (was originally using invokeAll). The program just finished in 6.3 minutes. I've been advised to use ctrl + break to obtain a thread dump, but I am not experienced enough to use these results effectively yet.

Finally, running the profiler again: ListTask.call(), ArrayList.get(int) and reduct2 all have total time between 100-200 seconds with call() being the highest midway through execution. However, two other methods are significantly higher (each of these has a total time of about 1200 seconds, approaching 2000 and higher).

1. sun.rmi.transport.tcp (ConnectionHandler)

2. ThreadPoolExecutor$Worker.run() / ThreadPoolExecutor$Worker.runWorker

3. GetTask, poll, transfer stack operations, and parkNanos lockSupport are all slightly higher than ListTask.call() as well. Is my multithreading successful?

I have a Java method that is run 2N times (or ~4.3 billion times in my case). I am specifically asking about this method because individual tests and profiling through JVisualVM suggest it is slowing my program's performance.

My largest time sink is from ArrayList.get(int). My second largest time sink is from the reduct2/ListTask.call() methods. Reduct2 is designed to take an array (called row) of length N + M + 2 (length 88 in this case) and iterate over a list of lists to determine which columns are 'scheduled for deletion' by the presence of a 2. It also does some 'book checking' for lack of a better term.

I am running this method with executor service, multi-threaded across 4 processors. Disabling the reduct2 method allows the program is finish in 5 minutes across 4.3 billion iterations of rows. Enabling reduct2 with ExecutorService causes my program to take 1 hour for 4.3 billion iterations.

A new ListTask object is defined every 728 iterations of row, so I define ~5.9 million ListTask objects. I have 9 Daemon threads/14 Live threads and 94% of my memory usage comes from int[] objects; Average heap is about 35-45 megabytes with a maximum of 112 megabytes. I am using 100% Metaspace at 8.9 megabytes.

ListTask.call() method:

class ListTask implements Callable<Void> {
private List<List<Integer>> matTan;
private List<int[]> combination;
private List<int[]> jobs;
private boolean gumDrop;
private int minimum;

public Void call() {
for (int[] row : jobs) {
int temp = Multi.reduct2(row, matTan, minimum, gumDrop);
if (temp > ValMax.valMax) { //ValMax.valMax //userinput
ValMax.valMax = row[0]; //userinput
System.out.print(ValMax.valMax + " ");
}
}

return null;
}
}


reduct2 method:

public static int reduct2(int[] row, List<List<Integer>> matCan, int minimum, boolean gumDrop) {
int b = 0, c = 0, d = 0, e = 0, g = 0, high = inverse.length;
final int B = matCan.get(0).size() - 1;
final int A = matCan.size();
for (int x = 0; x < high; x++) {
if (x < A) {
if (row[x + 1] > 0) {
b++;
for (int y = 0; y < B + 1; y++) {
if (matCan.get(x).get(y) == 2 && row[2 + A + y] == 0) {
row[2 + A + y] = 1; // 1s mean that a column was deleted, 0 is kept.
d -= e;
} else if (row[2 + A + y] == 0) {
d++;
}
}
e++;
}
}

if (inverse[x] == 0|| gumDrop == true) {
if (row[x - c + 1] == 1) {
row[x - c + 1] = 1 + c + g;
g++;
} else {
g++;
}
} else {
c++;
}
}

if (d / b < minimum && gumDrop == true) {
row[0] = 0;
d = 0;
} else {
row[0] = d;
}

return d;
}


How it works:

ListTask is my class for submitting jobs to ExecutorService. I have an int[] array with an index for every 'row' and 'column' in a list of lists I parsed from an actual (or random) data set. I go through every combination of N rows and represent it as a binary number in the int[] array at their respective spots.

A zero means that row is 'scheduled for deletion' while a 1 means it's kept. reduct2 edits this int[] to change every 1 to it's representative row's position in the list of lists (from 1 to N), determines which columns do not have any 2's (and edits these into the int[] array with 0 meaning kept), and records how many values are left after every 'scheduled' row and column is removed (saved as ValMax.valMax).

Based on how I choose to handle ValMax.valMax in ListTask.call(), it affects how these results are saved into a separate list of arrays which are then exported for I/O and other meaningful tasks (not recorded in my run time). All in total, before the analysis I have 447 lines of code minus skipped lines.

## migrated from programmers.stackexchange.comNov 18 '14 at 20:17

This question came from our site for professionals, academics, and students working within the systems development life cycle.

• Sorry, I'm also quite new to Stack Exchange/Overflow. I've run into migration problems a couple times and it's finally dawned on me to check the different forums in detail before posting. But may I ask why this post received a -1? Is it too personal of an issue? – user3834916 Nov 18 '14 at 15:58
• @user3834916 Probably because it's not a good fit for the site. The number of votes on a question is supposed to indicate its overall usefulness, so downvoting off-topic questions helps keep them under the radar so the right questions are easier to find. I'm not sure if migration resets the question to 0, but in practice most questions don't seem to drop below -2 or -3 before they're closed and forgotten or they're moved somewhere appropriate. – Doval Nov 18 '14 at 16:02
• Meanwhile, you appear to be in guesswork mode. Just try this. – Mike Dunlavey Nov 18 '14 at 17:44
• This is definitely a candidate for migration to Code Review, but being beta there is no direct migration path. A moderator would need to move it. @user3834916 don't take the votes personally, the question is not bad it is just on the wrong site. Personally I do not downvote a good question on the wrong site. I vote or flag to migrate, but not everyone feels the way I do. – user31517 Nov 18 '14 at 17:44
• What are ListTask and reduct2 supposed to do? – raptortech97 Nov 18 '14 at 17:53

if (temp > ValMax.valMax) { //ValMax.valMax //userinput
...
ValMax.valMax = row[0]; //userinput


In a multithreaded code, this doesn't look right. Another thread can change ValMax.valMax in the meantime. A proper way would require either a lock, or AtomicInteger.compareAndSet.

Actually, this should be done in ValMax, instead of fooling around with its fields.

public static int reduct2(int[] row, List<List<Integer>> matCan, int minimum, boolean gumDrop) {
int b = 0, c = 0, d = 0, e = 0, g = 0, high = inverse.length;


Is anyone supposed to understand it? With names like b, c, ..., z, the chances don't look well. I gave it up.

row[2 + A + y] = 1; // 1s mean that a column was deleted, 0 is kept.


A zero means that row is 'scheduled for deletion' while a 1 means it's kept.

Do you a favor. Use a properly named boolean (like boolean keep;) or define some constants. Name them properly and you need no comments neither in code nor elsewhere.

I'd split reduct2 into multiple methods. This is nearly always good and could also allow you to find out more about the timing.

What many people say is they can't tell you what the performance problem is without seeing and understanding your code.

I think that's like handing a hungry person a fish, which only solves the immediate problem.

I would rather teach the hungry person to fish, thus solving the long-term problem.

I want you to know how to find the performance problem. This answer shows you how to do it.

In a nut-shell, it is this: You've got some performance problem, and it is causing a big slowdown like (pick a number) 4 times. That means if it takes 60 seconds, 45 of those seconds are spent executing (or waiting for) the performance problem. That means if you randomly pause it under a debugger, you have a 75% chance of catching it in the act and seeing exactly what the problem is. Do this 8 times, and 6 of those times, on average, it will point out to you exactly what the problem is.

That's far more effective than fiddling with profiler measurements or trying to be a good guesser, or asking other people to.