Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perfomance boost with ip2addr #1771

Open
wants to merge 3 commits into
base: development03
Choose a base branch
from

Conversation

DanielR92
Copy link
Collaborator

Die optimierte Version der Funktion ip2Arrbietet mehrere Performance-Verbesserungen im Vergleich zur ursprünglichen Version. Hier sind die wichtigsten Punkte:

Verbesserungen und Einsparungen

  1. Verwendung von strtol:

    • Alte Version: Die ursprüngliche Version verwendet atoi, was ineffizient ist, da es den String mehrfach durchsucht.
    • Neue Version: strtolkonvertiert den String direkt in einen Long-Wert und gibt den Pointer auf den nächsten nicht-konvertierten Charakter zurück. Dies spart Zeit, da die Konvertierung und das Finden der nächsten Zahl in einem Schritt erledigt werden.
  2. Reduzierung der Schleifenvariable:

    • Alte Version: Die Schleifenvariable i war als uint8_t deklariert, was bereits effizient ist.
    • Neue Version: Keine Änderung hier, da uint8_t bereits optimal ist.
  3. Vermeidung von mehrfachen containsKey-Aufrufen:

    • Alte Version: Die ursprüngliche Version ruft atoi mehrmals auf, was ineffizient ist.
    • Neue Version: Die neue Version vermeidet dies, indem sie strtol verwendet, was die Anzahl der Funktionsaufrufe reduziert.

Quantitative Einsparungen

  • Zeitersparnis: Die Verwendung von strtol reduziert die Anzahl der Funktionsaufrufe und die Zeit, die für die Konvertierung und das Finden der nächsten Zahl benötigt wird. Dies kann die Ausführungszeit der Funktion erheblich verkürzen, insbesondere bei langen IP-Strings.
  • Speicherersparnis: Die Verwendung von uint8_t für die Schleifenvariable und die direkte Konvertierung mit strtol spart Speicher, da weniger temporäre Variablen und weniger Speicher für Funktionsaufrufe benötigt werden.

Zusammenfassung

Die optimierte Version der Funktion ip2Arr bietet eine effizientere und schnellere Möglichkeit, einen IP-String in ein Array von uint8_t zu konvertieren. Die Hauptvorteile sind die Reduzierung der Anzahl der Funktionsaufrufe und die effizientere Nutzung des Speichers. Diese Verbesserungen führen zu einer besseren Performance und einem geringeren Speicherverbrauch, was besonders wichtig ist für eingebettete Systeme wie den ESP32-S3.

@DanielR92
Copy link
Collaborator Author

Ja, die round3-Funktion kann ebenfalls verbessert werden, indem wir die round-Funktion aus der <cmath>-Bibliothek verwenden.

Diese Version ist präziser und nutzt die vorhandenen Funktionen der Standardbibliothek, was zu einer besseren Lesbarkeit und Wartbarkeit des Codes führt.

@DanielR92 DanielR92 added the enhancement New feature or request label Oct 20, 2024
@Flole998
Copy link

Flole998 commented Oct 20, 2024

Irgendwie erscheint mir der Code komplett falsch, es wird nun 4 mal derselbe String gelesen, die variable uint8_t p ist nun komplett ungenutzt und verbraucht immer noch Speicher (gut, der Compiler wird das vermutlich wegoptimieren und es ist einfach nur störend beim Code lesen).

@Flole998
Copy link

Achja: Hast du den "performance boost" mal gemessen? atoi() ist deutlich schlanker als strtol, und das handling als long braucht auch mehr speicher (und dementsprechend mehr Instruktionen für eine Kopie). Das initialie memset() darf man bei dem Vergleich natürlich auch nicht weglassen, denn das ist schon ein Unterschied.

@DanielR92
Copy link
Collaborator Author

DanielR92 commented Oct 21, 2024

void ip2Arr(const char *ip, uint8_t *arr) {
    for (int i = 0; i < 4; i++) {
        char *endptr;
        arr[i] = (uint8_t)strtol(ip, &endptr, 10);  // strtol für bessere Fehlerbehandlung
        if (endptr == ip || (*endptr != '.' && *endptr != '\0')) {
            // Fehler: Ungültiges IP-Format
            break;
        }
        ip = endptr + 1;  // Zeiger auf das nächste Segment setzen
    }
}

Der Code könnte so aussehen, jedoch besteht das Problem, dass man davon ausgehen muss, dass auch falsche Eingaben möglich sind. Der Code wird nur einmalig beim Start oder beim Aufbau eines Interfaces ausgeführt. Das ist für diesen Anwendungsfall tolerierbar.

In einer idealen Welt, in der jede Eingabe fehlerfrei ist, gebe ich dir Recht, dass atoi die bessere Wahl wäre. Da dies jedoch nicht immer der Fall ist, sollten wir auf Fehler achten. Ich sehe, dass du sehr engagiert in diesem Thema bist, genauso wie bei OpenHAB und Tvheadend(damals selbst genutzt). Es wäre großartig für mich und die gesamte Community, wenn du uns auch hier unterstützen würdest. Wir würden deine Hilfe sehr schätzen.

@Flole998
Copy link

Nun wird ein Fehler zwar erkannt, aber dann einfach der Rest der Adresse nicht mehr gelesen. Wenn man diesen Code anfassen möchte dann sollte man (auch als Vorbereitung für Arduino 3) das ganze IPv6-Ready machen, und dafür könnte man auf die Funktionen von lwIP zurückgreifen. Da muss aber etwas mehr geändert werden als nur diese Funktion.

@DanielR92
Copy link
Collaborator Author

Die Vorbereitung auf IPv6 ist etwas umfangreicher. Das stimmt und wir haben uns in der Vergangenheit auch schon Gedanken gemacht.
Wir gehen noch davon aus, dass der Otto Normalverbraucher das Ding nur lokal nutzt. - IPv6 mit LwIP ist schon eine gute Sache. Wir müssen uns aber noch intern abstimmen, ob wir dafür den ESP8266 überhaupt weiter pflegen wollen oder ob wir ihn komplett verwerfen.

Das würde viele Vorteile bringen. Es macht Sinn den aktuellen Code so gut wie möglich auf eine stabile Version zu bringen, damit zumindest 80% der Nutzer zufrieden sind.

Erst dann kann man zu den 20% übergehen und gleichzeitig evtl. ESP8266 "archivieren".

Daher bin ich dran den Code allgemein zu verbessern.

@DanielR92
Copy link
Collaborator Author

Folgende Ideen habe ich noch dazu:

strtol ist flexibel, aber nicht die schnellste Methode zur Konvertierung von Strings in Zahlen. Eine manuelle Konvertierung könnte schneller sein. Möglichkeit Nr.1:

void ip2Arr(const char *ip, uint8_t *arr) {
    for (int i = 0; i < 4; i++) {
        int num = 0;
        while (*ip >= '0' && *ip <= '9') {
            num = num * 10 + (*ip - '0');
            ip++;
        }
        arr[i] = (uint8_t)num;
        if (*ip == '.') {
            ip++;
        } else if (i < 3) {
            // Fehler: Ungültiges IP-Format
            break;
        }
    }
}

@DanielR92
Copy link
Collaborator Author

Möglichkeit Nr.2:
Wenn man sicher ist dass die Eingabe immer eine gültige IP-Adresse ist, kann man einige der Fehlerprüfungen weglassen:

void ip2Arr(const char *ip, uint8_t *arr) {
    for (int i = 0; i < 4; i++) {
        int num = 0;
        while (*ip >= '0' && *ip <= '9') {
            num = num * 10 + (*ip - '0');
            ip++;
        }
        arr[i] = (uint8_t)num;
        ip++;  // Überspringe den Punkt
    }
}

@DanielR92
Copy link
Collaborator Author

Möglichkeit Nr.3:
sscanf kann mehrere Werte auf einmal einlesen, was den Code kürzer und möglicherweise schneller macht.
Die Nutzung von sscanf könnte die Performance verbessern, indem sie die Anzahl der Operationen und die Komplexität reduziert:

void ip2Arr(const char *ip, uint8_t *arr) {
    sscanf(ip, "%hhu.%hhu.%hhu.%hhu", &arr[0], &arr[1], &arr[2], &arr[3]);
}

@DanielR92
Copy link
Collaborator Author

Das wäre die Lösung mit atoi:

void ip2Arr(const char *ip, uint8_t *arr) {
    for (int i = 0; i < 4; i++) {
        arr[i] = (uint8_t)atoi(ip);
        while (*ip != '.' && *ip != '\0') {
            ip++;
        }
        if (*ip == '.') {
            ip++;
        }
    }
}

@Flole998
Copy link

An diesem Code die Performance zu optimieren ist ja sowieso eher eine akademische Aufgabe, einen wirklichen Sinn macht das nun nicht, da dieser Code ausschließlich bei Änderungen im Webinterface ausgeführt wird. Ob dieser Code nun ein paar Taktzyklen mehr oder weniger braucht macht in der Praxis eigentlich keinen Unterschied, aber es ist natürlich spannend sich so eine Funktion herauszupicken und dann daran zu optimieren so viel wie irgendwie möglich. Bei sscanf würde ich das mit der Schnelligkeit mal ganz schnell wieder vergessen, da wird die Anzahl der Operationen und Komplexität dieser eher in die Höhe getrieben. Mein Ansatz wäre mit inet_pton zu konvertieren, und dann das Ergebnis als u32_t zurückzugeben und so weiter zu verarbeiten. Man kann aber nicht den u32_t in das byte-array schreiben, das verletzt die strict aliasing rule (auch wenn es in bestimmten Fällen funktionieren würde). Der Compiler wird einem das aber angemessen optimieren. Nebenbei wäre es (bis zu einem gewissen Grad) schon die Vorbereitung auf IPv6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants