WMI-Abfragen Skript schneller machen

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
uni_Ox
User
Beiträge: 4
Registriert: Mittwoch 17. Oktober 2018, 10:04

Mittwoch 17. Oktober 2018, 10:11

Hi,

habe folgendes Skript geschrieben, dass den zuletzt angemeldeten User von PC xy zeigt.

Code: Alles auswählen

for x in testContent:
    try:
        c = wmi.WMI(x)
        for us in c.Win32_LogonSession():
             for user in us.references("Win32_LoggedOnUser"):
                 lastUserLoggedOnArray.append(user.Antecedent.Caption)
    except:
         lastUserLoggedOnArray.append("No Access")
         continue
da der Array mit den PCs (hier: testContent) mehrere Tausend Objekte beeinhaltet, braucht das Skript hochgerechnet über 12h.

Gibt es einen Weg das Skript schneller zu machen?
Möglicherweise über Multithreading (bin Anfänger in Python also kein Multithreading Experte)
Benutzeravatar
snafu
User
Beiträge: 5590
Registriert: Donnerstag 21. Februar 2008, 17:31
Wohnort: Gelsenkirchen

Mittwoch 17. Oktober 2018, 12:14

Da wahrscheinlich der Netzwerkverkehr durch die WMI-Verbindungen der Flaschenhals ist, kann Threading hier durchaus sinnvoll sein. Schau dir mal das Pythonmodul concurrent.futures näher an und hier insbesondere die map()-Methode. Da kannst du die Bezeichnungen für die ganzen Rechner als Liste übergeben sowie deine hier gezeigte Funktion, die dann automatisch pro Rechner aufgerufen wird und somit mehrere WMI-Verbindungen parallel herstellt. Die Funktion würde natürlich als Argument dann jeweils einen Rechnernamen annehmen. Eine komplette Einführung in Python werde ich hier aber nicht geben, da du ja meintest dass du Anfänger bist...
shcol (Repo | Doc | PyPi)
uni_Ox
User
Beiträge: 4
Registriert: Mittwoch 17. Oktober 2018, 10:04

Mittwoch 17. Oktober 2018, 14:32

Ich glaub ich habe da was falsch gemacht. Habe ohne ThreadPool die gleiche Laufzeit wie ohne :o
Output passt.

Code: Alles auswählen

import sys
import wmi
from concurrent import futures
import time

start = time.time()
testContent = ["pcname1","pcname2","pcname3"]
lastUserLoggedOnArray = []

def lastuser(x):
        try:
            c = wmi.WMI(x)
            for us in c.Win32_LogonSession():
                 for user in us.references("Win32_LoggedOnUser"):
                     lastUserLoggedOnArray.append(user.Antecedent.Caption)
                     print user.Antecedent.Caption
        except:
             lastUserLoggedOnArray.append("No Access")
             print 'No Access'
             pass

def main():
    for y in range(len(testContent)):
        lastuser(testContent[y])

    end = time.time()
    print end - start
    
if __name__ == '__main__':
    start = time.time()
    ex = futures.ThreadPoolExecutor(max_workers=2)
    print 'main: starting'
    f = ex.map(lastuser, testContent)

main()

Benutzeravatar
__blackjack__
User
Beiträge: 1448
Registriert: Samstag 2. Juni 2018, 10:21

Mittwoch 17. Oktober 2018, 14:46

@uni_Ox: Da Du ja sagst Du bist Anfänger in Python: Die Namensgebungen sind komisch bis falsch. Konvention ist klein_mit_unterstrichen für alles ausser Konstanten (KOMPLETT_GROSS) und Klassen (MixedCase). Abkürzungen sollte man vermeiden denn die Namen sind ja für den menschlichen Leser, damit der weiss was der Wert bedeutet. Bei `x`, `c`, und `us` ist das alles andere als klar. Auch `testContent` könnte besser benannt sein.

`lastUserLoggedOnArray` ist falsch benannt weil das kein Array ist sondern ich vermute mal eine Liste. Es gibt in Python auch Arrays — im `array`-Modul in der Standardbibliothek, aber meistens sind `Numpy`-Arrays damit gemeint. Solche Grunddatentypen sollten sowieso nicht in Namen stehen, denn es kommt gar nicht so selten vor, dass man den Datentyp im Laufe der Zeit durch einen anderen ersetzt, zum Beispiel eine Liste mit einem Numpy-Array, und dann muss man überall die betroffenen Namen ändern, oder man hat irreführende Namen im Programm. Bei Sequenztypen kann man in der Regel einfach die Mehrzahl von dem verwenden was man für ein einzelnes Element als Namen nehmen würde.

Die Ausnahmebehandlung ist zu weit gefasst, da wird ja wirklich *jede* Ausnahme beghandelt in dem 'No Access' an die Liste angehängt wird. Also auch welche mit denen Du gar nicht rechnest und die gar nichts damit zu tun haben ob Du Zugriff hast oder nicht. Ein `MemoryError`, ein `NameError`, der Versuch das mit Strg+C abzubrechen, … — alles Sachen bei denen die aktuelle Ausnahmebehandlung nicht sinnvoll ist. Man sollte immer nur konkret die Ausnahme(n) behandeln die man auch erwartet. Ansonsten sollte man sicherstellen das Ausnahmen nicht ”verschluckt” werden, weil man sonst sehr viel Spass mit Fehlersuche haben kann. Wenn man alle Ausnahmen behandelt sollte im ”nackten” ``except`` entweder ein ebenso nacktes ``raise`` stehen oder die Ausnahme samt Traceback irgendwo protokolliert werden. Das `logging`-Modul hat dafür auch was passendes.

Macht das 'No Access' in der Liste denn überhaupt Sinn? Man weiss doch hinterher gar nicht mehr zu welchem Rechner das gehört.

Das ``continue`` hat keinen Einfluss auf den Programmablauf, kann also entfallen.

Ungetestet:

Code: Alles auswählen

    for computer_name in computer_names:
        try:
            for session in wmi.WMI(computer_name).Win32_LogonSession():
                for user in session.references('Win32_LoggedOnUser'):
                    last_logged_on_users.append(user.Antecedent.Caption)
        except:
            # 
            # FIXME Just handle specific exception(s) or log exception
            #   and traceback.
            # 
            last_logged_on_users.append('No Access')

Code: Alles auswählen

    **** COMMODORE 64 BASIC V2 ****
 64K RAM SYSTEM  38911 BASIC BYTES FREE
   CYBERPUNX RETRO REPLAY 64KB - 3.8P
READY.
█
Benutzeravatar
__blackjack__
User
Beiträge: 1448
Registriert: Samstag 2. Juni 2018, 10:21

Mittwoch 17. Oktober 2018, 15:27

@uni_Ox: Der Code mit dem `concurrent.futures` macht ja beides, also alles zweimal (beziehungsweise durch einen Fehler im Programm dann doch nicht). Mit dem ``if __name__ …`` und der `main()`-Funktion das ist auch etwas durcheinander. Das macht man ja um das Hauptprogramm von Modulebene in eine Funktion zu bekommen um globale Variablen loszuwerden und das Modul ohne Nebeneffekte importieren zu können. Du hast trotzdem globale Variablen, Programm auf Modulebene, und rufst die `main()` auch noch ausserhalb des ``if __name__ …`` auf.

Eingerückt wird vier Leerzeichen pro Ebene.

`sys` wird importiert, aber nirgends verwendet.

`start` wird zweimal definiert, der erste Wert wird nirgends verwendet.

Funktionen werden üblicherweise nach der Tätigkeit benannt die sie ausführen. `lastuser` wäre ein Name für einen Wert der den letzten Benutzer repräsentiert (mit Unterstrich zwischen den Worten wäre es besser), eine Funktion sollte eher `get_last_user()` oder so ähnlich heissen. Alles was eine Funktion an Werten benötigt sollte die Funktion als Argument betreten und Ergebnisse die Funktion als Rückgabewert verlassen. Insbesondere wenn die Funktion mit einer Funktion wie `map()` verwendet wird, braucht sie einen Rückgabewert, denn sonst macht `map()` keinen Sinn. Das liefert ja die Rückgabewerte von den Aufrufen.

``pass`` macht in einem nichtleeren Block keinen Sinn.

Nun zum Fehler: Du musst die Abfragen nicht nur parallel über den `ThreadPoolExecutor` starten, sondern auch die Ergebnisse verarbeiten. Sonst passiert das parallel solange wie das Hauptprogramm läuft und wird abgebrochen sobald es fertig ist, ohne das Du irgendetwas mit den Ergebnissen gemacht hast. (Die es dafür wie gesagt auch geben muss: also Rückgabewerte von den Aufrufen.)

Ich würde die `max_worker` nicht so stark einschränken wenn man erwartet das die eher blockierend in Kommunikation hängen bleiben. Denn auch bei zweien ist die Chance gross, dass beide auf Daten warten.

``for i in range(len(sequence)):`` und `i` dann als Index in `sequence` zu verwenden ist in Python ein „anti pattern“. Man kann direkt über die Elemente von Sequenztypen iterieren.

Ungetestet:

Code: Alles auswählen

#!/usr/bin/env python2
from __future__ import absolute_import, division, print_function
import time
from itertools import imap

from concurrent import futures
import wmi


def get_last_user(computer_name):
    users = list()
    try:
        for session in wmi.WMI(computer_name).Win32_LogonSession():
            for user in session.references('Win32_LoggedOnUser'):
                users.append(user.Antecedent.Caption)
    except:
        # 
        # FIXME Just handle specific exception(s) or log exception
        #   and traceback.
        # 
        users.append('No Access')

    return users


def main():
    use_executor = True
    computer_names = ['pcname1', 'pcname2', 'pcname3']
    
    print('main: starting')
    start = time.time()
    
    last_logged_on_users = list()
    if use_executor:
        # 
        # TODO Better `max_workers` value.
        # 
        map_function = futures.ThreadPoolExecutor(max_workers=2).map
    else:
        map_function = imap

    for users in map_function(get_last_user, computer_names):
        last_logged_on_users.extend(users)

    end = time.time()
    print(end - start)
    

if __name__ == '__main__':
    main()

Code: Alles auswählen

    **** COMMODORE 64 BASIC V2 ****
 64K RAM SYSTEM  38911 BASIC BYTES FREE
   CYBERPUNX RETRO REPLAY 64KB - 3.8P
READY.
█
Benutzeravatar
snafu
User
Beiträge: 5590
Registriert: Donnerstag 21. Februar 2008, 17:31
Wohnort: Gelsenkirchen

Mittwoch 17. Oktober 2018, 16:15

Ich würde das auf jeden Fall mit mehr Testdaten füttern und - wie schon angeregt wurde - die Anzahl der Worker deutlich erhöhen. In aktuellen Python 3 Versionen kann die Angabe auch weg gelassen werden. Dann nimmt er automatisch Prozessorkerne * 5 für die Anzahl der Worker. Natürlich ist es nicht sinnvoll, dies bis ins "Unendliche" zu steigern, aber eine gewisse Zahl an parallelen Verbindungen sollte der Netzwerk-Stack schon packen.
shcol (Repo | Doc | PyPi)
uni_Ox
User
Beiträge: 4
Registriert: Mittwoch 17. Oktober 2018, 10:04

Donnerstag 18. Oktober 2018, 09:34

__blackjack__ hat geschrieben:
Mittwoch 17. Oktober 2018, 15:27
@uni_Ox: Mit dem ``if __name__ …`` und der `main()`-Funktion das ist auch etwas durcheinander. Das macht man ja um das Hauptprogramm von Modulebene in eine Funktion zu bekommen um globale Variablen loszuwerden und das Modul ohne Nebeneffekte importieren zu können. Du hast trotzdem globale Variablen, Programm auf Modulebene, und rufst die `main()` auch noch ausserhalb des ``if __name__ …`` auf.
Vielen Dank für den Hinweis. Jetzt wo ich darüber nachdenke, frage ich mich selber was ich damit bewirken wollte :D
__blackjack__ hat geschrieben:
Mittwoch 17. Oktober 2018, 15:27
Eingerückt wird vier Leerzeichen pro Ebene.
Funktioniert das nicht genauso gut mit einem Tab statt vier Leerzeichen?
__blackjack__ hat geschrieben:
Mittwoch 17. Oktober 2018, 15:27
Ich würde die `max_worker` nicht so stark einschränken wenn man erwartet das die eher blockierend in Kommunikation hängen bleiben. Denn auch bei zweien ist die Chance gross, dass beide auf Daten warten.
Wie viele max_worker kann ich denn benutzen?
Genauso viele wie meine CPU Threads hat?
__blackjack__ hat geschrieben:
Mittwoch 17. Oktober 2018, 15:27
``for i in range(len(sequence)):`` und `i` dann als Index in `sequence` zu verwenden ist in Python ein „anti pattern“. Man kann direkt über die Elemente von Sequenztypen iterieren.
Heißt ich brauche in Python keinen Index angeben wenn ich durch alle Elemente will? Oder wie ist das gemeint?

Nun zu deinem ungetesteten Programm:
Vielen Dank für das Programm.

Leider wird das Programm in 0,001 Sekunden beendet.
Wie es scheint hat die users List nur ein Element "No Access". (ausgelesen mit

Code: Alles auswählen

print(*users)
am Ende von main())

Code: Alles auswählen

use_executor = False
hat dazu geführt, dass das Programm 21 Sekunden braucht aber users ebenfalls nur ein Element beeinhaltet: "No Access".
Kann es sein dass irgendetwas im 'except' Teil der Funktion 'get_last_user()' noch etwas fehlt?
Sirius3
User
Beiträge: 8608
Registriert: Sonntag 21. Oktober 2012, 17:20

Donnerstag 18. Oktober 2018, 15:51

Leerzeichen sind eine Konvention, an die sich alle halten, weil das mischen von Tabs und Leerzeichen Probleme macht. Wenn Du von irgendwoher Code kopierst, der mit Leerzeichen einrückt und Du Tabs benutzt, dann mußt Du erst alle Einrückungen konvertieren, das wäre doch umständlich.

Threads funktionieren in Python nur gut, wenn sie auf Input warten, wie in Deinem Fall. Und da warten keine CPU braucht, kann man deutlich mehr Threads als CPUs benutzen.

Wie das mit den for-Schleifen gemeint ist, siehst Du schon an Deinem eigenen (kopierten?) Code. Der Index ist nur ein Umweg, den es nicht braucht. for-Schleifen über eine Liste liefern die Listenelemente direkt.

Das mit dem »no access« ist genau das Problem, das __blackjack__ schon angesprochen hat. Da Du alle Fehlermeldungen ignorierst und durch »no access« ersetzt, hast Du keine Change, an die Ursache des Fehlers zu kommen. Entferne das try und den except-Block und schau nach, welcher Fehler da tatsächlich kommt.
Benutzeravatar
__blackjack__
User
Beiträge: 1448
Registriert: Samstag 2. Juni 2018, 10:21

Donnerstag 18. Oktober 2018, 18:46

Und das habe ich natürlich absichtlich eingebaut um genau das zu demonstrieren. (*Hust*, mal seh'n ob mir das irgendwer glaubt. ;-))

Code: Alles auswählen

    **** COMMODORE 64 BASIC V2 ****
 64K RAM SYSTEM  38911 BASIC BYTES FREE
   CYBERPUNX RETRO REPLAY 64KB - 3.8P
READY.
█
uni_Ox
User
Beiträge: 4
Registriert: Mittwoch 17. Oktober 2018, 10:04

Freitag 19. Oktober 2018, 09:51

Vielen Dank für die ganze Hilfe und Infos. Ich probiere mich mal selber am debuggen. Wenn ich es gar nicht schaffen sollte melde ich mich hier wieder :)

__blackjack__ hat geschrieben:
Donnerstag 18. Oktober 2018, 18:46
Und das habe ich natürlich absichtlich eingebaut um genau das zu demonstrieren. (*Hust*, mal seh'n ob mir das irgendwer glaubt. ;-))
Also ich hätte es dir abgekauft :D
Antworten