Dubbio architetturale - Githubissues

Ciao,

scrivo qui ed in italiano, ma se è un problema, seghiamo tutto e rifaccio in inglese.

Introduzione

Mi sono recentemente lanciato nel refactoring del MAT plugin. Inizialmente gli obiettivi erano 2:

Threading model, rimuovere i vecchi RTThread e passare alle strategies
Rimuovere i DataPacket
Snellire e semplificare il processing dei canali migliorando tutta la parte di inizializzazione per evitare milioni di lookup a runtime usando, di fatto, un approccio ad observer

Concentriamoci sul punto 2. Si tratta di una strategia adottata con grande successo in contesti più semplici (i.e. altri plugins) in cui una volta identificato un channel processor dall'ID, di fatto si delega a lui la gestione dei samples raw, che vengono processati in una catena predeterminata dino a finire sul data bus Certo, con il plugin MAT era più sfidante... ma potevo non provare? E infatti...

NB: quando ho iniziato non avevo guardato il tuo codice di questo repo o quantomeno non lo ricordavo

Esempi semplici

Facendo il refactoring hanno cominciato a venir via pezzi come da un intonaco pieno di umidità. Partendo dalle cose semplici, al processor di un parametro slowrow viene d'ufficio attaccato un observer il cui compito è aggiornare lo SlowRowStorage. Quindi basta metti la cera, togli la cera dai DataPacket (metti e poi itera per togliere), basta anche con un VirtualChannelEngine con dentro le liste su cui fare lookup. Altro esempio semplice, al processor di un parametro regular viene d'ufficio attaccato un observer che scrive nel coverage engine

Virtuali

La cosa fittava anche per i virtuali, ovviamente. Già che c'ero, potevo anche rimuovere il gap di 2 secondi perché, ho pensato, se invece di un grande DataLog mi faccio tanti piccoli DataLog, uno per virtuale da usare come context, ecco che di fatto azzero praticamente il delay. Bottom line, con qualche altra piccola ottimizzazione, funziona e oggi ho deciso di riguardare questo tuo codice e, senza troppa sorpresa, ho scoperto che è praticamente uguale al mio.

Ci sono delle differenze, minime, ma che volevo condividere.

Io dalla parte interna del progetto virtual channel engine ho cercato di tirare via tutte le specificità MAT, tra cui slowrow, tu ho visto che invece lo hai lasciato dentro come parte strutturale. Mi chiedo solo se non abbia più senso avere un progetto generico e sopra questo fare una lib specifica. Me lo chiedo, ma non ho una risposta, perché è pur vero che si può sempre dire che slowrow è qualcosa che cambia lentamente e bon
Anche perchè il fatto che un virtuale sia slowrow sia nel mio vecchio codice che nel tuo si evince dai componenti. Che da una parte è ok, ma è pur vero che in tabella lo abbiamo scritto perché lo abbiamo già controllato. Quindi di nuovo: chiamante o engine?
Io dalla parte interna del progetto virtual channel engine non ho fatto arrivare gli ID di pubblicazione del dato. perché ho delegato al chiamante, mentre il virtual channel engine (l'equivalente circa nel tuo progetto del VirtualChannelBuilder) a me torna un valutatore che, dato un contesto, torna dei numeri. Questo lo rende ovviamente agnostico di tutto (compreso se è slowrow o no). Questo mi ha anche permesso di costruire valutataori senza nemmeno il name del virtuale. Ma solo un semplice .BuildEvaluator(string formula, Action<IVirtualChannelEvaluator> visitor)
Io ho ovviamente tenuto il DataLog, seppur multi istanza, tu qui fortunatamente no. Il tuo mi piace molto di più perché non ha le pages non avendo un concetto di deferred etc...
Al netto dell'implementazione di cui al punto precedente, io ho fatto una scelta diversa: l'observer sul componente scrive nel datalog e quando il datalog dice che ha una pagina pronta, allora io costruisco un contesto basto su quella page e la passo al valutatore e i valori da esso tornati li pubblico. È quindi il chiamante che si tiene l'ID. Di fatto il tuo virtual channel racchiude nello stesso oggetto responsabilità che io ho sparpagliato. Non credo che ci sia un giusto, ma c'è un nesso tra le scelte. Avere il datalog fuori scritto da altri, necessita di un contesto dinamico, il fatto che invece il tuo channel scriva da solo nel suo context, lo rende molto più stabile. Idee? Pareri?

Il caso delle costanti

Nel refactoring mi sono imbattoto in un caso simpatico. x = 3.13 Non è regular, ma non è nemmeno slowrow e soprattutto, nel mio modello a observer... non viene mai triggerato il calcolo e quindi la pubblicazione, non avendo nessun componente. Mi sembra che anche il tuo codice soffra di questo problema. Lo costruisce come sloworow, ma io chiamante da fuori come faccio a sapere che va triggerato a parte? Io l'ho risolta così. Visto che gli slowrow arrivano a pacchetti e i virtuali slowrow si sono sempre calcolati subito, ho migliorato il processo facendo segnare come dirty, il virtuale slowrow ogni volta che mi arriva un suo componente e poi quindi calcolo tutti i dirty a quel timestamp. Ho aggiunto qui anche una lista di valutatori di costante che vengono quindi triggerati dal calcolo per timestamp dei virtuali slowrow. Non è aggiustatissimo ma insomma... però la cosa da portarsi via è: l'ho agganciato a quelle costanti perché da fuorisapevo che era costante.

La mia sensazione è che il VirtualChannelBuilder debba stare nel chiamante, non nel progetto stesso, in modo che poi sia io chiamante a poter discriminare alcune cose. Mi aspetterei quindi un progetto VirtualEngine e poi sopra un progetto VirtualEngine.MAT che aggiunga e metta le proprie cose specifiche .

Sono andato lunghissimo

Grazie A

Ciao! Ci ho messo un po' a rispondere perche' sinceramente ricordavo poco

Prima di tutto, complimenti! Sono contento tu stia lavorando su questo, era una spina nel fianco e l'ultimo pezzo da attaccare. Sono anche contento di sentire che i pezzi vengono via come l'intonaco vecchio, segno che il lavoro preparatorio e' stato grande e che ora e' il momento di raccogliere i frutti :)

Vedo di rispondere un po' a tutti i punti, tu dimmi se ho saltato qualcosa o qualcosa e' poco chiaro e ci ritorno su. Le costanti: ah che ricordi! E' stato il mio primo svarione con TB, ricordi? Abbiamo dovuto fare almento 3 patch, e ci siamo fermati anche a fare retrospettiva su come mai. Credo che sia parte del motivo per cui avevamo proposto di usare c18 come ambiente di validazione. Comunque sia, si, il problema viene bellamente ignorato qui (e quindi va affrontato). Anche io pensavo di agganciare "in qualche modo"/"a qualche evento" l'output dei valori costanti; direi che agganciarlo a slowrow ha assolutamente senso. Usare i virtuali in questo modo e' un po' un'anomalia direi (i virtuali non hanno frequenza se non quella dei canali su cui si appoggiano - le costanti hanno frequenza 0) - the next best thing a "frequenza 0" sono gli slowrow ,quindi direi legittimo! Mi chiedo come Atlas risolva il problema (o se li supporta del tutto!).

Per il resto: non sono sorpreso nemmeno io che ci siano notevoli somiglianze. Tu evidenzi alcune differenze, che alla fine sono molto legate tra loro:

quello che hai fatto tu ha senso soprattutto se vuoi riusare il motore dei canali virtuali "altrove". Se ritieni che sia specifico MAT, allora va bene avere li' il concetto di slowrow, altrimenti e' meglio estrarlo in una libreria.
il punto precedente e questo insieme influenzano molto il design; il builder, se le specificita' MAT come slowrow sono in una lib a parte, deve per forza stare nel chiamante
questo a mio parere (ID di pubblicazione del dato) e gia' leggermente piu' opinabile: si potrebbe argomentare che un ID numerico e' gia' un'astrazione - purche' l'ID sia condiviso tra engine e chiamante, sei a posto. Detto questo, trovo che il tuo design sia piu' elegante perche' permette questo, ma anche altro (se una sorgente non ha ID numerici ma un diverso "tipo nativo", col tuo design non hai bisogno di una mappa di traduzione, avendo solo un valutatore e un contesto). Di contro, potrebbe essere piu' efficiente evitare troppo avanti-indietro (ma i parla di cose rilevabilli solo con micro-benchmarking - andrebbe misurato ed e' solo un possibile sospetto visto che non ho visto il tuo codice)
infatti mi chiedevo se avevi tenuto il DataLog o no... Io ho avuto il privilegio/l'obbligo di fare tutto da 0, che ha influenzato un po' la mia parte del design (a volte in bene, come in questo caso - vedi sotto per dettagli)
c'è decisamente un nesso tra le scelte...

Secondo me molti aspetti sono collegati e dovuti all'ambiente e al momento un cui abbiamo fatto i nostri design. Ricorda che io lo ho fatto in lockdown: zero accesso al codice esistente e solo la memoria ad aiutarmi. Non e' una giustificazione, solo un contesto per farti capire alcune scelte. Il fatto che sia molto orientato verso 1 componente, lasciando meno responsabilita' al chiamante, e' figlio di questo: avrei dovuto "immaginare" o mockare un chiamante, al rischio di sbagliare interazioni per aver perso qualche aspetto. Il fatto di avere concetti MAT (slowrow) dentro, l'uso di ID invece di tornare un valutatore, ovviamente il fatto di costruirmi le mie strutture dati invece di usare un datalog (punto 4), anche il fatto di non avere un observer (punto 5) sono un po' figli di queta situazione. Sicuramente mi piace la tua idea di spostare il Builder: apre la strada a separare il concetto di slowrow in una libreria esterna e al riuso del motore per sorgenti non-MAT in modo pulito, e anche al fatto di dimenticare gli ID e usare un valutatore + contesto. L'ultimo punto credo dipenda da tutto: non solo dal datalog ma anche dalla gestione degli ID, la posizione del builder, ecc.

Io farei cosi': procederei come hai scritto tu (builder fuori, slowrow in lib separata.MAT, no ID) e proverei a vedere di sostituire il datalog con le strutture dati di questo progetto (che son incredibilmente piu' semplici e leggere!). Da li', vedi se ha ancora senso avere un concetto di observer o se qui la separaione e' eccessiva e peggiora la comprensione invece che migliorarla.

Ciao! Lorenzo

Ciao,

grazie per la risposta! Non c'era nulla di polemico o di giudicante nella tua architettura, mi ricordo perfettamente che fu fatta in lockdown e quindi diciamo "disaccoppiata" dal codice reale e peraltro alcune scelte le trovo assolutamente ragionevoli.

Meanwhile... ho buttato il Datalog. Con commit dedicato, screenshot e dedica a te. In grande stile insomma. Alla fine il Datalog non era male, nel senso che aveva un senso a patto di usarlo per i segnali, ma usarlo per come era finito ad essere usato (solo i virtuali), era un suicidio. Poi chiaro l'implementazione era il solito gioco al massacro del riempi liste e scorri liste e onestamente la macchina a stati di un SampleBlock era piuttosto complicata (uno dei primi casi di race condition nello stesso thread che ho mai visto... lo status cambiava a sorpresa e quindi c'era un ordine obbligatorio di chiamate...). Ma alla fine tenerlo aveva poco senso e fare refactoring pure, quindi CircularBuffer. Non è stato inutile comunque, il concetto rimane per le UDPSources e quindi le lesson learnt torneranno comunque utili (in realtà anche lì potrei soppiantare con N CircularBuffer o uno solo oggetto tipo CoverageEngine ma insomma qui lo scenario è un po' diverso, è proprio più un buffer, quindi da valutare).

Il concetto di Observer però vorrei chiarire che è parte integrante della struttura nuova e non verrà rimosso. Cioè, ho cambiato tutta la parte di inizializzazione (TAG320TelemetrySource) dei canali MAT creando per ogni loggable item (cioè un parametro) una sua pipeline di processing definita upfront, in programmazione. Quindi quando arrivano samples di un parametro io chiamo la sua pipeline che parte con un raw to eng converter che passa poi la palla ai suoi Observer. Uno di questi pubblica sul bus, un altro fa da feed per gli eventuali virtuali che da esso dipendono o scrive nel SlowRowStorage se è uno slowrow e, prossimamente, ci vorrei attaccare anche il MATErrorEvaluator che oggi ti ricordo viene creato in post program e deve ascoltare il ring per aprire e ciclare sui pacchetti in cerca dei segnali. Con il concetto di Observer invece vorrei attaccare alle portanti direttamente il calcolatore.

Tornando ai virtuali, l' Observer rimarrà, rimane da capire se esso scrive nel buffer e poi, sulla callback di readiness del buffer esso fa forward all'evaluator costruendo un Context ad hoc (com'è in questo momento) oppure se l'observer è già il virtual evaluator più embedded, cioè quello dela tua lib con storage integrato nel valutatore.

Sulle costanti, sto spingendo per avere un concetto di costante fino al viewer, quindi nella GlobalTableRepository aggiungere a fianco del classico AddChannel un bel AddConstant da mettere per dire negli stream infos. A quel punto il virtuale lo si potrebbe valutare subito e registrare il risultato come costante e bon.

Grazie ancora! A

Bene, grazie a te per avermi aggiornato! Ora che me lo scrivi cosi' ha assolutamente senso: l'observer e' quel "ring interno" di cui parlavamo! Che poi, essendo sullo stesso thread, e' semplicemente un observer all'atto pratico.

rimane da capire se esso scrive nel buffer e poi, sulla callback di readiness del buffer esso fa forward all'evaluator costruendo un Context ad hoc (com'è in questo momento) oppure se l'observer è già il virtual evaluator più embedded, cioè quello dela tua lib con storage integrato nel valutatore.

Questo e' quello a cui mi riferivo: vedi tu qual e' il design piu' pulito e bom. Ho avuto l'impressione che quello di oggi sia (almeno in parte) dovuto al fatto di dover usare datalog e quindi forse si puo' migliorare rendendo direttamente il virtual evaluator observer. Ma non e' detto: puo' essere che sia un buon design a prescindere. Vedi tu col codice sotto.

Per le UDPSources (se non lo avevo gia' fatto... non ricordo!) credo si possa mantenere ma semplificare largamente, togliendo quegli inciampi che descrivi (ricordo anche io che tra live, no live, deferred, fatto che all'interno della stessa stack call e' sia live che deferred perche' a un certo punto lo modifiche e le funzioni successive "se lo aspettano" era decisamente difficile da seguire)

Alla fine

// As it is now
var analyzer = new VirtualChannelAnalyzer(table);
analyzer.Analyze(virtDefinition.Formula, (dependecyIndexes, evaluator) => 
{
  // Evaluator is build with Expression and can be called like
  // evaluator.Evaluate(context, values => {});
  // indexes I need them to understand the kind of virtual (sr, single, multi, constant...)
  // basically I am recreating the builder but the evaluator is completely
  // decoupled from the storage
});

// I could switch to
analyzer.Analyze(virtDefinition.Formula, visitor => 
{
  // Given the visitor, I can replicate the Builder here, the info are
  // provided by the visitor, I could then create here the evaluators, that would embed the 
  // storage. Cons, the lib is aware of slowrow and const. 
  // Honestly both are very similar, but I would probably go with this
  // one.
});

Per le UDPSources (se non lo avevo gia' fatto... non ricordo!) credo si possa mantenere ma semplificare largamente, togliendo quegli inciampi che descrivi (ricordo anche io che tra live, no live, deferred, fatto che all'interno della stessa stack call e' sia live che deferred perche' a un certo punto lo modifiche e le funzioni successive "se lo aspettano" era decisamente difficile da seguire)

Sono già stati fatti ampi refactoring per vari motivi, primo tra tutti ho storato direttamente i datagram, non i points estratti e ho reso molto più performante l'inserimento, evitando di calcolare N volte i time e i gap e i rate. Cercherò di guardare e sistemare anche la questione deferred, la strategia che voglio adottare è la solita, tell don't ask, ovvero vorrei che fosse il sampleblock a dirmi che è pronto o cambiare o status in modo più furbo.

Ti aggiorno Grazie! A

ldematte / SM.Expressions

Dubbio architetturale #22

Introduzione

Esempi semplici

Virtuali

Il caso delle costanti