GMG
GMG

Reputation: 1618

Memory leak with an array

I have a very strange memory problem with my Android app. My app use 3 classes that are the following:

public class RGB 
{
    public int R;
    public int G;
    public int B;
}

public class CMYK 
{
    public int C;
    public int M;
    public int Y;
    public int K;
}

public class COLOR 
{
    public String id;
    public CMYK cmyk = new CMYK();
    public RGB rgb = new RGB();

    public COLOR(String id, int c, int m, int y, int k, int r, int g, int b)
    {
        this.id = id;

        this.cmyk.C = c;
        this.cmyk.M = m;
        this.cmyk.Y = y;
        this.cmyk.K = k;

        this.rgb.R = r;
        this.rgb.G = g;
        this.rgb.B = b;
    }
}

then somewere in the code I have to load 2000 colors from a file (file is about 65K lenght and has exactly 2000 records) and is placed in assets folder

public COLOR[] color_list = new COLOR[2000];
...
...
do
{
    s = reader.readLine();
    if (s != null)
    {
        String[] x = s.split(" ");
        COLOR c = new COLOR(x[0], Integer.parseInt(x[1]), Integer.parseInt(x[2]), Integer.parseInt(x[3]), Integer.parseInt(x[4]), Integer.parseInt(x[5]), Integer.parseInt(x[6]), Integer.parseInt(x[7]));
        color_list[j++] = c;
    }

} while (s != null);

after this the app will crash and stop working. If I remove the do..while all is working, so I think my array will be more and more and more then 65K, what I have done wrong? On Android LogCat I have reached the full of HEAP space (26MB) !!!

Best regards GMG

Upvotes: 1

Views: 1007

Answers (3)

Raffaele
Raffaele

Reputation: 20885

I don't think that code is responsible of the OutOfMemoryException. Maybe there are other fields you don't mention, but without running the code one can't tell.

However there may be a small leak when you create the ID. Whenever you create a String from an existing one (either substring()-based or methods in the regex package), the returned string keeps an internal reference to the old one: it's just a thin wrapper around the old sequence of characters, just with different start and different length. This means that you'd better create your ID like this

String id = new String(x[0]);

This way you don't keep the whole line in memory just to store a few characters.

However, this is an optimization, since you state that your file is 65KB, so even if you retain it all in-memory, it wouldn't crash your application. Post the whole code so we can run and analyze it.

BTW, you can save an indentation level this way:

String line;
Pattern pattern = Pattern.compile(" "); // Help the GC ;)

while ((line = in.readLine()) != null) {
    String[] data = pattern.split(line);

    // Ugly, but still better than a 8-args constructor
    RGB rgb = new RGB(data, 1, 3);
    CMYK cmyk = new CMYK(data, 4, 4);

    // the best would be a constructor like Color(String[8])
    colors[j++] = new Color(new String(data[0]), rgb, cmyk);
}

I also changed the API a bit (I found this more comfortable)

Upvotes: 2

wolfcastle
wolfcastle

Reputation: 5930

It's hard to tell without the error, but I assume you're getting an IndexOutofBoundExceptions or something. You initialize the array to 2000 elements, but keep reading your file until you reach the end. What if there are 2001 entries in it? Then you'll blow past the end. Or what if there are only 100? Then you've wasted a ton of space.

Like Ralgha said, use an ArrayList, not an array.

For your file parsing, you might want to consider the Scanner class.

Upvotes: 1

Khantahr
Khantahr

Reputation: 8528

Try using an ArrayList instead of a basic array, it makes memory management much easier.

Upvotes: 2

Related Questions