Open pcdavid opened 3 months ago
A few remarks:
previousDiagram
and diagram
in DiagramCreationService.refresh
to detect if there was any actual change may not be free in terms of performance. This will need to be measured to see if the actual check will not be counter-productive.DiagramCreationService.refresh()
would need to evolve to be able to return "nothing changed". It already returns an Optional<Diagram
>, so there may be nothing to change in terms of Java types, but the way this Optional is interpreted by DiagramEventProcessor.refresh(ChangeDescription)
is that if the returned Optional is empty, it means the diagram could not be rendered, not that it was rendered and produced the same result as before.
This is formulated about diagrams, but the issue is more general. It's just more impacting in terms of performance for diagrams which can quickly become large compared to the other kinds of representations (only large explorer trees with thousands of tree items could also be a problem I think for now).
The issue is that on any semantic change (in practice, any execution of a tool which may have a semantic impact, whether it actually did change something, see this comment in another context) we must refresh/update all the active representations (e.g. diagrams) in the same editing context. Such is the open-ended nature of Sirius that we can not reliably predict, for a given tool execution or semantic change, which representations it will actually impact or not, so we must update all.
However, once the backend has done its work, i.e.
DiagramCreationService.refresh(IEditingContext, IDiagramContext)
has obtained the updated version of theDiagram
, we unconditionally send the whole "new version" to all client browsers who currently display that diagram. For large diagrams this can have significant performance impact. It means we serialize theDiagram
DTO as a GraphQL payload (with a non-trivial overhead in invoking all the field resolvers), potentially once per browser/client, and then send large amount of data (a medium-sized diagram can represent several megabytes of payload) to the clients, which then have to re-render it.This whole process is required in the case where the new/updated version of the diagram is actually different from the previous one, but is 100% overhead when the diagram was not actually impacted by the semantic change which triggered the refresh, and the newly computed diagram is identical to the previous one.
This not an uncommon case at all. It two users Alice and Bob are working on the same project, each in separate subsets of the project and with their own diagrams opened, any change by Alice in the semantic subset she is working on will trigger a refresh of both Alice & Bob's diagrams (which is normal), but also, in this case, re-sending the same diagram he currently has displayed to Bob for no reason.