0

I'm trying to get the method to complete in less than 50 milliseconds, but I can't seem to figure out how I can increase the overall speed of the method. I'm using an object for the pixel because I need to be able to check for null when I'm compressing the data.

public static Frame getFrame(Dimension d, Robot r, Rectangle s, Resolution rs)
{
    int w = d.width;
    int h = d.height;
    BufferedImage b = r.createScreenCapture(s);
    Pixel[] pixels = new Pixel[w * h];
    for(int i = 0; i < w; i++)
    {
        for(int j = 0; j < h; j++)
        {
            pixels[j * w + i] = new Pixel(b.getRGB(i, j));
        }
    }
    return new Frame(rs, pixels, true);
}

Here's the constructor for the Pixel class

public Pixel(int c)
{
    if((c & 0xFF) == 0xA || (c & 0xFF) == 0xD)
        c++;
    if((c & 0xFF00) == 0xA00 || (c & 0xFF00) == 0xD00)
        c += 0x100;
    if((c & 0xFF0000) == 0xA0000 || (c & 0xFF0000) == 0xD0000)
        c += 0x10000;
    if((c & 0xFF000000) == 0xA000000 || (c & 0xFF000000) == 0xD000000)
        c += 0x1000000;
    color = c;
}

And here's the constructor for the Frame class

public Frame(Resolution res, Pixel[] pix, boolean ignoreCheck)
{
    if(!ignoreCheck)
    {
        if(pix.length < res.getTotalPixels())
            throw new NotEnoughPixelsException(res.getTotalPixels() - pix.length);
        else if(pix.length > res.getTotalPixels())
            throw new TooManyPixelsException(pix.length - res.getTotalPixels());
    }
    resolution = res;
    pixels = pix;
}
EnderShadow
  • 105
  • 8
  • 6
    This question appears to be off-topic as it is about a code review - http://codereview.stackexchange.com/ – dsgriffin Dec 19 '13 at 23:11
  • 1
    In my experience the best way to fix performance problems is to get a profiler and see where the JVM is really spending its time. Everything else is guess work in that you make a guess, do a lot of work, and then find out your guess was wrong. Ugh. The profiler is your friend. – Bob Kuhar Dec 19 '13 at 23:21
  • I don't agree that it's off topic. There is a specific question "how do I get the performance of this improved" and the answers show that a concrete response can be given. It's not a general "how can this code be improved". I agree it's pushing the boundaries a bit though. :) – Tim B Dec 19 '13 at 23:29
  • I second the profiler comment. I'll also throw out that you might not want to use a sampler for this -- use an instrumented profiler, because it will help you get the finer-grained insight you'll probably need. Samplers have noise ([especially in Java](http://pl.cs.colorado.edu/papers/profilers-pldi10.html)) that will probably hurt you, and this small amount of code should be fine to instrument. – yshavit Dec 19 '13 at 23:37
  • Check out [VisualVM](http://visualvm.java.net/) for an easy solution to Java profiling. If you're using JDK 1.7 you most likely already have the binary. – x4nd3r Dec 19 '13 at 23:45
  • There's an easy way to see exactly where the problem is and why: [*just pause it a few times*](http://stackoverflow.com/a/378024/23771). You will see what you already know: that `new Pixel` is taking essentially all the time. That's a big price to pay for maybe being able to detect nulls. Good news: fix it and you'll get a *whopping* speedup. – Mike Dunlavey Dec 20 '13 at 02:13

1 Answers1

4

Don't use a class for Pixel. You will be constructing hundreds of thousands, if not millions of them.

slipperyseal
  • 2,728
  • 1
  • 13
  • 15
  • I'm using pixels so that I can check for null at later points in the code. I do this so that I can compress the save file. – EnderShadow Dec 19 '13 at 23:20
  • 1
    The way your code is written, you will never have any entries in the pixels array that are null (or do you null them out later). If you are in search of naked inefficiencies, that is one right there (creating something just to null it out later). Get a profile, find your hot spots. I'm with the Seal guy, the value of the Pixel class is suspect and construction ain't free. – Bob Kuhar Dec 19 '13 at 23:24
  • 2
    in that case use something like an array if primitive ints and a primative array of bool. set the values of the bool to true when a pixel is set. OR put Alpha values in the 32 bit ints, just like regular ARBG does. Normally we say "don't optimise" but in the case of imaging, a class for Pixel is total overkill. If you need your code to be faster than it is, you are going to have to do this. – slipperyseal Dec 19 '13 at 23:25
  • This code never generates any null pixels. Just what the heck does a "null" pixel mean? If you mean "transparent", that's what alpha is for. And @SlipperySeal presents some alternatives (including alpha) if you really need this "null" idea. – user949300 Dec 19 '13 at 23:25
  • (If you stick with the Pixel class, you will probably also run into memory problems) – slipperyseal Dec 19 '13 at 23:27
  • 1
    Just a small amendment to @SlipperySeal, a `BitSet` could also work instead of `boolean[]`. It requires a bit more computation per access, but might be more compact (leading to better CPU caching, etc). Worth experimenting with. – yshavit Dec 19 '13 at 23:43