Valori

Se non è rotto, non aggiustarlo

Troppa parte del «design» di Darktable è iniziata con «sarebbe bello se potessimo …». Ti dico io cosa è bello: appendere al più presto belle tue fotografie alle pareti. Le arti visive non sono arti performative (come la musica o il teatro), quindi conta solo il risultato. Tutto ciò che viene prima è overhead, e di norma vuoi ridurlo al minimo. Questo non significa che il processo non possa essere apprezzato di per sé. Tuttavia, per godere del processo, devi padroneggiare i tuoi strumenti e piegarli alla tua volontà, altrimenti non fai che combatterli e l’intero processo si riduce a frustrazione. Il problema è che il «design» di Darktable si sforza troppo di essere diverso tanto per esserlo.

In questo processo di aggiunta di «nuova roba fica», Darktable ha rotto scorciatoie da tastiera e molti comportamenti di base della GUI, sostituendo codice pulito con spaghetti e aggiungendo altra confusione nella GUI senza mai sfoltire nulla.

Ansel ha un processo di progettazione esplicito che parte obbligatoriamente da problemi definiti incontrati da utenti definiti. Si scopre che la quantità di codice da scrivere è inversamente proporzionale alla quantità di riflessione che hai dedicato alla tua soluzione, tipicamente per individuare il problema alla radice a partire da ciò che ti dicono gli utenti, e trovare il percorso più semplice verso la soluzione (che spesso non è nemmeno una soluzione software…).

Ma i bug non ti aspettano nella riflessione, aspettano solo nel codice che hai scritto. Quindi, più pensi, meno scrivi codice, meno onere di manutenzione crei per te stesso in futuro. Ma ovviamente… devi avere abbastanza tempo per ragionare bene sulle cose. In sostanza, questo significa addio all’hacking amatoriale del sabato pomeriggio!

Non estenderlo se prima non riesci a semplificarlo

Gran parte dell’hacking di Darktable è stata fatta copiando e incollando codice, da altre parti del software, o persino da altri progetti, soprattutto perché i collaboratori non hanno il tempo né le competenze per intraprendere grandi riscritture. Questo provoca duplicazione di codice e aumenta la lunghezza delle funzioni, aggiungendo diramazioni interne e introducendo if e switch case annidati a volte su più di 4 livelli, rendendo la struttura e la logica più difficili da cogliere e i bug più difficili (e frustranti) da inseguire, oltre che più probabili.

In ogni caso, quando il codice responsabile delle funzioni esistenti non fa che crescere (a volte di un fattore 10 in 4 anni), si pongono seri interrogativi sulla manutenibilità futura, in un contesto in cui i collaboratori restano non più di un paio d’anni, e gli sviluppatori hanno un tempo limitato da investire. È semplicemente irresponsabile, perché sacrifica la manutenibilità a lungo termine per novità luccicanti.

Semplificare e generalizzare il codice, attraverso API pulite, prima di aggiungere nuove funzioni è un obbligo e Ansel accetta solo codice che io capisco personalmente e che ho le competenze per mantenere. KISS.

Logica di codifica di base

Le pull request che non soddisfano i requisiti minimi di qualità del codice non saranno accettate. Questi requisiti mirano a garantire manutenibilità e stabilità a lungo termine imponendo codice chiaro, leggibile e strutturato con una logica semplice.

  1. Le procedure devono essere suddivise in funzioni unitarie e riutilizzabili, quando possibile. Fanno eccezione le procedure lineari specializzate (senza diramazioni) che svolgono compiti troppo specifici per essere riutilizzati altrove, ma in questo caso usa i commenti per suddividere le procedure in «capitoli» o passaggi facilmente individuabili e comprensibili.
  2. Le funzioni dovrebbero svolgere un solo compito alla volta. Per esempio, il codice della GUI non dovrebbe essere mescolato con codice SQL o di elaborazione dei pixel. Getter e setter dovrebbero essere funzioni distinte.
  3. Le funzioni dovrebbero avere un solo punto di ingresso e un solo punto di uscita (return). Le uniche eccezioni accettate sono un ritorno anticipato se il buffer di memoria su cui la funzione dovrebbe operare non è inizializzato oppure se un mutex lock di thread è già acquisito.
  4. Le funzioni dovrebbero avere nomi leggibili ed espliciti e nomi di argomenti che ne dichiarino lo scopo. I programmi sono fatti per essere letti dagli esseri umani; se programmi per la macchina, fallo in binario.
  5. Le funzioni possono annidare al massimo 2 strutture condizionali if. Se servono più di 2 if annidati, la struttura del tuo codice deve essere rivalutata e probabilmente suddivisa in funzioni più granulari.
  6. if dovrebbe testare solo casi uniformi come lo stato o il valore idealmente di una (ma forse più) variabile/i dello stesso tipo. Se occorre testare casi non uniformi (come IF user param IS value AND picture buffer IS initialized AND picture IS raw AND picture HAS embedded color profile AND color profile coeff[0] IS NOT NaN), dovrebbero essere delegati a una funzione di controllo che restituisce un gboolean TRUE o FALSE e nominata in modo appropriato, così che gli altri sviluppatori comprendano lo scopo del controllo senza ambiguità a una lettura superficiale del codice, come color_matrix_should_apply(). Il codice di diramazione sarà quindi if(color_matrix_should_apply()) pix_out = dot_product(pix_in, matrix);
  7. I commenti dovrebbero indicare perché hai fatto quello che hai fatto, come le tue assunzioni di base, le tue ragioni e qualsiasi riferimento accademico o di documentazione che hai usato come base (DOI e URL dovrebbero esserci). Il tuo codice dovrebbe dire esplicitamente cosa hai fatto. Se ti ritrovi a dover spiegare nei commenti cosa fa il tuo codice, di solito è segno che il tuo codice è mal strutturato, che variabili e funzioni sono mal nominate, ecc.
  8. Le soluzioni rapide di ripiego che nascondono i problemi invece di affrontarli alla radice non saranno accettate. Se ti interessano, potresti considerare di contribuire invece a darktable a monte. Le uniche eccezioni saranno se i problemi sono bloccanti (fanno crashare il software) e non è stata trovata una soluzione migliore dopo una discreta quantità di tempo dedicato alla ricerca.
  9. Ricorda sempre che il codice migliore è il più semplice. KISS. Per raggiungere questo obiettivo, di solito è meglio scrivere codice da zero piuttosto che cercare di combinare pezzi di codice esistente con pesanti copia-incolla.

In un mondo ideale, ogni PR seguirebbe le migliori pratiche dei design pattern .

Alcune perle di saggezza a caso da internet:

Tutti sanno che il debug è due volte più difficile che scrivere un programma la prima volta. Quindi, se sei il più intelligente possibile quando lo scrivi, come farai mai a farne il debug? — Brian W. Kernighan
Qualsiasi sciocco può scrivere codice che un computer capisce. I bravi programmatori scrivono codice che gli esseri umani possono capire. — Martin Fowler, Refactoring: Improving the Design of Existing Code
Scrivi sempre codice come se la persona che finirà per mantenere il tuo codice fosse uno psicopatico violento che sa dove abiti. — John Woods
Ogni volta che devo pensare per capire cosa sta facendo il codice, mi chiedo se posso rifattorizzare il codice per rendere quella comprensione più immediatamente evidente. — Martin Fowler, Refactoring: Improving the Design of Existing Code
Il codice è cattivo. Marcisce. Richiede manutenzione periodica. Ha bug che devono essere trovati. Le nuove funzioni comportano l'adattamento di codice vecchio. Più codice hai, più posti ci sono in cui i bug possono nascondersi. Più tempo richiedono i checkout o le compilazioni. Più tempo serve a un nuovo dipendente per capire il tuo sistema. Se devi rifattorizzare, c'è più roba da spostare.
Il codice è prodotto dagli ingegneri. Produrre più codice richiede più ingegneri. Gli ingegneri hanno costi di comunicazione pari a n^2, e tutto quel codice che aggiungono al sistema, pur ampliandone le capacità, aumenta anche un intero paniere di costi. Dovresti fare tutto il possibile per aumentare la produttività dei singoli programmatori in termini di potenza espressiva del codice che scrivono. Meno codice per fare la stessa cosa (e possibilmente meglio). Meno programmatori da assumere. Meno costi di comunicazione organizzativa.
— Rich Skrenta 
I bravi programmatori scrivono buon codice. I grandi programmatori non scrivono codice. I programmatori zen cancellano codice. — John Byrd 

Logica di codifica specifica del C

Ansel così come darktable sono scritti in C. Questo linguaggio è pensato per programmatori avanzati per scrivere bug veloci in sistemi operativi e applicazioni di livello di sistema. Dà troppa libertà di fare cose dannose e non può essere sottoposto a debug prima di eseguire il programma, o di scrivere i tuoi test (che possono a loro volta contenere bug, o possono introdurre una distorsione nel tipo di bug che lasciano passare, e comunque nessuno scrive test). Eppure la maggior parte dei collaboratori non è formata nel C, molti di loro non sono nemmeno programmatori professionisti (anche se i programmatori C professionisti probabilmente non dovrebbero essere lasciati avvicinare alle applicazioni per l’utente finale), quindi il C è un linguaggio pericoloso per qualsiasi app open source.

Il C ti permetterà di scrivere in buffer che non sono stati allocati (con conseguente errore di segfault) e ti permetterà di liberarli più di una volta, ma non libererà i buffer quando non servono più (con conseguenti memory leak se hai dimenticato di farlo manualmente). Il problema è che, poiché l’allocazione/liberazione dei buffer può essere lontana (nella vita del programma come nel codice sorgente) da dove li usi, è facile combinare pasticci. Il C ti permetterà anche di eseguire il cast di qualsiasi puntatore a qualsiasi tipo di dato, il che apre la porta a molti errori del programmatore e alla corruzione dei dati. I metodi nativi di gestione delle stringhe non sono sicuri (per ragioni che non mi sono mai preoccupato di capire), quindi dobbiamo usare quelli di GLib per prevenire exploit di sicurezza.

In sostanza, il C ti rende il tuo stesso peggior nemico, e sta a te osservare regole di sicurezza la cui saggezza diventerà chiara solo una volta che le avrai infrante. Proprio come i bug in un programma C. Considera che scrivi il tuo codice per essere letto da sprovveduti che non hanno mai programmato in C prima.

Devi anche tenere presente che il compilatore farà la maggior parte delle ottimizzazioni al posto tuo, ma sarà estremamente conservativo al riguardo. La regola generale è: se il tuo codice è facilmente comprensibile da un essere umano (logica semplice), sarà compreso correttamente dal compilatore, che adotterà le misure di ottimizzazione appropriate. Al contrario, le ottimizzazioni manuali nel codice, che producono codice criptico presunto più veloce sui sistemi a thread singolo, di solito si ritorcono contro e producono programmi più lenti dopo la compilazione.

Pattern e strutture

  1. I cicli for sono riservati all’iterazione su array di dimensione nota in anticipo, così che il numero di passi del ciclo sia noto. Estendendo questa logica, possono essere usati anche per iterare sugli elementi GList * (che non hanno una proprietà di dimensione dato che sono allocati dinamicamente), sebbene questo controlli se ogni elemento (GList *)->next non sia NULL. I cicli for generalmente non dovrebbero usare istruzioni break o return all’interno del loro flusso di controllo, a meno che il ciclo non stia cercando un elemento specifico all’interno dell’array e lo restituisca non appena viene trovato. Se il tuo ciclo ha una condizione di arresto, usa while.

  2. Il C non è un linguaggio orientato agli oggetti, ma puoi e dovresti usare la logica OO quando è pertinente in C usando strutture per memorizzare i dati e puntatori ai metodi, quindi getter e setter  uniformi per definire e accedere ai dati.

  3. strutture come while, for, if o switch non dovrebbero essere annidate su più di 3 (e preferibilmente 2) livelli. Usa le funzioni se ciò accade:

     1// Sbagliato
 2void stuff(float *array, char *output)
 3{
 4  if(condition)
 5  {
 6    for(int i = 0; i < 5; i++)
 7    {
 8      if(array[i] > 1.f)
 9        array[i] = ...
10    }
11    output = "true";
12  }
13  else
14  {
15    ...
16  }
17}
18
19// Corretto
20char *_process(float *array)
21{
22  for(int i = 0; i < 5; i++)
23  {
24    if(array[i] > 1.f)
25      array[i] = ...
26  }
27  return "true";
28}
29void stuff(float *array, char *output)
30{
31  if(condition)
32  {
33    output = _process(array);
34  }
35  else
36  {
37    output = _something_else(array);
38  }
39}

  4. Lunghe sequenze di controlli dovrebbero essere messe in una funzione che restituisce gboolean indicando chiaramente cosa stiamo controllando, così nelle procedure otteniamo:

     1gboolean _is_raw(dt_image_t *image)
 2{
 3  return (image->flag & DT_RAW == DT_RAW) &&
 4         (image->buffer != NULL) &&
 5         strcmp(image->ext, "dng");
 6}
 7
 8void stuff(dt_image_t *image)
 9{
10  if(_is_raw(image))
11    ...
12  else if(_is_raster(image))
13    ...
14}

    invece di

    1if((image->flag & DT_RAW == DT_RAW) && (image->buffer != NULL) && strcmp(image->ext, "dng"))
2  ...
3else if(...)
4  ...

  5. Accedi sempre ai dati dei buffer usando la sintassi in stile array, dal loro puntatore di base, invece di usare puntatori non costanti su cui esegui aritmetica. Per esempio, fai:

    1float *const buffer = malloc(64 * sizeof(float));
2for(int i = 0; i < 64; i++)
3{
4  buffer[i] = ...
5}

    Non fare:

    1float *buffer = malloc(64 * sizeof(float));
2for(int i = 0; i < 64; i++)
3{
4  *buffer++ = ...
5}

    Quest’ultima versione non solo è meno chiara da leggere, ma impedirà la parallelizzazione e le ottimizzazioni del compilatore perché il valore del puntatore dipende dall’iterazione del ciclo e dovrebbe essere condiviso tra i thread, se ce ne sono. La prima versione porta a una logica di accesso alla memoria indipendente dall’iterazione del ciclo e può essere parallelizzata in sicurezza.

  6. L’uso di incrementi inline di variabili (vedi un esempio da incubo qui ) è severamente vietato, a meno che non sia l’unica operazione della riga. Sono un pasticcio che causa molti errori di programmazione. Questo è permesso:

    1uint32_t counter;
2for(int i = 0; i < 64; i++)
3{
4  if(array[i] > threshold)
5    counter++;
6}

  7. Le istruzioni case nella struttura switch non dovrebbero essere additive. Non fare:

     1int tmp = 0;
 2switch(var)
 3{
 4  case VALUE1:
 5  case VALUE2:
 6    tmp += 1;
 7  case VALUE3:
 8    do_something(tmp);
 9    break;
10  case VALUE4:
11    do_something_else();
12    break;
13}

    A una lettura superficiale, non sarà immediatamente chiaro che il caso VALUE3 eredita le clausole definite dai casi precedenti, specialmente in situazioni in cui ci sono più casi. Fai:

     1int tmp = 0;
 2switch(var)
 3{
 4  case VALUE1:
 5  case VALUE2:
 6    do_something(tmp + 1);
 7    break;
 8  case VALUE3:
 9    do_something(tmp);
10    break;
11  case VALUE4:
12    do_something_else();
13    break;
14}

    Ogni caso è autonomo e il risultato non dipende dall’ordine di dichiarazione dei casi.

  8. Ordina e memorizza le tue variabili in strutture che passi come argomenti di funzione invece di usare funzioni con più di 8 argomenti. Non fare:

     1void function(float value, gboolean is_green, gboolean is_big, gboolean has_hair, int width, int height, ...)
 2{
 3  ...
 4}
 5
 6void main()
 7{
 8  if(condition1)
 9    function(3.f, TRUE, FALSE, TRUE, 80, 90, ...);
10  else if(condition2)
11    function(3.f, FALSE, TRUE, TRUE, 80, 90, ...);
12  else
13    function(3.f, FALSE, FALSE, FALSE, 110, 90, ...);
14}

    Fai:

     1typedef struct params_t
 2{
 3  gboolean is_green;
 4  gboolean is_big;
 5  gboolean has_hair;
 6  int width;
 7  int height;
 8} params_t;
 9
10void function(float value, params_t p)
11{
12  ...
13}
14
15void main()
16{
17  params_t p = { .is_green = (condition1),
18                .is_big = (condition2),
19                .has_hair = (condition1 || condition2),
20                .width =  (condition1 || condition2) ? 80 : 110,}
21                .height = 90 };
22  function(3.0f, p);
23}

    Il primo esempio è tratto da darktable . Il copia-incolla delle chiamate di funzione è superfluo e la moltiplicazione degli argomenti posizionali rende impossibile ricordare quale sia quale. Inoltre non mostra quali argomenti siano costanti tra le diverse diramazioni, il che renderà difficile il refactoring. Il secondo esempio non è più conciso, tuttavia la struttura non solo rende la funzione più facile da chiamare, ma la dichiarazione della struttura permette di impostare esplicitamente ciascun argomento, con controlli inline se necessario. Anche la dipendenza degli argomenti di input dalle condizioni esterne è resa immediatamente chiara, e gli argomenti booleani sono impostati direttamente dalle condizioni, il che renderà il programma più facile da estendere in futuro e meno soggetto a errori di programmazione dovuti a fraintendimenti nella dipendenza delle variabili.

Ottimizzazioni OpenMP

I pixel sono essenzialmente vettori RGBA a 4 dimensioni. Dal 2004, i processori hanno capacità speciali per elaborare vettori e applicare Istruzioni Singole su Dati Multipli (SIMD). Questo ci permette di velocizzare i calcoli elaborando un intero pixel (SSE2) fino a 4 pixel (AVX-512) contemporaneamente, risparmiando molti cicli di CPU.

I compilatori moderni hanno opzioni di auto-vettorizzazione che possono ottimizzare il C puro, e la libreria OpenMP permette di fornire suggerimenti per migliorarla, a condizione che il codice sia scritto in modo vettorizzabile e usi alcune pragma.

Scrivere codice vettorizzabile: https://info.ornl.gov/sites/publications/files/Pub69214.pdf 

Migliori pratiche per l’auto-vettorizzazione:

  • evita diramazioni nei cicli che cambiano il flusso di controllo. Usa istruzioni inline come absolute = (x > 0) ? x : -x; così che possano essere convertite in maschere di byte in SIMD,
  • i pixel dovrebbero essere referenziati solo dal puntatore di base del loro array e dagli indici dei cicli, in modo da poter prevedere quale indirizzo di memoria viene acceduto solo dall’indice del ciclo,
  • evita di portare argomenti struct nelle funzioni chiamate nei cicli OpenMP, e scompatta i membri della struct prima del ciclo. La vettorizzazione non può essere eseguita sulle strutture, ma solo su scalari e array float e int. Per esempio:
     1typedef struct iop_data_t
 2{
 3  float[4] pixel;
 4  float factor;
 5} iop_data_t;
 6
 7float foo(float x, struct iop_data_t *bar)
 8{
 9  return bar->factor * (x + bar->pixel[0] + bar->pixel[1] + bar->pixel[2] + bar->pixel[3]);
10}
11
12void loop(const float *in, float *out, const size_t width, const size_t height, const struct iop_data_t bar)
13{
14  for(size_t k = 0; k < height * width; ++k)
15  {
16    out[k] = foo(in[k], bar);
17    // la funzione non vettorizzata sarà chiamata a ogni iterazione (costoso)
18  }
19}
    dovrebbe essere scritto:
     1typedef struct iop_data_t
 2{
 3  float[4] pixel DT_ALIGNED_PIXEL; // allinea su indirizzi a 16 bit
 4  float factor;
 5} iop_data_t;
 6
 7#ifdef _OPENMP
 8#pragma declare simd
 9#endif
10/* dichiara la funzione vettorizzabile e falla inline per evitare chiamate dall'interno del ciclo */
11inline float foo(const float x, const float pixel[4], const float factor)
12{
13  float sum = x;
14
15  /* usa una riduzione SIMD per vettorizzare la somma */
16  #ifdef _OPENMP
17  #pragma omp simd aligned(pixel:16) reduction(+:sum)
18  #endif
19  for(size_t k = 0; k < 4; ++k)
20    sum += pixel[k];
21
22  return factor * sum;
23}
24
25void loop(const float *const restrict in,
26          float *const restrict out,
27          const size_t width, const size_t height,
28          const struct iop_data_t bar)
29{
30  /* scompatta i membri della struct */
31  const float *const restrict pixel = bar->pixel;
32  const float factor = bar-> factor;
33
34  #ifdef _OPENMP
35  #pragma omp parallel for simd default(none) \
36  dt_omp_firstprivate(in, out, pixel, factor, width, height) \
37  schedule(simd:static) aligned(in, out:64)
38  #endif
39  for(size_t k = 0; k < height * width; ++k)
40  {
41    out[k] = foo(in[k], pixel, factor);
42  }
43}
  • se usi cicli annidati (ad es. ciclo sulla larghezza e sull’altezza dell’array), dichiara i puntatori ai pixel nel ciclo più interno e usa collapse(2) nella pragma OpenMP così che il compilatore possa ottimizzare l’uso di cache/memoria e suddividere il ciclo in modo più uniforme tra i diversi thread,
  • usa l’indicizzazione piatta degli array quando possibile (for(size_t k = 0 ; k < ch * width * height ; k += ch)) invece di cicli annidati larghezza/altezza/canali,
  • usa la parola chiave restrict sui puntatori a immagini/pixel per evitare l’aliasing ed evita operazioni in place sui pixel (*out deve essere sempre diverso da *in) così da non innescare dipendenze tra variabili tra i thread
  • allinea gli array su blocchi di 64 byte e i pixel su blocchi di 16 byte così che la memoria sia contigua e la CPU possa caricare intere linee di cache (ed evitare i segfault),
  • scrivi funzioni piccole e ottimizza localmente (un ciclo/funzione), usando OpenMP e/o le pragma del compilatore,
  • mantieni il tuo codice stupidamente semplice, sistematico ed evita l’aritmetica dei puntatori da furbetto perché porterà solo il compilatore a rilevare dipendenze tra variabili e aliasing dei puntatori dove non ce ne sono,
  • evita i cast di tipo nei cicli,
  • dichiara i puntatori di input/output come *const e le variabili come const per evitare il false-sharing nei cicli paralleli (usando la pragma OpenMP shared(variable)).

Formattazione del codice

  • Usa spazi invece di tab,
  • L’indentazione usa 2 spazi,
  • Rimuovi gli spazi finali,
  • { e } vanno su una riga a sé,

Linee guida

  1. Fai cose che padroneggi: sì, è bello imparare cose nuove, ma Ansel non è un sandbox, è un software di produzione, e non è il posto giusto per fare la tua formazione.
  2. KISS e sii pigro: Ansel non ha 50 sviluppatori full-time a disposizione, essere minimalisti sia nelle funzioni sia nel volume di codice è ragionevole e sensato per la gestione attuale, ma anche per la manutenzione futura. (KISS: keep it stupid simple).
  3. Fai come il resto del mondo: certo, se tutti si buttano dalla finestra, hai il diritto di non seguirli, ma la maggior parte dei problemi di UI/UX del software è già stata risolta da qualche parte e nella maggior parte dei casi ha senso semplicemente riutilizzare quelle soluzioni, perché la maggior parte degli utenti avrà già familiarità con esse.
  4. Programmare non è l’obiettivo: programmare è un mezzo per un fine, il fine è poter elaborare grandi volumi di fotografie in poco tempo raggiungendo al contempo l’aspetto desiderato su ciascuna fotografia. I compiti di programmazione vanno considerati overhead e dovrebbero essere ridotti al minimo, e il volume di codice è un peso per qualsiasi progetto.
Translated from English by : Claude. In case of conflict, inconsistency or error, the English version shall prevail.