[WIP] Améliorer les Internals du Manager

[armetiz]

Actuellement les fonctions internes du Manager ne sont pas forcement optimiser pour les performances..
Prenons l'exemple de Manager::areConnected qui est assez lourde pour pas grand chose.

Peut-être que cela doit aboutir à une modification de RepositoryInterface

[choomz]

Je suis en train d'implémenter le ConnectionBundle avec Mongo dans un projet actuellement. De façon à me projeter sur des éléments solides, j'ai codé le cas d'utilisation qui me semblait le plus lourd : en entrée d'une API, 2 méthodes LINK & UNLINK (basés sur les verbes HTTP) qui permettent de lier n'importe quel resource existante à un utilisateur par ex (LINK /api/users/{userID})

Voici le code :
https://gist.github.com/choomz/5219790

Je veux bien concevoir que ce cas est lourd et qu'on aura pas forcément le besoin de réaliser plusieurs actions à la suite et en boucle, mais dans l'idée c'est pour mettre en évidence deux choses :

• nous n'avons pas la possibilité d'appeler persist() sans flush() au niveau des repos (on pourrait imagine l'appeler tout à la fin ?)
• actuellement l'utilisation des methodes areConnected, isConnected etc implique de faire des reqs très lourdes pour simplement récupérer un booléen, même si l'utilisation pourrait se justifier pour éviter d'appeler un disconnect() (suite à l'issue Modification de disconnect() du Manager #21) sur des connections non existantes, ça reste lourd au niveau du code.

Bref, ce que je me dis, c'est que si on cherche la performance et qu'on ne veut pas etre obligé de bypass le manager pour faire des reqs customs, il faudrait mettre en place dans le manager et par extension dans les repos un systeme de jointure qui évite de faire ce genre de trucs :
https://github.com/Kitano/KitanoConnectionBundle/blob/master/Manager/ConnectionManager.php#L78-L79

Donc deux choses :

• avoir la possibilité de flush() sur demande
• optimiser les reqs de sélection pour ne pas en faire 2 quand 1 serait possible

Vos avis ?

[armetiz]

Pour la deuxième partie de ton message,
Cela reprends le début de l'Issue, et à mon avis il faut directement implementer areConnected au sein des RepositoryInterface. C'est le seul moyen d'aboutir à quelque chose de plus light.

Concernant l'ajout de Connection en "masse", c'est en effet un gros soucis de performance en l'état.
J'aurai tendance à dire qu'il ne faut pas permettre à l'utilisateur de "flush" sur demande. C'est une notion spécifique au système de persistance.. QUID du ArrayRepository et d'une méthode flush vide.

Que penses-tu d'un système de "connection" optimisé pour la masse ?
Genre, un tableau qui contient les references de liaison que l'on souhaite réaliser.

$connectionDesc = array();

foreach($request->attributes->get('link') as $type => $objects)
{
    foreach($objects as $object)
    {
        $connectionDesc[] = array($user, $object, $type);
    }
}

$cm->connect($connectionDesc);

De cette maniere, on abstrait toujours la partie Persistance à l'utilisateur. Le soucis, c'est que l'on change le prototype de la méthode connect..
Si cette approche vous interesse, on peux faire en sorte de modifier RepositoryConnection::connect(array $connectionDesc);
On conserve :
ConnectionManager::connect(NodeInterface $source, NodeInterface $destination, $type);
ConnectionManager::disconnect(NodeInterface $source, NodeInterface $destination, $type);

On ajoute :
ConnectionManager::connectBulk(array $connectionDesc);
ConnectionManager::disconnectBulk(array $connectionDesc);

En interne, ConnectionManager::connect & disconnect s'occupe de construire un connectionDesc et d'utiliser connectBulk / disconnectBulk.

[choomz]

👍 pour les deux dans l'idée.

Par contre attention du coup à l'ordre des parametres dans l'array, là ton exemple implique que le premier = source & deuxieme = destination. C'est completement logique mais pas très explicite. On pourrait peut etre aller vers un objet ? Dans l'absolu les deux me vont, c'est juste pour faire 'propre'.

[armetiz]

Connaissant @benjamindulau il va vouloir faire de la POO jusqu'au bout :p
Et en effet, le plus propre c'est d'avoir un objet, sinon on risque de merder dans l'ordre des paramètres du tableau à un moment donnée...
ConnectionsDescription::addConnectionDesc($source, $destination, $type);

Un truc du genre ?

[benjamindulau]

Déjà 👍 pour les *Bulk() !

Ensuite, n'oubliez pas (c'est ce que dit @armetiz), que le repository est là pour justement se permettre de faire quelque chose de sale mais performant.

Ce n'est pas du tout aberrant que le manager se repose simplement sur le repository pour le areConnected. On peut donc virer cette boucle du manager et laisser faire le repo. Le manager lui peut faire un pre/post traitement et c'est tout (et c'est pe même pas la peine).

Après côté Repo on fait ce qu'on veut, si on doit faire du raw sql pour les perfs, go !

👍 pour un objet qui contient un bulk des connections à effectuer, on pourrai tl'appeler ConnectionCommand ;-)

[choomz]

Je pense avoir pas mal avancé sur le sujet, le seul truc pas encore tip top c'est la déconnection en masse qui reste assez consomateur en requettes, à voir si vous pouvez optimiser :)

https://github.com/Kitano/KitanoConnectionBundle/blob/master/Manager/ConnectionManager.php#L98
https://github.com/Kitano/KitanoConnectionBundle/blob/master/Manager/ConnectionManager.php#L226