CODEWITHSUNDEEP
javaandroidperformancefor-loopheap-memory
Woyzeck
Woyzeck

Reputation: 115

for-loop very slow on Android device

I just ran into an issue while trying to write an bitmap-manipulating algo for an android device.

I have a 1680x128 pixel Bitmap and need to apply a filter on it. But this very simple code-piece actually took almost 15-20 seconds to run on my Android device (xperia ray with a 1Ghz processor).

So I tried to find the bottleneck and reduced as many code lines as possible and ended up with the loop itself, which took almost the same time to run.

for (int j = 0; j < 128; j++) {
    for (int i = 0; i < 1680; i++) {
         Double test = Math.random();
    }
}

Is it normal for such a device taking so much time in a simple for-loop with no difficult operations?

I'm very new to programming on mobile devices so please excuse if this question may be stupid.

UPDATE: Got it faster now with some simpler operations.

But back to my main problem:

public static void filterImage(Bitmap img, FilterStrategy filter) {
    img.prepareToDraw();
    int height = img.getHeight();
    int width = img.getWidth();
            RGB rgb;
    for (int j = 0; j < height; j++) {
        for (int i = 0; i < width; i++) {
            rgb = new RGB(img.getPixel(i, j));
            if (filter.isBlack(rgb)) {
                img.setPixel(i, j, 0);
            } else
                img.setPixel(i, j, 0xffffffff);
        }
    }
    return;
}

The code above is what I really need to run faster on the device. (nearly immediate) Do you see any optimizing potential in it?

RGB is only a class that calculates the red, green and blue value and the filter simply returns true if all three color parts are below 100 or any othe specified value. Already the loop around img.getPixel(i,j) or setPixel takes 20 or more seconds. Is this such an expensive operation?

Upvotes: 3

Views: 2806

Answers (4)

pgsandstrom
pgsandstrom

Reputation: 14399

First of all Stephen C makes a good argument: Try to avoid creating a bunch of RGB-objects.

Second of all, you can make a huge improvement by replacing your relatively expensive calls to getPixel with a single call to getPixels

I made some quick testing and managed to cut to runtime to about 10%. Try it out. This was the code I used:

int[] pixels = new int[height * width];
img.getPixels(pixels, 0, width, 0, 0, width, height);

for(int pixel:pixels) {
    // check the pixel
}

Upvotes: 2

Stephen C
Stephen C

Reputation: 718788

RGB is only a class that calculates the red, green and blue value and the filter simply returns true if all three color parts are below 100 or any othe specified value.

One problem is that you are creating height * width instances of the RGB class, simply to test whether a single pizel is black. Replace that method with a static method call that takes the pixel to be tested as an argument.

More generally, if you don't know why some piece of code is slow ... profile it. In this case, the profiler would tell you that a significant amount of time is spent in the RGB constructor. And the memory profiler would tell you that large numbers of RGB objects are being created and garbage collected.

Upvotes: 0

Ian Warwick
Ian Warwick

Reputation: 4784

There is a disclaimer in the docs below for random that might be affecting performance, try creating an instance yourself rather than using the static version, I have highlighted the performance disclaimer in bold:

Returns a pseudo-random double n, where n >= 0.0 && n < 1.0. This method reuses a single instance of Random. This method is thread-safe because access to the Random is synchronized, but this harms scalability. Applications may find a performance benefit from allocating a Random for each of their threads.

Try creating your own random as a static field of your class to avoid synchronized access:

private static Random random = new Random();

Then use it as follows:

double r = random.nextDouble();

also consider using float (random.nextFloat()) if you do not need double precision.

Upvotes: 1

ngesh
ngesh

Reputation: 13501

It may be because too many Objects of type Double being created.. thus it increase heap size and device starts freezing..

A way around is

double[] arr = new double[128]
for (int j = 0; j < 128; j++) {
    for (int i = 0; i < 1680; i++) {
         arr[i]  = Math.random();
    }
}

Upvotes: 3

Related Questions