Laden...

GUI blockiert trotz Benutzung von async & await

Erstellt von Tician vor 6 Jahren Letzter Beitrag vor 6 Jahren 4.211 Views
T
Tician Themenstarter:in
25 Beiträge seit 2015
vor 6 Jahren
GUI blockiert trotz Benutzung von async & await

Hallöööchen mit 3 öchen,

Kurzum: Ich bin kein Programmierer in dem Sinne (lerne FISI) und programmiere weil es sich anbietet und es mir Spaß macht. Ich kenne die Grundlagen (oder hoffe es zumindest) und beschäftige mich aus gegebenen Anlässen meist mit neuen Dingen - wie jetzt mit async und await.

Das Problem: Ich habe 3 absolut rießige SQL-Abfragen (jeweils über 100 Zeilen) die los rennen sobald das Programm gestartet wird. Es dauert 20 Sekunden bis die GUI (WinForm-Anwendung) gezeigt wird - viel lieber hätte ich gerne die GUI angezeigt und Schritt für Schritt geupdated.

Mein Ansatz sieht so aus:

using Oracle.ManagedDataAccess.Client;
...

OracleConnection con = new OracleConnection();
DataTable dataRollkarte = new DataTable();

int sumRollkarte = 0;
		
		public Form1()
        {
			InitializeComponent();
			this.Shown += LoadSQL; //Oberfläche wird angezeigt, aber blockiert
		}
		
		public async void LoadSQL(object sender, EventArgs e)
        {
            //Rollkarte   
            label7.Text = "Processing";
            sumRollkarte = 0;
            dataRollkarte = await GetRollkarte();
            foreach (DataRow dr in dataRollkarte.Rows)
            {
                sumRollkarte += Convert.ToInt32(dr[11]); 
            }
            label2.Text = Convert.ToString(sumRollkarte);
            label1.BackColor = Color.LimeGreen;
            label2.BackColor = Color.LimeGreen;
            label7.Text = "Fertig";
        }
		
        private async Task<DataTable> GetRollkarte()
        {
            dbConnect();
            OracleCommand cmd = new OracleCommand();
            cmd.CommandType = CommandType.Text;
            cmd.CommandText = @"Viel zu lange...";
            cmd.Connection = con;

            DataTable dt = new DataTable();
            using (DbDataReader dr = await cmd.ExecuteReaderAsync())
            {                
                dt.Load(dr);
            }
            dbDisconnect();

            return dt;
        }

Die Idee war also die GUI anzuzeigen und dann per Form.Shown-Event die Abfragen losrennen zu lassen - die GUI wird auch angezeigt, aber ist trotzdem blockiert und die Elemente darauf werden nur als weiße Vierecke angezeigt.
Ich glaube ich muss mein Event mit 'Task' als Rückgabewert versehen und bei einer 'normalen' Methode verstehe ich die vorgehensweise auch, aber mit Events mangelt es mir an Verständnis weil ich da noch nie einen Rückgabewert gebraucht habe.

Könnte mir jemand auf die Sprünge helfen? Ich weiß, dass mein Code nicht der sauberste ist (und im Zweifelfall fallen den gelernten Programmierern bei meinem Code die Haare aus), aber eines nach dem anderen solange ich noch Zeit habe das Ding hier zu verbessern.

16.806 Beiträge seit 2008
vor 6 Jahren

Bitte in Zukunft im richtigen Forum posten.

Auch mit async/await brauchst Du einen extra Task, damit die Abfrage die GUI nicht blockiertzB mit await Task.Run(), sofern es sich um eine CPU-lastige Aufgabe handelt.
async/await hilft Dir, dass der Code einfacher wird und besser lesbar ist; baut Dir aber nicht automatisiert einen extra Task drum herum.

Bei Dir wird weiterhin alles im UI Thread ausgeführt und damit blockiert die UI natürlich.

Zudem ist es sehr unsauber Datenbank-Code im UI-Layer zu haben.
[Artikel] Drei-Schichten-Architektur

D
985 Beiträge seit 2014
vor 6 Jahren

Bei dem aktuellen Code wird nur cmd.ExecuteReaderAsync() nebenläufig ausgeführt. Der ganze Rest ist komplett synchron und damit blockierend.

1.029 Beiträge seit 2010
vor 6 Jahren

Hi,

ich sehe da direkt mehrere Fehler:

a) Zu jedem async - gehört auch ein await

this.Shown += LoadSQL; //Oberfläche wird angezeigt, aber blockiert

fehlt sowas z.B. mal als Beispiel:


public partial class Form1 : Form
    {
        public Form1()
        {
            InitializeComponent();
            Shown += async (sender, args) =>
            {
                await InitiateLengthyOperation();
            };
        }

        private async Task InitiateLengthyOperation()
        {
            label1.Text = "Processing";
            await DoLenghtyOperation();
            label1.Text = "Done";
        }

        private async Task DoLenghtyOperation()
        {
            await Task.Delay(5000);
        }
    }

b) Die Datenbankoperation ist nicht async - wird somit nicht awaited - und wird somit deine UI blockieren. (Siehe Mitteilung von Abt)

LG

16.806 Beiträge seit 2008
vor 6 Jahren

Taipi88, da sein Shown ein Event ist, ist das LoadSQL so prinzipiell kein Pitfall.

2.078 Beiträge seit 2012
vor 6 Jahren

Dass das Event keinen Rückgabewert hat, ist schon richtig so, solange es als async markiert ist und die langandernde Aufgabe awaited wird.
Dass der Handler keinen Task als Rückgabewert hat, bedeutet nur, dass beim Aufruf der Methode nicht auf ihre Beendigung gewartet werden kann.

Deine UI blockiert, weil das ExecuteReaderAsync zwar asynchron ausgeführt wird, der Rest der Methode danach (also auch Load) wird aber wieder im UI-Thread ausgeführt und kann den dann auch blockieren.
Es reicht also nicht aus, irgendwo ein await xy zu haben. Das ist immerhin die große Stärke an dem ganzen Konzept, dass es dir den Code danach wieder zurück synchronisiert 😉

Du kannst natürlich einfach alles in einen eigenen Task auslagern:
Task.Run-Methode: (Action)

Ich persönlich finde aber besser, wenn Du die Daten Zeile für Zeile liest.
Der DataReader ist so gebaut, dass er das kann, deshalb dauert auch nicht ExecuteReaderAsync lange, sondern Load.
Der DataReader liest also das erste Ergebnis, während die Datenbank das Zweite noch gar nicht gefunden hat.
Du solltest also lieber Zeile für Zeile lesen, nicht alles auf einmal.
Z.B. so:

async Task ReadRollkartenAsync(DbDataReader reader)
{
    using (reader)
    {
        while (await reader.ReadAsync())
        {
            var rollkarte = new Rollkarte()
            {
                Text = reader.GetString(0),
                Zahl = reader.GetInt32(0),
            };

            Rollkarten.Add(rollkarte);
        }
    }
}

Das setzt natürlich voraus, dass Du eine Klasse und eine Liste hast, was ich aber beides empfehlen würde.
Und - wie Abt schon sagt - das sollte nicht in einer UI-Klasse liegen, sondern in einer eigenen Daten-Schicht.

Ach und wegen Task, liest dich mal hier ein:
Await, SynchronizationContext, and Console Apps
Dieser Artikel hat mir persönlich sehr geholfen, das ganze Konzept hinter async/await zu verstehen.
Das beschäftigt sich damit, einen eigenen SynchronizationContext zu schreiben, der eine asynchrone Methode synchron zwingt.
Für dich bringt das in deiner Situation nichts, aber es hilft vielleicht, die Zusammenhänge zu verstehen.

@Taipi88:
a) stimmt so nicht ganz
Der EventHandler von Tician ist richtig.
Der muss async sein und der zeitfressende Teil danach muss in einem eigenen Task laufen.
Und da, nur da, liegt der Hund begraben: Der wirklich lang andauernde Teil läuft nicht in einem eigenen Task mit Thread vom ThreadPool sondern wird vom UI-Thread ausgeführt.
Der Fehler liegt also in der GetRollkarte-Methode

1.029 Beiträge seit 2010
vor 6 Jahren

Taipi88, da sein Shown ein Event ist, ist das LoadSQL so prinzipiell kein Pitfall.

Tatsache - hast natürlich Recht - allerdings gilt das halt auch nur, wenn er keinen EventListener hat, der nach diesem LoadSQL laufen soll. (Was hier nicht der Fall ist)

LG

T
Tician Themenstarter:in
25 Beiträge seit 2015
vor 6 Jahren

Hui das ging schnell, danke euch!
Ich schmeiß mal alles im Event nochmal in eine eigene Methode und schau mal ob es dann funktioniert.
Ich weiß es gibt tausend Dinge zum besser machen, aber wenn das dann läuft habe ich noch using-Anweisungen auf meiner Liste die um ein paar Lese- und Schreibvorgänge müssen, auch mein Exception-Handling ist aufgrund des Zeitdrucks unter aller Kanone.
Eins nach dem anderen, ich melde mich nochmal 😃

D
985 Beiträge seit 2014
vor 6 Jahren

Ich schmeiß mal alles im Event nochmal in eine eigene Methode und schau mal ob es dann funktioniert.

Das bringt dir nichts, denn auf die Controls musst du synchron zugreifen.

2.078 Beiträge seit 2012
vor 6 Jahren

Wenn's schnell gehen soll:

private async Task<DataTable> GetRollkarte()
{
    return Task.Run(() =>
    {
        dbConnect();
        OracleCommand cmd = new OracleCommand();
        cmd.CommandType = CommandType.Text;
        cmd.CommandText = @"Viel zu lange...";
        cmd.Connection = con;

        DataTable dt = new DataTable();
        using (DbDataReader dr = cmd.ExecuteReader())
        {
            dt.Load(dr);
        }
        dbDisconnect();

        return dt;
    });
}

Wirklich gut finde ich das aber immer noch nicht.

T
Tician Themenstarter:in
25 Beiträge seit 2015
vor 6 Jahren

Das macht mich gerade echt fertig.
Wie kommt es, dass ExecuteReaderAsync als einziges tatsächlich async ausgeführt wird und der Rest nicht?

Jetzt sieht mein Code so aus, gibt mir aber die Warnung, dass mir await-Operatoren fehlen.


		public Form1()
        {
			InitializeComponent();
			this.Shown += async (sender, args) =>
            {
                await LoadSQL();
            };
		}

		public async Task LoadSQL()
        {
            //Rollkarte   
            label7.Text = "Processing";
            sumRollkarte = 0;
            dataRollkarte = await GetRollkarte();
            foreach (DataRow dr in dataRollkarte.Rows)
            {
                sumRollkarte += Convert.ToInt32(dr[11]);
            }
            label2.Text = Convert.ToString(sumRollkarte);
            label1.BackColor = Color.LimeGreen;
            label2.BackColor = Color.LimeGreen;
            label7.Text = "Fertig";
        }
		
        private async Task<DataTable> GetRollkarte()
        {
            dbConnect();
            OracleCommand cmd = new OracleCommand();
            cmd.CommandType = CommandType.Text;
            cmd.CommandText = @"Viel zu lange...";
            cmd.Connection = con;

            DataTable dt = new DataTable();
            using (DbDataReader dr = await cmd.ExecuteReaderAsync())
            {                
                dt.Load(dr);
            }
            dbDisconnect();

            return dt;
        }

Alles was ich eigentlich will ist, dass meine Form angezeigt wird, nicht blockiert wird, meine labels sich verändern, die Methode GetRollkarte() komplett asynchron läuft - nicht nur die Abfrage - und sich danach die labels wieder verändern.
Ich glaube die Aufgabe übersteigt mein Verständnis, nicht nur das Async ich habe auch noch nie mit Lambda-Ausdrücken gearbeitet.

Tief durchatmen. Was ich aus den Antworten gerade mitnehme:
Zu jedem async brauche ich ein await - gecheckt, das fehlt meiner letzten Methode weil ich dachte ich könnte die komplette Methode async laufen lassen.
Irgendwie spielt das Task.Run noch eine Rolle darin, was das eigentliche asynchrone ausmacht, alleine async und await bringen nichts - fehlt mir noch glaube ich.
**Zeile für Zeile einlesen **- ich setze es auf die lange Liste der verbesserungen.

16.806 Beiträge seit 2008
vor 6 Jahren

Damit eine UI nicht blockiert, brauchst Du eine parallele Ausführung - durch Threads oder Tasks.

async/await ist KEINE parallel Ausführung, sondern eine asynchrone.
Und asynchron != parallel.
Das sind zwei verschiedene Konzepte.

Du brauchst also weiterhin einen Task, der Deine blockierende Logik ausführt.

Welche Zeile mäckert er an wegen einem fehlenden await?

Du brauchst unbedingt das Grundverständnis von asynchroner und paralleler Programmierung.

2.078 Beiträge seit 2012
vor 6 Jahren

Also ich kann da keine Stelle finden, wo ein await fehlt, es scheint alles da zu sein 🤔

Aber bei dir ist weiterhin das Problem dieses Stück Code:

using (DbDataReader dr = await cmd.ExecuteReaderAsync())
{
    dt.Load(dr);
}

lange dauert.

Der Inhalt von ExecuteReaderAsync kann (muss nicht) in einem eigenen Thread laufen.
Das await sorgt dafür, dass deine Methode so lange wartet (ohne dabei den UI-Thread zu blockieren) bis das Ergebnis von ExecuteReaderAsync da ist.
Danach arbeitet deine Methode im UI-Thread weiter.

Danach kommt aber das dt.Load(dr), was den Löwenanteil darstellt.
Du müsstest also schreiben:

using (DbDataReader dr = await cmd.ExecuteReaderAsync())
{
    await Task.Run(() => dt.Load(dr));
}

Das Task.Run baut mit der übergebene Aufgabe einen Task.
Dieser Task wird dann auf dem ThreadPool und damit irgendeinem x-beliebigen Thread (nicht der UI-Thread) ausgeführt.

Oder Du führt den ganzen Inhalt der Methode in einem Task aus, wie ich das weiter oben ja schon geschrieben habe.
Das würde ich übrigens auch bevorzugen, hauptsächlich weil ich nicht weiß, wie sich die Connection verhält, wenn sie in einem anderen Thread Daten laden soll.

T
Tician Themenstarter:in
25 Beiträge seit 2015
vor 6 Jahren

Da ist auch keine Stelle in der das await fehlt, ich hatte den Code falsch kopiert...

Etwas das ich komisch finde, das Shown-Event sollte doch erst triggern wenn mein Form komplett da ist oder? Das ist sie aber nicht.


   public partial class Form1 : Form
    {
            this.Shown += async (sender, args) =>
            {
                await LoadSQL();
            };
        }
        public async Task LoadSQL()
        {
            label7.Text = "Processing"; //Breakpoint
            sumRollkarte = 0;
            ...
        }

Wenn ich direkt am Anfang des Shown-Event einen Breakpoint mache habe ich die Form zwar irgendwie angezeigt, aber die Elemente sind nur als weiße vierecke dargestellt - da ich aber wie im Code zu sehen ist eigentlich will das man ein label sehen kann ist irgendwas mit dem Ansatz falsch.

Gelesen habe ich, dass mein Event ja trotzdem die GUI blockiert - und deswegen die GUI so seltsam aussieht.

Das bringt dir nichts, denn auf die Controls musst du synchron zugreifen.

Wie kann ich also mein Event von der GUI trennen und trotzdem zwischendrin labels verändern?

16.806 Beiträge seit 2008
vor 6 Jahren

Naja eigentlich braucht man dank async/await kein Invoke mehr; das sollte ja der Zustandsautomat im Hintergrund auflösen, sofern es um IO-Aufgaben geht.
CPU-lastige Aktionen müssen auch bei async/await in ein Task gepackt werden.

5.299 Beiträge seit 2008
vor 6 Jahren

braucht man dank async/await kein Invoke mehr jepp.

Ansonsten denke ich, ein grundlegender Fehler hier ist, dass versucht wird, die Arbeits-Methode iwie async zu modifizieren.
Das ist nicht erforderlich (kann sein, dasses möglich ist, aber erforderlich ists nicht, und obs und wanns sinnvoll ist weiss ich auch nicht, weil ich mach das nie so.)

Imo soll die Arbeits-Methode arbeiten, und sich nicht um die Nebenläufigkeit kümmern.
Nebenläufigkeit erreicht man durch einen bischen komischen Aufruf, mit Await, Task.Run() und den eiglichen Aufruf in eine anonyme Methode gewrappert.

Hier Tut dazu:
Async/Await: Unblock Gui without any additional Line of Code

Der frühe Apfel fängt den Wurm.