getOrCreateEditingContextEventProcessor hits the database on every invocation

[pcdavid]

EditingContextEventProcessorRegistry#getOrCreateEditingContextEventProcessor is invoked on every input to find (and create/load if necessary) the editing context event processor to dispatch the input to. It's on the critical path of almost every operation.

Currently the first thing it does is to test:

if (this.editingContextSearchService.existsById(editingContextId)) {

before looking into the currently loaded event processors (this.editingContextEventProcessors.get(editingContextId)).

The existsById call ends up in SemanticDataSearchService.existsByProject, which will search into the database:

This means we hit the DB at least once for every input. In the nominal case, we will only receive requests for already loaded editing context. Once a project is loaded, it will probably receive thousands of inputs before becoming inactive and being disposed.

On my machine where PostgreSQL is running locally, just this test costs about 7ms. It's probably more when the DB is on a remote host (though I have not measured).

Technically, changing the order of the test (look for an already loaded editing context in the fast patch) could change the semantics if the project has just been removed from the database and its editing context event processor not yet disposed/removed, thought I'm not even sure this can happen.

[pcdavid]

Actually, I don't measure the expected gains. On a scenario where I simply change the selected node on a very small diagram (only two nodes, Explorer and Details view closed), I see the number of DB queries sent reduced (existsByProjectId is gone), and a small amount of time gained, but not much:

[pcdavid]

I'm not sure we can avoid this, at least not as simply as what the current PR does.

When deleting a project, MutationDeleteProjectDataFetcher invokes directly IProjectApplicationService.deleteProject, which ends up removing the data from the DB (via ProjectDeletionService.deleteProject(UUID)), but this is in the application layer.

Even though we publish a ProjectDeletedEvent, the code in e.g. EditingContextEventProcessorRegistry (or anything in Sirius Components) has no way to react to this, so it currently must ask the database for the current state (otherwise this leads to symptoms like #3829).

Note that the problem is more general than "just" EditingContextEventProcessorRegistry#getOrCreateEditingContextEventProcessor. Every GraphQL operation inside Viewer#editingContext passes through ViewerEditingContextDataFetcher which starts by testing if this.editingContextApplicationService.existsById(editingContextId), and for the same reason the only way for it to get an up to date answer is to ask the DB.

This means that most interactions that seem inoccuous like clicking on an element in a diagram to get it's palette require (at least) one round trip to the database.

[pcdavid]

Note that have two different (but partially redundant?) paths used to test if an editing context with a given id exists. Both are invoked in a simple interaction like clicking on the background of a diagram and both call to the DB (with different queries):

One is in EditingContextEventProcessorRegistry#getOrCreateEditingContextEventProcessor, which tests for the existence of semantic data for this project id:

The second is in ViewerEditingContextDataFetcher which tests for the existence of the project corresponding to the editing context:

[pcdavid]

I'm not sure we can avoid this, at least not as simply as what the current PR does.

When deleting a project, MutationDeleteProjectDataFetcher invokes directly IProjectApplicationService.deleteProject, which ends up removing the data from the DB (via ProjectDeletionService.deleteProject(UUID)), but this is in the application layer.

For this part of the problem, it might actually just be enough to add:

this.editingContextEventProcessorRegistry.disposeEditingContextEventProcessor(input.projectId().toString());

in MutationDeleteProjectDataFetcher as it existed before.

[pcdavid]

See #3901 for MutationDeleteProjectDataFetcher cleanup.

Otherwise, after discussion with @sbegaudeau we decided to keep the database lookup in place despite the potential performance hit as it ensures better consistency between the runtime data and the database, especially when mechanisms from e.g. #3897 are used.