0

When I change the String m1 to a 2D int array it runs super fast but now it takes over an hour to just loop 10 pictures also each picture takes almost double the time the first one took . Is there a way where I can improve my code so it runs faster as I need to save all values as one String in the end ?

import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.ObjectOutput;
import java.io.*;
import java.util.*;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;
import javax.imageio.ImageIO;
import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.*;
import java.awt.Color;
import java.awt.image.BufferedImage;

public class Convertt {
    static String[][] allImages = new String[304][76808];
    static String m1 = "";
    static int f = 0;

    public static void rgb1(Path path) throws IOException {
        File file = new File(String.valueOf(path));
        BufferedImage img = ImageIO.read(file);
        for (int y = 0; y < img.getHeight(); y++) {
            for (int x = 0; x < img.getWidth(); x++) {
                int pixel = img.getRGB(x, y);
                Color color = new Color(pixel, true);
                int red = color.getRed();
                int green = color.getGreen();
                int blue = color.getBlue();
                if (m1 == "") {
                    m1 = (red + ":" + green + ":" + blue);
                    System.out.println(m1);
                } else {
                    m1 = m1 + ":" + red + ":" + green + ":" + blue;
                }
            }
        }
        f++;
        System.out.println("Done with" + f);
    }

    public static void main(String[] args) throws IOException {
        Path imgFolder = Paths.get("D:\\swim\\Frames1");
        int k = 1;
        for (int i = 0; i < 273; i++) {
            rgb1(imgFolder.resolve("frames" + k + ".png"));
            k++;
        }
        System.out.println("done");

        FileWriter writer = new FileWriter("D:\\swim\\Output\\new1.csv");
        writer.append(m1);
    }
}
Community
  • 1
  • 1
Kok129
  • 273
  • 2
  • 8
  • If you mean you need to store it as one string in the end so you can add to the file writer, you could append different parts (within the loop) to the file writer instead. – BeUndead Jun 06 '21 at 17:56
  • 1
    @BeUndead Thanks for your comment ! But I don't really get what you asked. My problem is that this code would take an hour for 10 images and I want to run it on 272 images so how can I run it faster ? – Kok129 Jun 06 '21 at 18:01
  • When you say ‘I need to save all values as 1 string in the end’, in the code, your reason for doing this is to do `writer.append(m1);`. Instead of that, you could make the `writer` _before_ the loop, and `append` to that, rather than adding to m1. – BeUndead Jun 06 '21 at 18:20
  • 1
    You seem to have accidentally replaced your question with nonsense. I undid that for you, assuming that you did not intentionally vanadalise it. – Yunnosch Jun 06 '21 at 18:27

1 Answers1

2

Your understanding of string performance may be the issue.

Strings are immuttable, so every change to a string makes a new String, it doesn't alter the existing string.

if (m1=="") {
    m1=(red+":"+green+":"+blue);
    System.out.println(m1);
} else {
    m1=m1+":"+red+":"+green+":"+blue;
}

doesn't create one string. It creates a string for the red value, then creates a string that containts that in addition to a ":", and then creates a string that contains that in addition to the green value, and then creates a string that contains that in addition to the .... (and so on)

So instead use a StringBuilder, which is like a buffer of RAM that holds String contents which can be manipulated without creating new Strings. When you want the String value for the buffer, call .toString() to create a new String.

Edwin Buck
  • 69,361
  • 7
  • 100
  • 138
  • Doesn't the compiler recognize such patterns and use StringBuilder in the resulting bytecode then? I thought i have read something like that some years ago. – dunni Jun 06 '21 at 18:27
  • 2
    @dunni In 1.8 [yes](https://stackoverflow.com/q/46512888/2541560), but it won't span loops, which causes the performance degradation here as x * y `StringBuilder` objects are created. – Kayaman Jun 06 '21 at 18:45
  • I still don't understand why we need the whole `String` in a `StringBuilder` at all. The only reason it's done as such is to write to a file (as one `String` at the end). Surely just periodically writing to the file instead of eventually having to call `builder.toString()` here and having the whole `String` built in memory would be more memory friendly (if not performant)? – BeUndead Jun 07 '21 at 16:02
  • @BeUndead If it's writing to a file, then just do the writes; but, use a Buffered file writer, because the unbuffered ones will do too many little writes instead of chunking them together in nice block-sized writes. – Edwin Buck Jun 07 '21 at 20:58
  • 1
    @dunni yes and no, the expression `red+":"+green+":"+blue` will be compiled to code creating a single `String`. Prior to Java 9, it was using a `StringBuilder` under the hood. Since Java 9, it uses an `invokedynamic` instruction letting the runtime generate specialized code. The real problem is the `m1=m1+…` statement that uses an existing string from the previous iteration to construct a new string in each iteration, regardless of which construct is used to construct it. – Holger Jun 09 '21 at 12:02
  • There's also a small problem at the beginning, where `if (m1 == "")` exists. That statement likely "just happens" to be true because the constant `""` is in the same complilation unit; but, in reality it should be `if (m1.equals(""))`, or `if (m1.isEmpty())` The core error is using objects like primitives, and in Java, that doesn't work the way one might think it should. Personally, I'd love a Java-like language where primitives didn't exist (Scala comes close); but, in Java, you can't reuse operations that apply to primitives to Objects, even if String breaks a few of those rules with `+`. – Edwin Buck Jun 09 '21 at 17:28