Statische Propertie in Task (Verständnisproblem)

Hallo,

Ich habe eine statische Klasse Statistics in der ich zu Testzwecken einige Werte die in meiner App anfallen ablege um verschiedene Vorgehensweisen hinsichtlich der Performance bewerten zu können. Dazu zählt eine Propertie TaskCounter mit der ich die Anzahl laufender Instanzen einer bestimmten Task überwachen will. Diese zähle ich mit der statischen Methode AddTaskCounter() hoch und mit SubstractTaskCounter() runter.

    internal void NewChannelData (List<DispatchChannel> channels)
    {
      //Methode wird aus Servertask herraus angestoßen, wenn Channel zu versenden sind. Bekommt _shouldBeDispatched übergeben
      //und fügt die einzelnen Channel daraus einer Synchronized Queue an, welche wiederrum von Clienttask abgearbeitet wird
      Statistics.AddTaskCounter ();
      new Task (
        () =>
        {
          channels.ForEach (channel => channelQueue.Enqueue (channel));
          //Variante B:
          //Statistics.SubstractTaskCounter();
        }).Start ();
      //Variante A
      Statistics.SubstractTaskCounter();
    }

Im Code seht ihr, dass ich zwei Varianten für die Position der Substract Anweisung markiert habe. Mein Problem ist nun, dass bei Variante B es manchmal vorkommt, dass TaskCounter negativ wird. Wie kann das sein? Den das hieße ja, dass SubstractTaskCounter() häufiger ausgeführt wurde als AddTaskCounter().
Den Wert der Propertie ändere ich sonst nirgens in der Applikation. Noch rufe ich die beiden Methode woanders auf als in NewChannelData.

Mit Variante A scheint alles ok, aber irgendwie habe ich nun kein Vertrauen mehr in meine Statistik.

EDIT:
Variante A ist auch kacke, weil der Zähler runter gezählt wird, auch dann wenn die Task noch nicht beendet ist. Oder?

Was benutzt Du zur Synchronisation? lock, Interlocked oder etwas anderes?
Deine Beschreibung legt nahe, daß Du das entweder gar nicht oder falsch machst.

Davon abgesehen, dass statische Klassen i.d.R. ein Hinweis auf ein falsche /suboptimales Design sind... was genau willst Du bei der Statistik denn abdecken?
Ist es Logging, Auditing, Performance-Messerungen, Queue-Überwachungen?

Für eigentlich alles gibt es schon externe Elemente, die sich einfach "draufpacken" lassen statt den eigenen Code beeinflussen.
Der Quellcode sollte nie so gestaltet sein, dass externe Aspekte wie Deployment oder Tracking den Code negativ in seiner Ausführung oder Aufbau beeinflussen.

Was benutzt Du zur Synchronisation? lock, Interlocked oder etwas anderes?
Deine Beschreibung legt nahe, daß Du das entweder gar nicht oder falsch machst.

Ich benutze eine synchronized Queue:

private readonly Queue channelQueue = Queue.Synchronized (new Queue ());

Queue.Synchronized Method

The wrapper returned by this method locks the queue before an operation is performed so that it is performed in a thread-safe manner.

Davon abgesehen, dass statische Klassen i.d.R. ein Hinweis auf ein falsche /suboptimales Design sind... was genau willst Du bei der Statistik denn abdecken?
Ist es Logging, Auditing, Performance-Messerungen, Queue-Überwachungen?

Für eigentlich alles gibt es schon externe Elemente, die sich einfach "draufpacken" lassen statt den eigenen Code beeinflussen.
Der Quellcode sollte nie so gestaltet sein, dass externe Aspekte wie Deployment oder Tracking den Code negativ in seiner Ausführung oder Aufbau beeinflussen.

Ich stecke grad noch in der Entwurfsphase und plane nicht die Applikation inklusive dieser statischen Klasse zu veröffentlichen. Für mich nur ein simpler Weg um schnell meine Performance grob bewerten zu können. (Ohne mich lange in irgendwas ein arbeiten zu müssen. Was allerdings langfristig sinnvoll sein wird.)

Naja, aber was genau ist für Dich denn Performance? Und auf welcher Basis?
Das wird mir hier nicht klar.

Gibt ja viele Wege die Performance einer Anwendung oder Schichten zu messen.
Normalerweise fasst man dafür aber eher nicht den Code an. Daher frage ich, was Du genau messen willst 😃

Ich bau eine Applikation die verschiedene Geräte über Channel verwaltet und als serialisierte Listen von einer Basisklassen von Channel(Wert und Addresse) an Clients über WCF (TCP) sendet. Ich will hier eigentich nur beobachten, dass es zu keiner "Anhäufung" von Task kommt. Also die Daten schnell genug in die Queue geschoben werden können. Also der Task der die Daten wieder raus holt und verarbeitet die Queue nicht so häufig speert, dass die Daten nicht schnell genug verarbeitet werden können, weil sie die meiste Zeit in der Queue abhängen würden und ich somit die Anforderung an die Frequenz mit denen ich Daten empfange nicht erfülle.

Okay, das wäre einfaches Monitoring von Zuständen und keine Performance-Analyse im eigentlichen Sinne.
Ich bin kein großer Fan von InProcess-Queues; würde das eher über RabbitMQ oder RethinkDB machen, da diese für sowas wie Queuing ausgelegt sind und auch viel leichter monitoren lassen.

Schau Dir mal die Monitoring-Namespace von .NET an.
Glaube nicht, dass man Dein Vorhaben von Hand implementieren muss.

Wenn Du es selbst machen willst dann würde ich die TPL Pipelines mit einer eigenen Klasse Wrappen und beim Entnehmen von Jobs anschließend einen Event werfen, der die Duration eines Jobs (also wie lange ist ein Job in der Queue) einfach nur protokolliert.
Würde dann hier unter Logging fallen.

PS: bitte keine Full-Quotes mehr.

Hallo,

Ich benutze eine synchronized Queue:

Ja, schon. Das ist aber hier nicht die Frage, sondern wie Du den Zugriff auf Deinen Counter synchronisierst - sprich, wie AddTaskCounter und SubstractTaskCounter implementiert sind.
Da scheint die Synchronisierung zu fehlen.

Variante A ist auch kacke, weil der Zähler runter gezählt wird, auch dann wenn die Task noch nicht beendet ist. Oder?

Richtig.

Gruß, MarsStein

Hab das ganze jetzt mal stark umgebaut und die Queue raus geworfen.

Das ist aber hier nicht die Frage, sondern wie Du den Zugriff auf Deinen Counter synchronisierst - sprich, wie AddTaskCounter und SubstractTaskCounter implementiert sind.

Gar nicht. Methoden sind aber auch sowiso raus geflogen und zähle jetzt TaskCounter direkt rauf und runter.

 internal async Task NewChannelDataAsync(List<DispatchChannel> channels)
    {
      await Task.Run(
      () =>
      {
        try
        {
          lock (_lockObject)
          {
            Statistics.TaskCounter++;
            channels.ForEach(channel => ChannelMapping(channel));
          }
        }
        catch (Exception ex)
        {
        }
        finally
        {
          lock(_lockObject)
          {
            Statistics.TaskCounter--;
          }
        }
      });
    }

Schau Dir mal die Monitoring-Namespace von .NET an.

Kannst du mir eine empfehlen? Scheint ja eine ganze Menge zu geben die sehr viel können. Aber für meine Zwecke und Fertigkeiten dann etwas zu unhandlich.

Wie wäre es den mit dem System.Diagnostics.PerformanceCounter mit Increment() und Decrement()?

Any public static members of this type are thread safe.

Bekomme den Burschen nur grad nicht zum laufen 😦

public static PerformanceCounter PerformanceCounter;
    static Statistics ()
    {
      _dataHistoryVector = new Vector (20);
      PerformanceCounter = new PerformanceCounter();
      PerformanceCounter.CategoryName = "TaskCounter";
      PerformanceCounter.CounterName = "PerformanceCounter";
      PerformanceCounter.InstanceName = "Client Task";
    }

Aber leider bekomme ich hier eine InvalidOpEx:

labelTaskCounter.Text = Statistics.PerformanceCounter.NextValue().ToString();

{"Die Kategorie ist nicht vorhanden."}

Muss der String in CategoryName ein bestimmter String sein?

EDIT:
Oder direkt Interlock auf dem der PerformanceCounter wohl basiert?

EDIT2:
Die Methode NewChannelData ist nur Async falls ich das mal brauche? Ist ja nicht schlimm außer, dass VS meckert, oder?

Der Code könnte ein Deadlock auslösen.
Besser wäre (ohne inhaltliche Bewrtung)

internal Task NewChannelDataAsync(List<DispatchChannel> channels)
    {
      return Task.Run(

Und um die Frage noch beantworten:
doch, es ist schlimm, denn das ist einfach das falsche Anwenden von async/await 😃

Und um die Frage noch beantworten:
doch, es ist schlimm, denn das ist einfach das falsche Anwenden von async/await 😃

Im Sinne von: Es ist falsch in einen geladenen Lauf zu gucken, aber kann man machen, wenn man Aufpasst, dass man nicht den Abzug drückt? Aber trotzdem nicht machen, weil man nicht weiss ob nicht ein andere am Abzug rum fummelt?

Habs jetzt mit PerformanceCounter gelösst. Sehe ich fürs erste als hinreichend in Abhängigkeit von Aufwand und ich bekomme noch Messungen des Arbeitsspeichers und der CPU dabei . Static fliegt später raus und die Daten landen in einer Datei die über eine extra App ausgewertet werden können und das Messen soll später in der Hauptapp zu und abschaltbar sein.

//gekürzter Auszug
public static class Statistics
  {
    public static PerformanceCounter CpuPerformanceCounter;
    public static PerformanceCounter RamPerformanceCounter;
    public static PerformanceCounter TaskPerformanceCounter;

    static Statistics ()
    {
      CpuPerformanceCounter = new PerformanceCounter("Processor", "% Processor Time", "_Total");
      RamPerformanceCounter = new PerformanceCounter("Memory", "Available MBytes");
      TaskPerformanceCounter = CreateCustomCounters("Task-Counter", true);
    }

private static PerformanceCounter CreateCustomCounters(string category, bool deleteIfExists)
    {
      PerformanceCounter performanceCounter = new PerformanceCounter();

      if (deleteIfExists && PerformanceCounterCategory.Exists(category))
      {
        PerformanceCounterCategory.Delete(category);
      }

      if (!PerformanceCounterCategory.Exists(category))
      {
        CounterCreationDataCollection counterCollection = new CounterCreationDataCollection();

        CounterCreationData opsPerSec = new CounterCreationData();
        opsPerSec.CounterName = "# requests /sec";
        opsPerSec.CounterHelp = "Number of requests executed per second";
        opsPerSec.CounterType = PerformanceCounterType.RateOfCountsPerSecond32;
        counterCollection.Add(opsPerSec);

        CounterCreationData operationTotal = new CounterCreationData();
        operationTotal.CounterName = "Total # requests";
        operationTotal.CounterHelp = "Total number of requests executed";
        operationTotal.CounterType = PerformanceCounterType.NumberOfItems32;
        counterCollection.Add(operationTotal);

        PerformanceCounterCategory.Create(
          category,
          "A custom counter category that tracks the number of Tasks running",
          PerformanceCounterCategoryType.SingleInstance,
          counterCollection);
      }

      performanceCounter = new PerformanceCounter(category, "Total # requests", false);
      performanceCounter.ReadOnly = false;
      performanceCounter.RawValue = 0;

      return performanceCounter;
    }
}

Und ClientTask mit normal und async, weil ich noch nich sicher bin was ich später brauchen/verwenden werde / sinnvoll ist:

internal void NewChannelData(List<DispatchChannel> channels)
    {
      NewChannelDataAsync(channels).Wait();
    }

    internal async Task NewChannelDataAsync(List<DispatchChannel> channels)
    {
      Statistics.TaskPerformanceCounter.Increment();
      await Task.Run(
      () =>
      {
        try
        {
          lock (_lockObject)
          {
            channels.ForEach(channel => ChannelMapping(channel));
          }
        }
        catch (Exception ex)
        {
          //TODO
        }
        finally
        {
         Statistics.TaskPerformanceCounter.Decrement();
        }
      });
    }

Vielen Dank für die Ratschläge 😃

Nein, es ist falsch weil es ein Fehler ist.
Du sagst, dass ein Task zurück gegeben wird; gibst diesen Task aber nicht zurück.
Sowas nennt man Pitfall und ist ein Fehler - nichts anderes. Sorry.
Du wartest (Wait()) hier nicht auf den Task, auf den Du denkst zu warten, sondern auf einen neuen gewrappten Task. Du hast also Task in Task ohne die passende Verbindung und das endet ganz einfach i.d.R. mit einem Deadlock. 😜
Warum überhaupt nen Task, wenn Du ohnehin auf das Ende wartest; macht doch irgendwie wenig sinn? 🙂

So programmiert man async/await nicht. 🙂 Wenn ich solchen Code seh dann seh ich direkt: alles klar, der Kerle hat async/await nicht verstanden.
Davon abgesehen glaube ich an dem Code ausschnitt zu erkennen, dass Du von async/await hier etwas erwartest, wofür es gar nicht gemacht wurde. Ich seh hier gar kein Benefit von async/await an der Stelle.

Wolltest Du hier evt. etwas parallel ausführen und dachtest, dass Du dafür async/await missbrauchen kannst? 😉

Das, was Du jetzt mit dem Performance-Counter misst, hat aber wieder nichts bzw. nicht wirklich was zutun mit dem, was Du bezüglich der Queue gesagt hast.
Sicher, dass Du nen plan hast, was Du überhaupt messen willst? 🙂
Die Anzahl von Tasks sagt doch überhaupt nichts über die Performance der Anwendung, der Queue oder der Durchlaufzeit aus..