Performance Drag and Drop & Timer

[Rhymen]

Hey Leute,

arbeite momentan an einem 'Spiel' und habe momentan Probleme mit der Performance an den Zielrechnern (Die Zielsystem sind sehr schwach). Bin jetzt schon den ganzen Tag an meinem PC am profilen um Schwachstellen im Programm zu finden. Die meiste Zeit geht an folgenden Teilen verloren:

Zum ersten hab ich nen Timer der nur aktiv ist wenn ein JDialog aufgerufen wird.

.sleep() ist nachträglich eingefügt. Performance ist jetzt wesentlich besser geworden, aber bin trotzdem neugierig was ihr dazu sagt.

    private class TimerThreadRunnable implements Runnable {

        @Override
        public void run() {
            long delta = 0, startTime= System.currentTimeMillis();
            while (delta < timerTime) {
                delta = (System.currentTimeMillis() - startTime) / 1000;
                timeLeft.setText(((timerTime - delta)) + " sec.       ");
                try {
                    Thread.sleep(500);
                } catch (InterruptedException ex) {
                    Logger.getLogger(QuestionFrame.class.getName()).log(Level.SEVERE, null, ex);
                }
            }
            frameComponent.setVisible(false);
        }
    }

Hab dazu mal ein bisschen gegoogelt und dort steht das man das setText() vom Label in nen SwingWorker auslagern soll damit das im EDT-Thread ausgeführt wird. Ist das wirklich sinnvoll jedesmal nen SwingWorker anzulegen um den Text zusetzen? Sollte ich direkt alles, also auch den eigentlichen Timer, in den SwingWorker auslagern? Wie caste ich long performance mäßig am besten? Habt ihr noch andere Vorschläge?

Thread 45ms
JLabel.setText() 17ms
StringBuilder.append() 16ms
StringBuilder.toString() 7ms
StringBuilder.append() 3.5ms

Das zweite Problem liegt in meinem Drag and Drop. Konnte es schon wesentlich verbessern, kann aber erst am Montag gucken ob es reicht für die Zielrechner.

    private class DnDMotionListener extends MouseInputAdapter implements Runnable {

        int innerX, innerY;
        int width, heigth;
        DragComponent dragComponent, stayingComponent;
        boolean enableDrag;
        Thread tick;

        DnDMotionListener() {
        }

        @Override
        public void mousePressed(MouseEvent e) {
            if (e.getButton() == MouseEvent.BUTTON1) {
                innerX = e.getX();
                innerY = e.getY();

                dragComponent = (DragComponent) e.getComponent();

                width = dragComponent.getWidth();
                heigth = dragComponent.getHeight();

                stayingComponent = new DragComponent(dragComponent.getText(), dragComponent.getHorizontalAlignment());
                stayingComponent.setBounds(dragComponent.getX(), dragComponent.getY(), width, heigth);
                stayingComponent.setFont(dragComponent.getFont());
                stayingComponent.setBorder(dragComponent.getBorder());
                dragComponent.getParent().add(stayingComponent);

                dragComponent.setBorder(null);
                dragComponent.setForeground(Color.GRAY);

                for (DropComponent c : dropComponents) {
                    if (dragComponent.getBounds().intersects(c.getBounds())) {
                        c.setEmpty(true);
                    }
                }


                tick = new Thread(this);
                tick.start();
                enableDrag = true;
            }
        }

        @Override
        public void mouseDragged(MouseEvent e) {
            if (enableDrag) {
                dragComponent.setBounds(dragComponent.getX() + e.getX() - innerX, dragComponent.getY() + e.getY() - innerY, width, heigth);
                dragComponent.getParent().repaint();
            }
        }

        @Override
        public void mouseReleased(MouseEvent e) {
            if (e.getButton() == MouseEvent.BUTTON1) {
                enableDrag = false;
                boolean validPosition = true;
                for (DropComponent c : dropComponents) {
                    if (dragComponent.getBounds().intersects(c.getBounds()) && c.isEmpty()) {
                        dragComponent.setBounds(c.getBounds());
                        c.setEmpty(false);
                    }
                }
                for (DragComponent c : dragComponents) {
                    validPosition &= !checkCollisions(c);
                }

                if (validPosition) {
                    dragComponent.setBorder(stayingComponent.getBorder());
                    dragComponent.setForeground(Color.BLACK);
                    stayingComponent.setVisible(false);
                } else {
                    dragComponent.setBorder(stayingComponent.getBorder());
                    dragComponent.setBounds(stayingComponent.getBounds());
                    dragComponent.setForeground(Color.BLACK);
                    stayingComponent.setVisible(false);
                }
            }
        }

        public boolean checkCollisions(DragComponent dndComponent) {
            return !dragComponent.equals(dndComponent) && (dragComponent.getBounds().intersects(dndComponent.getBounds()) || dragComponent.getX() <= 0 || dragComponent.getY() <= 0 || (dragComponent.getX() + width) >= getWidth() || (dragComponent.getY() + heigth) >= getHeight());
        }

        @Override
        public void run() {
            boolean validPosition;
            while (enableDrag) {
                validPosition = true;
                for (DragComponent c : dragComponents) {
                    if (checkCollisions(c)) {
                        validPosition = false;
                    }
                }
                if (!validPosition && enableDrag && dragComponent.getForeground() != Color.RED) {
                    dragComponent.setForeground(Color.RED);
                } else if (validPosition && dragComponent.getForeground() != Color.LIGHT_GRAY) {
                    dragComponent.setForeground(Color.LIGHT_GRAY);
                }
            }
        }
    }

DragComponent extends JLabel und DropComponent extends JComponent. Habe nicht viele Änderungen vorgenommen an den Klassen. Am meisten Zeit geht bei den checkCollision() aufrufen verloren. Weiß aber nicht genau wie ich es anders machen soll.

Das ist das erste Mal das ich auf Performance achten muss. Hab eigentlich keine Ahnung was ich hier mache, geb mir aber Mühe :rtfm: . Hoffe ihr habt ein paar Tipps für mich. Bin natürlich offen für andere Tipps und Tricks sowie no go's auf die ich achten sollte wenn es auf Performance ankommt.

 

[rme]

Hallo,

zu deinem ersten Problem: Man darf in Swing bis auf wenige Ausnahmen ausschließlich aus dem Event-Thread heraus Komponenten verändern. Wenn ich es richtig sehe, führst du oben einen eigenen Thread aus, der die Komponente aktualisiert. Das ist verboten und wird zu Problemen führen. Swing hat eine eigene Timer-Klasse, die das Problem genau für Fälle wie deinen umgeht - du kannst dem Timer deine Runnable übergeben und wünschen, dass die Runnable alle 500ms ausgeführt wird. Dann brauchst du auch die Schleife da drin nicht mehr, sondern nur eine Abbruch-Bedingung, die den Timer wieder beendet.

Beispiele hier: How to Use Swing Timers (The Java™ Tutorials > Creating a GUI With JFC/Swing > Using Other Swing Features)

Falls du noch andere Sachen in Swing-fremden Threads machst und von dort Swing-Komponenten ändern willst - dafür gibt es die Hilfsmethode SwingUtilities.invokeLater, die dafür sorgt, dass eine Runnable im Swing-Thread ausgeführt wird, z.B. so:

        SwingUtilities.invokeLater(new Runnable() {
            @Override
            public void run() {
                label.setText("Endlich bin ich legal");
            }
        });

Das ist nicht nur "sinnvoll", sondern erzwungenermaßen erforderlich, weil Swing nicht Thread-sicher ist.

Zu deinem zweiten Problem kann ich leider nichts sagen, mit Drag'n'Drop habe ich noch nie etwas gemacht. Allerdings sagst du ja, dass die meiste Zeit in der Kollisionsprüfung verwendet wird - und dort benutzt du intersects(). Ich glaube, dass diese Methode allgemein langsam ist, weil sie von beliebigen Polygonen ausgeht. Falls es sich um Rechtecke oder einfache Dinge handelt, evtl. mal selbst probieren?

 

[Rhymen]

Danke schon mal für die Hilfe rme!

Ich habe versucht das Programm nach dem MVC-Pattern zu gestalten.
Wenn ich meine Startklassen in der Main aufrufe mach ich das wie folgt:

final Logic l = new Logic();
final View v = new View();
SwingUtilities.invokeLater(v);
 new Controller(v, l);

Im Controller befinden sich alle AktionListener etc. Wenn ich also zB nen Button drücke wird der ActionListener im Controller aufgerufen und führt dann Methoden aus der Logic aus um zB was aus ner Datenbank auszulesen. Das Resultat wird dann über Setter in die View übergeben und innerhalb der Klasse dann dargestellt. Ist das "Regelkonform"? Hier mal ein Beispiel:

class Logic{

String irgendwas;

Logic(){
irgendwas = "test";
}

public String irgendwasAuslesen(){
return irgendwas;
}

class View extends Runnable{

private JButton btn;
private JLabel label;

View(){
}

public void run(){
//frame und components etc anlegen
}

public void addBtnActionListener(ActionListener l){
btn.addActionListener(l);
}

public void setText(String text){
label.setText(text);
}

}

class Controller{

private Logic l;
private View v;

Controller(View v, Logic l){
this.l = l;
this.v = v;
v.addBtnActionListener(new BtnActionListener());
}

class BtnActionListener implements ActionListener{

@Override
        public void actionPerformed(ActionEvent e) {
           v.setText(l.irgendwasAuslesen());
        }

}
}
}

An den Timer werde ich mich morgen mal ransetzen. Hatte sowas natürlich auch schon bei google gefunden, aber genau wie bei intersects() dachte ich das eine selbstgeschriebene Variante besser wäre.

Danke und Liebe Grüße Rhymen

 

[rme]

Auf diese Weise hält es sich zumindest an die Swing-Thread-Regeln, soweit ich das richtig sehe. Aber aus MVC- und aus Performance-Sicht ist das nicht schön.

MVC: Dein Controller musst sich jetzt um Kram wie ActionListeners kümmern, damit möchte ein Controller in MVC eigentlich nix zu tun haben. Ich mache das meistens so, dass der Controller sich seine GUI (JFrame z.B.) selbst erzeugt und sich selbst als Konstruktor-Parameter übergibt, d.h. die Haupt-GUI kennt den Controller. Alle Unter-Komponenten der GUI werden über getter-Methoden exportiert, d.h. der Controller kann sowas machen:

gui.getNameLabel().setText("New name");

Auf diese Weise kann der Controller alle Abläufer in der GUI steuern. Wie kommuniziert aber die GUI mit dem Controller? Nun, ich lasse die Buttons usw. ganz normale ActionListener innerhalb der GUI anlegen. Aber die Actions machen dann nur eine Delegation an den Controller, d.h. sieht ein ActionListener in der GUI z.B. so aus:

controller.onNameChangeRequest();

Die GUI sagt dem Controller also einfach nur, was gerade passiert ist. Da der Controller Zugriff auf die GUI-Komponenten hat, kann er nun die Abläufe regeln. Dabei muss er sich nicht um Listener und so Kram kümmern, das ist innerhalb der GUI-Kassen gekapselt.

Performance: Du rufst irgendwasAuslesen() innerhalb des Swing-Threads auf. Wenn die Methode etwas aus der Datenbank ausliest, kann das ja mal eine Sekunde oder mehr dauern - in dieser Zeit wird nun deine gesamte GUI einfrieren, weil der Swing-Thread blockiert ist. Der korrekte Weg ist:

1) GUI sagt dem Controller, dass der Benutzer das Auslesen auslösen will
2) Controller startet Thread, der Daten im Hintergrund ausliest
3) Controller sagt der GUI (noch im Swing-Thread, also nach dem Starten von 2), dass jetzt ausgelesen wird, also ein kleiner Warte-Dialog, ein Sanduhr-Cursor oder so
4) Thread bekommt Ergebnis aus der Datenbank
5) Thread benutzt SwingUtils.invokeLater, um das Ergebnis der GUI mitzuteilen

Beispiel:

   // wird im Controller aus dem Swing-Thread aufgerufen, da es ein Button-Event oder so ist
    void onIrgendwasAuslesen() {
        gui.setBusy("Dinge werden ausgelesen, bitte warten");
        new Thread(new Runnable() {
            public void run() {
                // lies Dinge aus, kann ruhig lange dauern
                SwingUtilities.invokeLater(new Runnable() {
                    @Override
                    public void run() {
                        gui.getResultLabel().setText("Ergebnis: ...");
                    }
                });
            }
        }).start();
    }

Damit man das mit den Threads nicht umständlich selbst machen muss, gibt es ein paar Hilfsklassen dafür, die du dir anschauen könntest. Stichwort z.B.: RunnableScheduledFuture. Damit kann man dann solche Anfragen auch wieder abbrechen lassen usw.

Edit: Noch sauberer wäre es, wenn es die Methode getResultLabel() nicht gibt, denn dann muss der Controller ja wieder etwas über GUI-Komponenten wissen (Hier: was ein JLabel ist). Die GUI könnte direkt eine Methode setResult() haben, die das dann an das Label delegiert. Man muss selbst entscheiden, wie strikt man sich an solchen Dogmen hält.