Analiza ogromnej bazy kodu w Pythonie

Question

Napisałem program w Pythonie, aby analizować ogromną bazę kodu C ++ zawierającą miliony linii kodu. Zadaniem programu jest po prostu wyszukanie wszystkich komentarzy w stylu C i C ++ i wyodrębnienie wszystkich komentarzy, które zawierają określone słowa kluczowe i frazy, oraz zapisanie ich w powiązanym pliku. Jak dotąd program działa wystarczająco dobrze. Jest dość szybki i łatwy w konfiguracji i spełnia swoje zadanie, ale nie wyszukuje w pobliżu tak szybko, jak bym chciał, i chciałbym uzyskać porady, jak przyspieszyć działanie.

Konfiguracja

Program jest bardzo szybki i łatwy w konfiguracji. Jeśli chcesz przeszukać bazę kodu dla pojedynczego wyrażenia, wszystko, co musisz zrobić, to przekazać to wyrażenie jako argument i katalog do przeszukania, a resztę wykona za Ciebie. Aby wyszukać komentarze zawierające słowo „hack” w katalogu src /, możesz po prostu napisać:

./main.py -e hack -s ./src/

Jeśli jednak chcesz przekazać wiele wyrażeń, musisz użyć specjalnie spreparowanego pliku JSON i przekazać nazwę tego pliku jako argument. Przykładowy plik JSON może wyglądać następująco:

[
  {
    "hack-stuff": [
      "hack",
      "hacked",
      "hacky"
    ]
  },
  "WARNING",
  "DANGER",
  [
    "bad",
    "badly"
  ]
]

Plik jest następnie przekazywany do programu w następujący sposób:

./main.py -f test-words.json -s ./src/

Spowoduje to utworzenie czterech plików: „hack-stuff”, „WARNING”, „DANGER” i „bad”. Plik „hack-stuff” będzie miejscem, w którym będą umieszczane wszystkie komentarze zawierające słowa hack, hacked i hacky, „zły” - tam, gdzie będą umieszczane wszystkie komentarze zawierające „zły” i „zły”, a „OSTRZEŻENIE” i „ NIEBEZPIECZEŃSTWO ”będzie miejscem, w którym zostaną umieszczone komentarze zawierające odpowiednio„ OSTRZEŻENIE ”i„ NIEBEZPIECZEŃSTWO ”. Ten przykład demonstruje trzy sposoby (ciąg, lista lub słownik), na które możesz określić, które komentarze pasują do których chcesz umieścić w których plikach.

Struktura programu

Rdzeniem programu jest klasa SourceWalker, która zawiera całą wewnętrzną logikę wymaganą do analizy kodu źródłowego i zapisywania komentarzy do odpowiednich plików. W oddzielnym pliku znajduje się również główna funkcja, która wczytuje argumenty do odpowiednich zmiennych i wykonuje przetwarzanie JSON (w razie potrzeby) przed inicjalizacją i uruchomieniem instancji klasy za pomocą metody walk ().

Wydajność

Próbowałem wielu rzeczy, aby program był tak wydajny, jak to tylko możliwe, w tym włączenie przetwarzania wieloprocesowego, które przyniosło ogromne ulepszenia, ale nie jestem pewien, co mogę zrobić, aby w tym momencie był jeszcze szybszy. Główne spowolnienie jest spowodowane pętlą for wewnątrz _process_files () w linii 117. Jest to część programu, która działa w każdym z procesów potomnych i przeszukuje każdy plik w poszukiwaniu prawidłowych komentarzy przed sprawdzeniem ich w szeregu skompilowane wyrażenia regularne, aby sprawdzić, czy pasują do jednego z wyrażeń, których szukamy. Jestem pewien, że istnieją lepsze sposoby wyodrębnienia komentarzy z każdego pliku, a następnie ich przeszukania, ale nie jestem pewien, jakie by to były. Wszelkie sugestie będą mile widziane.

dodatkowe komentarze

Chociaż wydajność jest tutaj moim głównym zmartwieniem, byłbym również wdzięczny za wszelkie opinie na temat poprawności i stylu mojego programu. Wygląda na to, że działa zgodnie z przeznaczeniem, ale nie mogę zagwarantować, że nie przeoczyłem niektórych skrajnych przypadków podczas moich testów.

Kod

main.py

#!/usr/bin/python3

import sys
import json
import os
import argparse
import SourceWalker

def initialiseParser():
    parser = argparse.ArgumentParser(description = "Search the contents of comments within source code files")

    parser.add_argument("--file_name", "--file", "-f", help = "Name of the file containing the JSON list of expressions to search for", type = str)
    parser.add_argument("--source-dir", "--src", "-s", help = "The root directory of the source files to search over", type = str)
    parser.add_argument("--output-dir", "--out", "-o", help = "The directory the output files will be placed in", type = str)
    parser.add_argument("--expression", "--expr", "-e", help = "The expression to search for within the source comments", type = str)
    parser.add_argument("--language", "--lang", "-l", help = "The style of comments to look for within the file", type = str)

    return parser

def main():
    parser = initialiseParser()
    args = parser.parse_args()

    if args.source_dir:
        source_dir = args.source_dir
    else:
        sys.exit("Source directory must be specified!")

    if args.file_name:
        file_name = args.file_name
        input_file = open(file_name, "r")
        expressions = json.loads(input_file.read())
    elif args.expression:
        expressions = []
        expressions.append(str(args.expression))
    else:
        sys.exit("Error: Expression or file containing expressions must be specified!")

    output_dir = "./comments/"

    if args.output_dir:
        output_dir = args.output_dir

    if not os.path.exists(output_dir):
        os.makedirs(output_dir)

    walker = SourceWalker.SourceWalker(source_dir, output_dir, expressions, extensions=[ ".c", ".cpp", ".h", ".cxx" ])
    walker.walk()

    if "input_file" in locals():
        input_file.close()

    return

if __name__=="__main__":
    main()

SourceWalker.py #! / Usr / bin / python3

import sys
import json
import re
import os
import multiprocessing
import codecs
from pathlib import Path

class SourceWalkerException(Exception):
    """
    Raised when there is an error processing the given expressions
    TODO: Make error handling more informative and clean up. Should display a message to the user explaing what went wrong and close all open files.
    """
    pass

class SourceWalker:
    _output_file_names = []
    _regexes = {}
    _expr_file_names = {}

    def __init__(self, source_dir, output_dir, expr_list, encoding = "ISO-8859-1", process_count = 12, extensions = [ ".c", ".h" ]):
        try:
            if not os.path.exists(source_dir) or not os.path.exists(output_dir):
                raise NotADirectoryError

            if process_count < 1:
                raise SourceWalkerException("Process count cannot be less than one!")

            codecs.lookup(encoding)

            if not isinstance(extensions, list):
                raise SourceWalkerException("Extensions must be passed as a list!")

            for extension in extensions:
                if extension[0] != '.':
                    raise SourceWalkerException("Extensions must start with a \'.\'!")
                elif len(extension) <= 1:
                    raise SourceWalkerException("Extensions must be more than one character long!")
        except NotADirectoryError as exception:
            raise SourceWalkerException("Directory does not exist! " + str(exception))
        else:
            self._source_dir = source_dir
            self._output_dir = output_dir
            self._encoding = encoding
            self._expr_list = expr_list
            self._process_count = process_count
            self._extensions = extensions
            self._process_expr_list()

    def _process_expr_list(self):
        for expr in self._expr_list:
            try:
                if isinstance(expr, list):
                    if len(expr) == 0:
                        raise SourceWalkerException("Expression list cannot be empty!")
                    
                    output_file_name = expr[0]

                    if not isinstance(output_file_name, str):
                        raise SourceWalkerException("Expression sub-lists can only contain strings!")
                    
                    for sub_expr in expr:
                        if not isinstance(sub_expr, str):
                            raise SourceWalkerException("Expression sub-lists can only contain strings!")
                        elif sub_expr in self._regexes.keys():
                            raise SourceWalkerException("Expressions can only appear once in the expression list!")

                        self._regexes[sub_expr] = re.compile("\s+%s(\s|,|:|;|\n)+" % (sub_expr)) # Naieve regex to catch expressions
                        self._expr_file_names[sub_expr] = self._output_dir + output_file_name

                    self._output_file_names.append(self._output_dir + output_file_name)
                elif isinstance(expr, dict):
                    if len(expr.keys()) == 0:
                        raise SourceWalkerException("Expression dictionary cannot be empty!")

                    output_file_name = list(expr)[0]

                    if not isinstance(expr[output_file_name], list):
                        raise SourceWalkerException("Expression dictionary cannot be empty!")
                    
                    for sub_expr in expr[output_file_name]:
                        if not isinstance(sub_expr, str):
                            raise SourceWalkerException("Expression sub-lists can only contain strings!")
                        elif sub_expr in self._regexes.keys():
                            raise SourceWalkerException("Expressions can only appear once in the expression list!")

                        self._regexes[sub_expr] = re.compile("\s+%s(\s|,|:|;|\n)+" % (sub_expr))
                        self._expr_file_names[sub_expr] = self._output_dir + output_file_name

                    self._output_file_names.append(self._output_dir + output_file_name)
                elif isinstance(expr, str):
                    if expr in self._regexes.keys():
                        raise SourceWalkerException("Expressions can only appear once in the expression list!")

                    self._output_file_names.append(self._output_dir + expr)
                    self._regexes[expr] = re.compile("\s+%s(\s|,|:|;|\n)+" % (expr))
                    self._expr_file_names[expr] = self._output_dir + expr
                else:
                    raise SourceWalkerException("Expression list can only contain dictionaries, lists, and strings!")
            except SourceWalkerException as exception:
                self.cleanup()
                raise

    def _process_files(self, input_files, output_files, mutexes): # Find way to process different types of source file, I'd rather not be limited to C only...
        for file_name in iter(input_files.get, None):
            with open(file_name, "r", encoding = self._encoding) as file_object:
                in_multi_comment = False
                in_single_comment = False
                in_string = False
                prev_char = ''
                comment = ''

                for line_num, line in enumerate(file_object, 1):
                    for char in line:
                        if char == '/':
                            if in_string or in_single_comment:
                                prev_char = char
                                continue
                            if prev_char == '*':
                                in_multi_comment = False
                                comment += char

                                for expr in self._regexes.keys():
                                    if self._regexes[expr].search(comment):
                                        mutexes[expr].acquire()
                                        os.write(output_files[expr], ("%s: %s %s\n" % (file_name, str(line_num), comment)).encode())
                                        mutexes[expr].release()

                                comment = ''
                            elif prev_char == '/':
                                in_single_comment = True
                                comment += prev_char
                        elif char == '*':
                            if in_string or in_single_comment or in_multi_comment:
                                if in_single_comment or in_multi_comment:
                                    comment += char

                                prev_char = char
                                continue
                            if prev_char == '/':
                                in_multi_comment = True
                                comment += prev_char
                        elif char == '"':
                            if prev_char == '\\' or in_single_comment or in_multi_comment:
                                prev_char = char
                                continue
                            
                            in_string = not in_string

                        prev_char = char

                        if in_single_comment or in_multi_comment:
                            comment += char

                    if in_single_comment:
                        in_single_comment = False
                        
                        for expr in self._regexes.keys():
                            if self._regexes[expr].search(comment):
                                mutexes[expr].acquire()
                                os.write(output_files[expr], ("%s: %s %s" % (file_name, str(line_num), comment)).encode())
                                mutexes[expr].release()

                        comment = ''

    def walk(self):
        input_files = multiprocessing.Queue(0)
        processes = []
        mutexes = {}
        output_files = {}

        for fname in self._output_file_names:
            try:
                file_handle = os.open(fname, os.O_WRONLY | os.O_CREAT)
                mutex = multiprocessing.Lock()
            except IOError:
                for file in output_files.keys():
                    output_files[file].close()

                raise SourceWalkerException("Error: Could not open output file %s, skipping!" % fname)

            for expr in self._expr_file_names.keys():
                if self._expr_file_names[expr] == fname:
                    output_files[expr] = file_handle
                    mutexes[expr] = mutex

        for root, dirs, file_names in os.walk(self._source_dir):
            for file_name in file_names:
                if any(ext in Path(file_name).suffix for ext in self._extensions):
                    input_files.put(os.path.join(root, file_name))

        for i in range(self._process_count):
            input_files.put(None)
        
        for cur_process in range(self._process_count):
            process = multiprocessing.Process(target = self._process_files, args = (input_files, output_files, mutexes))
            processes.append(process)
            process.start()

        for i in range(1, self._process_count):
            processes[i].join()

        for file in output_files.keys(): # Close the file associated with each expression
            try:
                os.close(output_files[file]) # Since multiple expressions can be associated with the same file we need to avoid invalid file closures
            except:
                pass

Answer 1

Jeśli rozumiem twój opis, szukasz tylko komentarzy, ale za każdym razem przeszukujesz pełną bazę kodu. Ponieważ komentarze stanowią zwykle niewielką część kodu (mniej niż 10%?), Sugeruję wykonanie najpierw etapu przedprocesowego, w którym po prostu wyodrębniasz wszystkie komentarze, a następnie przeszukujesz je.

Przez „wyodrębnianie” rozumiem zapisywanie komentarza w oddzielnych plikach, aby można było wyszukiwać tylko w tych plikach.

W przypadku powtarzających się wyszukiwań w tej samej bazie kodu powinno to pomóc, ponieważ przetwarzanie wstępne można wykonać raz, a wtedy rzeczywiste wyszukiwanie ma mniej tekstu do przejrzenia.

Answer 2

Możesz określić wymagane argumenty w argparsezamiast obsługiwać je samodzielnie. Możesz także określić wartości domyślne , na przykład dla output_dir.

os.makedirstrwa, exist_ok=Trueaby wskazać, że powinien utworzyć katalog tylko wtedy, gdy jeszcze nie istnieje.

extensionsArgument SourceWalkerkonstruktora prawdopodobnie nie powinien być ustalony.

Użyj, with open(file_name, "r") as file_handle:aby upewnić się, że plik jest zawsze zamknięty, gdy opuszczasz ten kontekst. W związku z tym locals()IMO powinno być używane tylko w ostateczności, ponieważ trudno jest śledzić kod, w którym ciągi są używane do odwoływania się do zmiennych, a środowiska IDE nie mogą zrobić nic użytecznego z tym kodem. Na przykład, jeśli miałbyś zmienić nazwę input_filena input_file_path(w każdym przypadku zalecałbym to), byłoby trywialne zapomnienie o zmianie odwołania do łańcucha.

Zalecałbym użycie blacki isortsformatowanie kodu. W ten sposób będzie bliżej stylu idiomatycznego, bez ręcznej pracy.

Po sformatowaniu polecałbym bieganie, flake8aby znaleźć inny kod nie idiomatyczny. Na przykład wygląda na to, że SourceWalkerkod jest uszkodzony - metody są na tym samym poziomie, co definicja klasy.

Tworzenie wyjątku ze statycznym ciągiem znaków, takim jak SourceWalkerException("Process count cannot be less than one!"), jest zapachem kodu. To nie jest koniecznie złe, ale jak to zwykle parametrów logicznych oznacza coś powinno być rozrywane. W takim przypadku byłoby lepiej mieć oddzielną klasę wyjątków dla zbyt małej liczby procesów (i innych błędów), co mogłoby być czymś tak prostym, jak ProcessCountTooLowError. W ten sposób jedyną rzeczą przekazywaną do konstruktora wyjątków (jeśli cokolwiek) jest dowolna zawartość dynamiczna, której można użyć do debugowania tego wyjątku. Oznacza to również, że jeśli kiedykolwiek zechcesz obsłużyć wyjątek, możesz except ProcessCountTooLowErrorzamiast analizować komunikat o błędzie w ramach generycznego except SourceWalkerException. (Aha, a niestandardowe klasy wyjątków powinny kończyć się na Error, nie Exception).

Zamiast sprawdzać, czy istnieje katalog wyjściowy, pozwolę aplikacji zakończyć się niepowodzeniem, gdy dotrze do kodu, który próbuje zapisać w tym katalogu. Nazywa się to „czasem sprawdzenia do czasu użycia” - w zasadzie to, czy katalog istnieje, kiedy dokonujesz tego sprawdzenia, nie ma wpływu na to, czy istnieje, kiedy kod faktycznie próbuje go użyć. Jest to również zbyt wąska kontrola, ponieważ na przykład katalog może również nie być zapisywalny przez bieżącego użytkownika lub system plików może być pełny.

Podobnie, uruchomienie w codecs.lookup(encoding)celu sprawdzenia, czy kodowanie istnieje, zanim faktycznie go użyjesz w zupełnie innym wywołaniu, wydaje się, że łatwo może być niepełne sprawdzenie. Lepiej byłoby ograniczyć parametr kodowania optionstylko do dostępnych kodowań. W ten sposób jest sprawdzany tak wcześnie, jak to możliwe, użytkownicy otrzymują niezły błąd, który można wykonać, a tekst pomocy pokazuje możliwe wartości.

Liczba mnoga wyrażenia „regex” to „regex e s”.

Nie używaj zmiennych domyślnych argumentów ( dyskusja ). A propos, kiedy w ogóle należy używać argumentów domyślnych? Dwie praktyczne zasady:

1. Jeśli wartość domyślna nigdy nie jest używana, ponieważ wszystkie wywołania określają wartość, wartość domyślna jest bezcelowa.
2. Jeśli żadne z wywołań nie zastępuje wartości domyślnej, jest to również bezcelowe - równie dobrze może to być zmienna lub stała.

if not isinstance(extensions, list):nie jest idiomatyczne; powinno być całkowicie poprawne, aby przekazać dowolną iterowalną, taką jak tuple.

Generalnie konstruktorzy nie powinni robić nic bardziej złożonego niż ustawianie wartości pól. Gdy to zrobisz run, processnależy uruchomić osobno metodę lub inną metodę, aby wykonać faktyczne przetwarzanie. Nie pamiętam, gdzie po raz pierwszy zobaczyłem to wyjaśnione jasno, ale zobacz na przykład Dlaczego używanie efektów ubocznych jest złą praktyką w konstruktorach JavaScript? a konstruktorzy muszą być wolni od kodu

Answer 3

Lista literałów

    expressions = []
    expressions.append(str(args.expression))

powinno po prostu być

    expressions = [str(args.expression)]

Pathlib

To:

if not os.path.exists(output_dir):
    os.makedirs(output_dir)

powinien użyć nowego, błyszczącego pathlibodpowiednika:

Path(output_dir).mkdir(exist_ok=True)

Ten sam moduł może być użyty do

if not os.path.exists(source_dir) or not os.path.exists(output_dir):

Zmienne istnienie

if "input_file" in locals():
    input_file.close()

jest szkicowy. Zwykle sposobem wskazania w Pythonie, że zmienna ma wartość lub nie, jest jej potencjalne przyjęcie None, a nie niezadeklarowana. Możesz nawet oznaczyć go jako może - brak, używając Optionalpodpowiedzi typu.

Powrót

Singiel returnna końcu main()jest zbędny.

Długość słownika

if len(expr.keys()) == 0:

może być

if len(expr) == 0:

lub nawet

if not expr:

Kompilacja Regex

                self._regexs[sub_expr] = re.compile("\s+%s(\s|,|:|;|\n)+" % (sub_expr))

potrzebuje przynajmniej doprowadzenia, raby ten ciąg był dosłownie surowy.

Ogólnie biorąc, pobieranie danych wejściowych z pliku i traktowanie ich jako niezwalidowanego podrzędnego wyrażenia regularnego jest złym pomysłem. Nie jest evalzły, ale nie jest dobry. Czy są to rzeczywiste wyrażenia regularne, czy tylko podciągi? Jeśli są to tylko podciągi, wywołaj escapeje przed wstawieniem do zewnętrznego wyrażenia regularnego.

Jeśli faktycznie są one własnym wyrażeniem regularnym, będziesz chciał przynajmniej umieścić je w osobnej grupie nieprzechwytywanej w wyrażeniu zewnętrznym, aby uniknąć przykrych niespodzianek.

Łańcuch wyjątków

except NotADirectoryError as exception:
    raise SourceWalkerException("Directory does not exist! " + str(exception))

Powinien być

except NotADirectoryError as exception:
    raise SourceWalkerException("Directory does not exist! " + str(exception)) from exception