Samenvatting van de vorige afleveringen
- Tussen 2020 en 2022 onderging Darktable een onderneming tot massavernietiging, door een handjevol gasten met meer vrije tijd en welwillendheid dan werkelijke vaardigheden,
- In 2022 begon ik een vervelende vertraging op te merken tussen GUI-interacties met schuifregelaars en de feedback/update van diezelfde schuifregelaars. Bij gebrek aan feedback die aangaf dat de waardewijziging was geregistreerd, konden gebruikers de waarde opnieuw wijzigen, waardoor er extra pipeline-herberekeningen werden gestart en hun computer feitelijk bevroor omdat de stomme GUI nooit zei „begrepen, wacht nu even".
- Ik ontdekte dat opdrachten voor pipeline-herberekening tweemaal per klik werden uitgegeven (eenmaal bij het „knop ingedrukt"-event, eenmaal bij het „knop losgelaten"-event), en nog eens voor elke muisbeweging, maar ook dat de GUI-toestanden schijnbaar ná de pipe-herberekening werden bijgewerkt.
- Ik loste dat op door de aangepaste GUI-regelaars (de Bauhaus-bibliotheek) vrijwel te herschrijven. Ik dacht dat het voorkomen van roekeloze herberekenopdrachten de vertraging zou verhelpen: dat deed het niet. Vervolgens ontdekte ik dat het aanvragen van een nieuwe pipeline-herberekening vóórdat de vorige klaar was, wachtte tot de vorige klaar was, ondanks een uitschakelmechanisme dat vele jaren geleden was geïmplementeerd en dat had moeten werken.
- Ik loste dat op door een kill-switch-mechanisme op pipelines te implementeren, op basis van commentaar in de code uit de jaren 2010 en interne hulpprogramma’s die misschien wel nooit gewerkt hebben. Dit werkte niet altijd omdat de kill-opdracht vaak met een merkbare vertraging aankwam. Ook nu weer werd de GUI-vertraging niet verholpen.
…
Aflevering 5: de technische schuld afbetalen
Wat ik ontdekte zou werkelijk in de handboeken informatica moeten belanden, in het hoofdstuk over wat je niet moet doen als je een enigszins betrouwbare applicatie wilt schrijven.
Dus telkens als een beeldbewerkingsparameter in een module werd gewijzigd, werd er een verzoek verstuurd om een nieuwe geschiedenisvermelding aan de database toe te voegen (vaak meer dan eens per interactie, zoals hierboven getoond). Geschiedenisvermeldingen zijn niets meer dan een momentopname van de interne parameters van één module (inclusief maskers). Als er een wijziging werd gedetecteerd ten opzichte van de vorige geschiedenisvermelding, werd een PIPELINE_STATE-vlag op de waarde DIRTY gezet om aan te geven dat de pipe een herberekening nodig zou hebben, en werd er een gtk_widget_queue_draw() verstuurd die, zoals de naam al doet vermoeden, Gtk vraagt om de hoofdvoorvertoning van de donkere kamer en de navigatieminiatuur opnieuw te tekenen, maar op een asynchrone manier (begrijp: wanneer het er tijd voor vindt, nadat alles wat eerder gestart is voltooid is). Dit zal later zijn belang hebben.
Het kostte me heel veel tijd om erachter te komen hoe de pipeline eigenlijk werd gestart, want geen enkel stuk code dat aan modules en pipeline was gekoppeld bevatte iets wat zei „ga dat berekenen". Met andere woorden, geen enkele module-code bevatte enige expliciete herberekeninstructie.
Ik moest de pipeline-code van de andere kant af reverse-engineeren, op zoek naar hoe de pipeline zou kunnen worden gestart, en elke optie greppen, tot ik het onuitspreekbare doorhad: de eerste generatie Darktable-ontwikkelaars had een callback-functie gekoppeld aan het redraw-event op de hoofdvoorvertoning van de donkere kamer en de navigatieminiatuur, maar op een volstrekt niet-gerelateerde plek in de code. In die GUI-callback werd de waarde van de PIPELINE_STATE-vlag gecontroleerd, en werd ofwel de backbuffer-pixmap rechtstreeks naar de widget verzonden als de vlag VALID was, ofwel een pipe-herberekening aangevraagd als de vlag DIRTY was, en die herberekening vroeg zelf een gtk_widget_queue_draw() aan na voltooiing.
Deze methode heeft één verdienste: het is lui coderen. Verder heeft het een hoop nadelen en problemen:
- het is niet dev-vriendelijk, vooral niet in een softwareproject waar het greppen van code en commentaar alle documentatie is waar we van kunnen dromen. Het kostte vele uren om de logica te begrijpen via programma-archeologie. Als er een opdracht wordt uitgegeven, wil ik
command_issued()lezen op de juiste plek in de code, want C is al moeilijk genoeg te volgen zonder er raadsels doorheen te mengen bij het debuggen. - aangezien
gtk_widget_queue_draw()(in het slechtste geval tweemaal aangeroepen) alleen aan de wachtrij wordt toegevoegd en asynchroon wordt verwerkt, voegt het elke vertraging toe die Gtk zou kunnen ondervinden (terwijl het andere stukjes van de GUI of eerdere frames verwerkt) voordat er ook maar enige pipeline-herberekening wordt gestart, wat onnodig is aangezien de pipeline in zijn eigen thread parallel leeft, - de grote MIDI-turducken, die luistert naar aanwijs-, toetsenbord- en MIDI-events om sneltoetsen te verdelen, leek de globale GUI overbelast te hebben met listeners die over alle bekende sneltoetsen loopten, waardoor Gtk zodanig vertraagde dat het merkbaar werd,
- het verhindert dat welk kill-switch-mechanisme dan ook nuttig is, zowel vanwege vertragingen als omdat het lezen van vlaggen verweven was met thread-locks (en race condities). Bovendien zou het wachten om de thread-lock (mutex) van de pipeline te verwerven de GUI-thread gedurende de bijbehorende tijd bevriezen, wat waarschijnlijk een van de oorzaken was van de schuifregelaar-vertraging voordat zijn positie werd bijgewerkt,
- de geketende aanroepen naar de callback van het
redraw-event, viagtk_widget_queue_draw(), bevorderden „eindeloze" haperende lussen van (nutteloze) tussenliggende hertekeningen die mensen met trage computers meer leken te treffen dan mensen met krachtige beesten. Die waren bijzonder moeilijk te reproduceren, afhankelijk van de hardwareprestaties, dus je kunt fora vinden waar mensen ervan overtuigd zijn dat Darktable de traagste software ooit is, terwijl anderen uitstekende prestaties melden.
Dus loste ik de hele logica op door:
- de
redraw-callback dom te maken (welke pixmap-buffer dan ook beschikbaar was onvoorwaardelijk te tekenen), - expliciete pipeline-herberekeningen af te handelen in de module- en geschiedeniscode, waarbij de pipeline-herberekeningen om een widget-hertekening vragen na voltooiing van de pipeline, (ja, het is meer code, en het is bewerkelijk, maar nu kun je herberekeningen handmatig optimaliseren — prestaties zijn belangrijk),
- de speciale afhandeling van „dubbele" geschiedenisitems te verwijderen (wat tot enige vervuiling leidt bij het werken met maskers, dit zal later moeten worden opgelost).
Je zou kunnen denken dat dat een opgelost probleem was en een klus goed geklaard, maar dan laat je de genieën van Darktable buiten beschouwing.
Kijk, de modules crop en perspective zijn speciale modules: als je ze opent, wordt er een „bewerkingsmodus" ingeschakeld die elke uitsnijding uitschakelt om het volledige beeld te tonen. Dit is nodig om het uitsnijkader te verslepen (of andere posities aan te passen) vanuit de hoofdvoorvertoning, over het volledige originele beeld. Het probleem is dat er geen expliciete manier was om een pipe-herberekening aan te vragen… anders dan een nieuw geschiedenisitem toe te voegen. Dus voegden de modules een nep-geschiedenisitem toe (later teruggedraaid) alleen om de pipe ongeldig te maken en de functie gtk_widget_queue_draw() aan te roepen. Maar toen vervuilde dat de geschiedenisstapel met „lege" stappen, dus voegde een andere gast een speciaal afhandelingsgeval toe dat geschiedenisstappen samenvoegde als er geen parameterwijzigingen plaatsvonden. Maar toen bleek dat de geschiedenisstapel (uit de history-module, zoals opgeslagen in de database) de ongedaan maken/opnieuw uitvoeren-geschiedenisstapel niet volgt, wat bij gebruikers tot misverstanden leidt over wat ongedaan maken/opnieuw uitvoeren echt doet.
En dit, dames en heren, is hoe waardeloos ontwerp nog waardelozer ontwerp bevordert in een eindeloze wildgroei van waanzin.
Bedenk dat dit allemaal voortkomt uit de noodzaak om de pipe-kill-switch te laten werken, zodat je een herberekening halverwege kunt onderbreken als je weet dat de uitvoer ervan toch weggegooid zal worden. Dus daarvoor moest ik het herberekenverzoek uit de Gtk-code halen en het overal aanroepen waar nodig. Maar toen moest ik de logica voor het bijwerken van de pipeline in de modules crop, perspective en rotatie, liquify en borders opnieuw bedraden, en ik moet retouch nog steeds oplossen (wat de ergste PITA van het stel is).
Naast dat het duidelijker leesbaar wordt en het mogelijk wordt om de aanroepen te optimaliseren, start de huidige logica de pipe ook buiten de GUI-thread, zonder te wachten tot Gtk alsjeblieft tijd vindt om het frame opnieuw te tekenen. Zoals gewoonlijk zullen mensen met een krankzinnige CPU qua prestaties weinig tot geen voordeel merken, wat waarschijnlijk de reden is waarom dit in het Darktable-team überhaupt geen kwestie is.
Aflevering 6: de achterstallige rente op de technische schuld afbetalen
Op dat punt had ik de pipeline-herberekeningen dus expliciet gemaakt vanuit de modules en GUI-regelaars, en ze spaarzaam verdeeld (wat het voordeel is van ze expliciet verdelen). En toch merkte ik dat het spelen met modules die laat in de pipe komen traag was. Sterker nog, het uitvoeren van ansel -d perf liet zien dat
de hele pipeline, beginnend bij de module demosaicing, werd herberekend, ook al was ik bezig met een late module die zijn invoer haalde uit color balance.
Darktable heeft altijd al een pixelcache gehad. Die slaat in feite de tussenliggende toestanden van het beeld op, tussen modules in. Dus als pipe-herberekeningen begonnen vanaf ver onder de huidige module, betekende dat dat hij grotendeels nutteloos was. Het bleek dat de cache slechts 8 cachelijnen gebruikte, wat de krankzinnige hoeveelheden RAM van tegenwoordig echt onderbenut. Maar dit verhogen naar 64 hielp niet tegen cache misses: de cache was nog steeds grotendeels nutteloos, en het grootste deel van de pipe werd nog steeds herberekend.
We moeten hier even pauzeren. Zelfs een werktuigbouwkundig ingenieur zonder degelijke programmeeropleiding zoals ikzelf weet wat een LRU-cache is:
- je maakt een vaste lijst met slots (cachelijnen),
- zodra je iets hebt om te cachen, wijs je een geheugenbuffer van tevoren bekende grootte toe aan een van die slots en geef je hem een unieke identificator. Dat kan een checksum, een willekeurige hash of zelfs een tijdstempel zijn, hij hoeft alleen altijd op dezelfde manier klaargemaakt te worden en tot iets unieks te leiden,
- wanneer je gegevens nodig hebt die bij een bepaalde unieke identificator horen, doorzoek je de lijst met slots en kijk je of die ID bekend is:
- als dat zo is, haal je de bijbehorende buffer op,
- als dat niet zo is:
- als je nog lege slots hebt, maak je de bijbehorende buffer aan en kopieer je de gegevens voor later hergebruik,
- als dat niet zo is, maak je het oudste slot leeg en hergebruik je het om je nieuwe gegevens onder te brengen.
In dat proces hoef je alleen de grootte van de buffers en de ID’s te kennen. Het is heel algemeen, je kunt van alles cachen, zelfs verschillende objecten, je cache hoeft zich niet bewust te zijn van de inhoud, zelfs niet van hoe de ID’s worden gegenereerd. Het is schoon, het is elegant, het is bescheiden, het is generiek, ik zou het mijn leven toevertrouwen omdat het veel robuuster is dan welk beveiligingssysteem je ook in moderne auto’s vindt.
Dus als iets dat eenvoudigs niet werkt, komt dat meestal omdat iemand iets „slims" heeft geprobeerd en gefaald heeft. Wat het Darktable-team in dat geval doorgaans doet, is zich een weg switch case-en door alle pathologische randgevallen en er iets nog ingewikkelders van maken (door alle uitzonderingen handmatig af te handelen met heuristieken), gewoon om ervoor te zorgen dat niemand later een kans heeft om de grondoorzaak van de fout te vinden.
Er waren bijvoorbeeld pogingen om de prioriteit van de cachelijnen te herwegen om te zorgen dat de module vóór de module die momenteel in de GUI wordt bewerkt, gecachet werd. Het werkte niet alleen niet, maar het versterkte ook de banden tussen pipeline-code en GUI-code, op een manier die niet eens thread-safe was (wat de reden is waarom het niet werkte). GUI-zaken zouden aan de invoer- en aan de uitvoerkant van de pipeline-berekeningen moeten plaatsvinden, niet ertussenin, want nogmaals, verschillende threads, maar ook schendt het het modulariteitsprincipe (houd programmalagen zoveel mogelijk gescheiden en afgebakend), en deze software moet ermee stoppen om alles van alles afhankelijk te maken.
Nogmaals, het kostte me 8 maanden, inclusief verplichte pauzes van die complete puinhoop, om tot de bodem van het probleem te komen op een manier die tot een vereenvoudigende oplossing leidt. En ik zal de bevindingen op een lineaire manier presenteren, als een verhaal, maar houd in gedachten dat ik dingen op een vage en willekeurige manier begon te ontdekken omdat het allemaal verspreid zit in de broncode, dus het zal er minder rommelig uitzien dan het werkelijk was.
We beginnen met de unieke ID. Wat vertegenwoordigt de toestand van een module werkelijk op een unieke manier? Nou, een „cryptografische" checksum van zijn interne parameters. Cool, dus Darktable had dat al lang geïmplementeerd. Behalve dat het geen rekening hield met het instantienummer van de module, en onderweg allerlei if-en afhandelde. Niet volledig, niet robuust, zelfs niet nodig. Hash alles, de hash zal de toestand van de variabelen vertegenwoordigen.
Ja, maar modules kunnen worden herordend, dus hoe zorgen we voor de pipeline-volgorde? Nou, je neemt alle hashes van alle modules, in pipeline-volgorde, en begint lineair op te tellen. Prima. Behalve dat Darktable er eigenlijk 2 van had, één voor GUI-doeleinden die vanaf het einde van de pipeline begon (dus in omgekeerde volgorde), één voor pipeline-doeleinden, in de pipeline-volgorde maar ontoegankelijk vanuit de GUI (bijvoorbeeld… om een histogram te krijgen), en wederom mengden beide dat met allerlei controles om speciale gevallen af te handelen (kleurenpipet, maskervoorvertoning, enz.).
Om nog maar te zwijgen van het feit dat de interne toestand van de module niet verandert of je nu in de volledige voorvertoning of in de navigatieminiatuur zit, in de donkere kamer. En toch werd de checksum volledig tweemaal herberekend, eenmaal voor elke pipeline. Maak daar eigenlijk maar vier keer van, aangezien er ook nog de GUI-checksum is (voornamelijk gebruikt voor de modules perspective en retouch)
En, last but not least, wanneer je in de donkere kamer bent ingezoomd, wordt alleen het zichtbare deel van het beeld berekend (het Region Of Interest, ofwel ROI), wat betekent dat we in ons cachemechanisme moeten bijhouden waar we ons in het beeld bevinden. Maar dat was volledig weggelaten uit de checksum. Grote bug hier, en oud.
Dus, hoe slaagde Darktable er tóch in om te „werken", vraag je je af?
Nou, door de cache min of meer volledig leeg te spoelen bij elke pathologische bewerking: zoomen, pannen, maskervoorvertoning, kleurenpipet, het inschakelen/uitschakelen van de bewerkingstoestand van de modules crop en perspective. Dat is een manier om met consistentie om te gaan zonder met consistentie om te gaan: fik het op. Waardoor hij grotendeels nutteloos werd, zoals de zeer lage cache-hitstatistieken laten zien (start gewoon ansel -d dev om het te tonen).
Hoe heb ik het probleem opgelost?
- Wanneer er een nieuwe module-geschiedenisvermelding wordt toegevoegd, wordt de parameter-checksum berekend, waarbij rekening wordt gehouden met parameters, maskers, mengopties, instantienummer, volgorde in de pipeline, enz. Dit betekent dat alle pipelines hier dezelfde checksum/ID delen (een mogelijk toekomstig gebruik zou zijn om die in de database op te slaan),
- Voordat een pipeline wordt berekend, berekenen we de globale checksum van alle modules, van begin tot eind, rekening houdend met de maskerweergavetoestand, de checksum van voorgaande modules, en de ROI (grootte en coördinaten). Deze checksum kan later direct worden benaderd, zonder aanvullende berekening.
- De cache heeft te maken met deze globale checksum, en alleen dat. Geen ifs, geen maren, geen heuristieken, geen condities, geen omwegen.
- Modules kunnen een cache-bypass aanvragen, bijvoorbeeld bij gebruik van de kleurenpipet. Dit besmet latere modules in de pipeline voordat de pipe wordt berekend, zodat de cacheloze toestand vroeg bekend is en geen invloed heeft op stroomopwaartse modules. Dat zou slechts een omweg moeten zijn totdat kleurenpipetten daadwerkelijk cachelijnen rechtstreeks kunnen gebruiken, en zou kunnen worden hergebruikt voor toekomstige modules die niet-standaard dingen doen (schilderen?).
Voordelen:
- De module-gewijze, interne checksum wordt eenmaal berekend voor alle pipelines,
- Omdat de pipeline-gewijze, globale checksum van elke module bekend is voordat de pipeline-herberekening wordt gestart:
- kan hij ook worden gebruikt voor GUI-synchronisatie, dus voegde ik beide Darktable-checksums samen tot één,
- is hij constant binnen het bereik van de pipeline, wat het mogelijk maakt om cachelijnen te delen tussen meerdere pipelines (bijvoorbeeld demosaicing en ruisonderdrukking) met beperkte thread-lock-problemen1
- Modules die rare dingen doen hebben een uniforme en voorspelbare manier om een cache-bypass aan te vragen vanuit GUI-events, mochten ze die nodig hebben.
Deze logica is niet alleen efficiënter (minder berekeningen), hij is ook eenvoudiger en kan worden uitgebreid voor interessante functies. Vanuit het cacheperspectief hebben we met niets anders dan een checksum te maken, elke module-toestand van belang is erin verweven.
Maar, wat belangrijker is, de cache is eindelijk nuttig, vooral bij het heen en weer gaan in de bewerkingsgeschiedenis, bij het gebruik van ongedaan maken/opnieuw uitvoeren, of bij het inschakelen/uitschakelen van modules. De algehele responsiviteit van de GUI is veel beter.
Ik weet zeker dat er onontdekte valkuilen en details zijn die ik vergeten ben opnieuw te bedraden naar de nieuwe logica, en de module retouch is nog steeds grotendeels kapot, maar aanpassen aan iets dat zo eenvoudig is zou haalbaar moeten zijn.
Ondertussen in Darktable 4.8
- De pipeline-checksum wordt berekend tijdens de pipeline-runtime, dus hij is onbekend erbuiten,
- Daardoor hebben ze de GUI- versus pipeline-checksums niet gededupliceerd… veel succes met het opsporen van inconsistenties tussen beide in de toekomst,
- Hun code voor cache-afhandeling is meer dan twee keer zo groot als de mijne en gebruikt heuristieken (over het type pipeline, soort module, toestand van maskerweergave, gebruik van kleurenpipet, en handmatig in modules gedefinieerde caching-hints) om problemen te omzeilen. De cache is niet inhoud-agnostisch meer en veel succes met het debuggen van deze spaghetti.2
- Ze berekenen de (interne) module-parameter-checksum nog steeds volledig tweemaal, eenmaal voor elke pipeline,
- Het kostte hen bijna 2 jaar om daar te komen (sinds de 4.0-versie),
- Ik zou graag hun cache-hits/misses-statistieken zien (wil ik mijn PTSS nieuw leven inblazen door die software ooit nog te openen? Ik sla over, dank je).
- Mensen die denken dat het hebben van meer apen die met hun handen in de lucht zwaaien betere kwaliteit garandeert, zouden moeten stoppen met denken.
Conclusie
De hoeveelheid bestede tijd en recent kapotgemaakte rotzooi die opgelost moest worden om daar te komen was regelrecht onverdraaglijk, maar het werd verergerd door code die op een niet-modulaire manier verspreid zit zonder een duidelijk onderscheid tussen wat tot de (G)UI behoort, wat tot de backend behoort, wat tot module-geschiedenissen behoort en wat tot pipeline-nodes behoort. Het cache-gedoe kostte alleen al 8 maanden, voornamelijk archeologie en reverse-engineering, bovenop wat er al gedaan was aan GUI-regelaars en expliciete pipeline-herberekeningen.
Er zijn nog steeds problemen op te lossen:
- het aantal beschikbare cachelijnen is een gebruikersvoorkeur en controleert niet op het beschikbare geheugen dat nog op het apparaat over is,
- de histogram/scopes-module is grotendeels kapot door het ontwerp, omdat het werd afgehandeld via speciale heuristieken (nu verwijderd) op een module die onzichtbaar is in de GUI (
gamma.c). De nieuwe logica maakt het mogelijk om hem geforceerd te cachen en de cachelijn op te halen vanuit de GUI-thread. - de interne histogrammen van de module worden niet onmiddellijk getekend bij het binnengaan van de donkere kamer,
- de afhandeling van kleurenpipetten zou kunnen worden vereenvoudigd en eleganter gemaakt,
- de afhandeling van geschiedenis heeft nog steeds enkele randgevallen.
Aangezien ik echter weiger om ook maar iets te „repareren" als mijn reparatie de dingen niet eenvoudiger maakt, begint die strategie zich uit te betalen omdat de code een stuk lineairder is, met minder gevallen om te testen, en uiteindelijk iets sneller. Naarmate ik vorder, wordt hij langzaam leesbaarder en beter repareerbaar. Dan is het natuurlijk zo dat het schudden aan de kern van de software in die mate onvermijdelijk dingen kapotmaakt (die niet zouden moeten breken als de code modulair was).
Daar komt de legitieme vraag: waarom moeite doen om de lelijke erfenis van Ansel/Darktable te repareren en niet overstappen op iets beters, snellers en glimmenders (zoals Vkdt)? Nou, Vkdt (of wat dan ook nieuws) zal een ruw prototype blijven, dat concurreert met andere ruwe prototypes (dat is Open Source in een notendop), jaren verwijderd van een algemeen bruikbaar product. Nog een onafgemaakt/half-bakken prototype aan het landschap toevoegen zal geen goed doen. Het zou fijn zijn om voor de verandering eens iets te hebben dat niet slonzig is en redelijk afgewerkt. Bovendien is de (zeer) oude code van Darktable schoon en solide (nou ja, grotendeels), het zijn alleen de afgelopen paar jaar die een wending naar het waardeloze hebben genomen. git blame toont altijd dezelfde 3 namen op de echt waardeloze regels, tot op het punt dat ik mezelf soms betrap op het automatisch verwijderen van de betreffende regels toen ik zag wie ze geschreven had, uit gewoonte.
Er is ook de vrees dat, hoe snel Vulkan Vkdt ook maakt, wat Darktable werkelijk waardeloos maakt slechte beslissingen, slechte prioriteiten, programmeerfouten en niet-geleerde lessen zijn, en als die fouten over Vkdt worden herhaald, kan het langer duren om de gevolgen te beseffen met meer paardenkracht, maar uiteindelijk zullen de dingen dezelfde kant op gaan. Meer middelen hebben maakt het betaalbaarder om dom te zijn… tot het niet meer zo is en je beseft hoe klem je zit.
Translated from English by : Claude. In case of conflict, inconsistency or error, the English version shall prevail.
The source code actually has a 10-years-old
TODOcomment detailing how to do that. ↩︎It should be noted that “my” cache code is actualy pretty much how Roman Lebedev and Johannes Hanika wrote it 10 years ago. I simplified a couple of things, mostly removing stuff added since then, and added nothing of my own, because it’s a Garbage In/Garbage Out situation where you should rather clean your input rather than trying to handle any corner case internally through unlegible heuristics. ↩︎