beryx / text-io

A library for creating interactive console applications in Java
http://text-io.beryx.org/
Apache License 2.0
343 stars 45 forks source link

Fixed SwingTextTerminal memory leak #13

Closed mattco98 closed 6 years ago

mattco98 commented 6 years ago

When a SwingTextTerminal calls getStyle(StyleData), it always adds a new style to it's Document. After a couple thousand lines of printed text, the unnecessary references start to cause issues. This change modifies the style name to be a hashcode of the style, and only adds a new style to its document when document.getStyle(<hash>) returns null.

Test code (which, in a perfect world, takes 20 seconds to run):

import org.beryx.textio.TextIO;
import org.beryx.textio.TextIoFactory;
import org.beryx.textio.swing.SwingTextTerminal;

import java.awt.*;
import java.util.concurrent.atomic.AtomicInteger;

public class Main {
    public static void main(String[] args) throws Exception {
        TextIO io = TextIoFactory.getTextIO();
        SwingTextTerminal terminal = (SwingTextTerminal) io.getTextTerminal();
        terminal.display();
        long initialTime = System.nanoTime();

        for (int i = 0; i < 100; i++) {
            EventQueue.invokeAndWait(() -> spam(terminal));

            // Delay so the text actually appears in the terminal
            Thread.sleep(200);

            terminal.resetToOffset(0);
        }

        double timeInSeconds = (System.nanoTime() - initialTime) / 1000000000.0;
        System.out.println("Elapsed time: " + timeInSeconds);
    }

    private static void spam(SwingTextTerminal t) {
        for (int i = 0; i < 100; i++) {
            t.println("This text is long enough to fill up the entire line.");
        }
    }
}

Output before PR: Elapsed time: 141.755707654

Output after PR: Elapsed time: 20.540818568

Note: I'm not quite sure if this change breaks anything else, as I don't have the most knowledge of Swing. I was experiencing trouble in an application I'm developing with this library and used YourKit to analyze the program, and it led me straight to the getStyle() method. As soon as I implemented this change, the problem went away, and as far as I could tell, there weren't any side effects. Let me know if there's anything else I need to modify to improve this PR.

siordache commented 6 years ago

Thanks, Matthew! I was not aware of this memory leak.

siordache commented 6 years ago

@mattco98: Text-IO 3.1.4 includes your fix and is now available in Maven Central and JCenter.

I replaced hashCode() with a new method called getStyleName(), because there is a very small (but non-zero) probability that two different styles produce the same hashCode.

mattco98 commented 6 years ago

Makes sense, I didn't consider that. Certainly a more readable identifier, at the very least. Glad I could help!