Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purposes
This PR is a first draft to experiment with a typer domain, i.e., a dedicated domain that performs typing computations (partially) in parallel with the main domain, which manages the server and analyzes the results. The final implementation should also support:
Merlin mock up to test design
The design implements here is first tried here .
Todo
There is currently a bug where tests can never finished when an exception is passed from the typer domain to the main domain. Also the reason is still unclear to me, I think part of the issue is because I use 2 different atomic to communicate between the main domain and the typer domain :
message
used during typing for cancellation and to to pass the result lock to the main domain for analysisclose
used in the highest-level loop to pass exception from the typer to the main domain and when closing merlin.Current Changes
For now, this PR introduces the following modifications:
Local_store
scope to support computations in both domains.New_commands.run
to be able to determine if the typer can return a partial result depending on the command.Design Notes & Expected Changes
Laziness
Since laziness is inherently unsafe (see Lazy module documentation:) in a multicore context, it has been removed. However, this introduces new bugs as some previously deferred computations now run eagerly:
local_store
scope must be carefully extended.Also a type
'a lazy
can't be replaced byunit -> 'a
because in some cases, the typer result is used twice and without caches, it is useful as some times, some lazy values are never forced. But with caches and a cancellation mechanism it seems okay to actually always compute as much as possible.The issue with the uncaught exception can be managed by moving the computation of the pipeline as far as possible in the code (i.e.
New_commands.run
) to ensure if an exception is caught by the typer domain and passed to the main domain, the main domain is always protected under atry .. with
. This needs to be checkedLocal Store Management
Most computation must take place when the
local_store
isbound
. Previously, all computations were enclosed withinLocal_store.with_store
(viaMocaml.with_state
). However, this approach is not compatible with computations involving two domains.To address this,
with_store
has been replaced withopen_store
andclose_store
, allowing more flexible management of the store’s scope.This design is likely to evolve further, particularly to enable proper interruption of the typer when needed.
Concurrency with a Typer
The shared values are currently protected under
mutex
, or areatomic
values depending on if a passive or active wait is prefered.Todo
Currently the critical section are quite extended, especially in
Mpipeline.typer_domain
andMpipeline.get
where the whole loops are under mutex. Theprotect
should be moved inside the loop, before the read of the protected value. This should not change a lot of thing but it should simplify reasoning and the risk of creating deadlocks.Credits
This preliminary work was done with @voodoos and @xvw.