Simplify concurrent API

[ebousse]

This PR aims to simplify as much as possible the concurrent API, which means:

• removing as many operations as possible (at the moment most operations are only used by the engine itself, therefore exposing them is not necessary)
• moving out everything that is coupled to CCSL/TimeSquare

This should hopefully:

• make it easier to implement a concurrent engine (eg. Henshin)
• make a step in the direction of merging the interfaces of all engines into a unified one, in the future

Among other things, this PR:

• renames the concurrent engine into MoccmlExecutionEngine, and renames many other moccml-specific classes,
• introduces new execution platform / execution context interfaces and classes,
• introduce several new abstract classes, such as AbstractSolverCodeExecutorConcurrentEngine which contains all the (not moccml-specific) code aiming to drive a concurrent execution with a solver and a code executor. The moccml engine now extends this abstract class, and provide all the moccml specific logic.
• moves up to AbstractConcurrentExecutionEngine all code that can be shared among concurrent engines.
• simplifies the concurrent engine API to its smallest, which means now to create a new concurrent engine one must mostly implement the following methods:

public class SampleNewConcurrentEngine extends AbstractConcurrentExecutionEngine {

	@Override
	public String engineKindName() {
		// TODO mandatory method to provide the name of the engine
		return null;
	}

	@Override
	protected List<Step<?>> computePossibleLogicalSteps() {
		// TODO mandatory method to provide a list of possible next steps (most likely a
		// list of ParallelSteps, each containing a list of SmallSteps)
		return null;
	}

	@Override
	protected void executeSmallStep(SmallStep smallStep) throws CodeExecutionException {
		// TODO mandatory method to execute a small step (ie. a single method/rule call)
	}

	@Override
	protected void performInitialize(IExecutionContext executionContext) {
		// TODO optional method that gets called when the engine is initialized (eg. to
		// prepare its solver)
	}

	@Override
	protected void doAfterLogicalStepExecution(Step logicalStep) {
		// TODO optional method that gets called right after a logical step was executed
		// (eg. to notify its solver)
	}

	@Override
	protected void finishDispose() {
		// TODO optional method that gets called when the engine is disposed of (eg. to
		// clean its solver)
	}

}

[ebousse]

Note: adding you as reviewers just for the discussion, this is not ready for merge or review yet :).

I still haven't executed any model with these changes, as I am struggling to run a concurrent example at the moment.

[jdeantoni]

Hi, for your information, I did a PR that align some "classical" concurrent language examples (namely sigpml and TFSM). Maybe it could help. I'm looking at your proposal ASAP. j

…

On 10/4/19 3:15 PM, Erwan Bousse wrote: Note: adding you as reviewers just for the discussion, this is not ready for merge or review yet :). I still haven't executed any model with these changes, as I am struggling to run a concurrent example at the moment. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#20?email_source=notifications&email_token=ABPASHXFZLZCALWZNVLF5LDQM46YNA5CNFSM4I5PU5X2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEALTMQA#issuecomment-538392128>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABPASHSHREPO6DMGCIPRPB3QM46YNANCNFSM4I5PU5XQ>.

-- Julien Deantoni Associate Professor I3S Lab - UMR 7271 -KAIROS INRIA Sophia Antipolis Méditerranée 2004 rte des Lucioles (Lagrange L-041) BP93, F-06902 Sophia Antipolis Cedex, France tel: +334 92 38 77 66 http://www.i3s.unice.fr/~deantoni/ Don’t take life too seriously! Nobody gets out alive anyway.(Dawn Gluskin)

[ebousse]

FYI this part here is to make sure to crash the execution engine when the code executor does not succeed to call an operation. While this is a bit out the scope of this PR, I believe this is better.

[ebousse]

@jdeantoni perfect, thanks! I'll look into that :)

Also some information on the refactoring:

• I started with IConcurrentExecutionEngine and removed as many operations there as I could (as long as it kept compiling).
• Accordingly, in ConcurrentExecutionEngine, I made as many members private as possible (as long as it kept compiling)
• I moved all operations of ISolver that were overly specific to CCSL to an interface called ICCSLSolver.
• Main problem of this refactoring: I had to add a ICCSLSolver type cast inside the EventSchedulingModelExecutionTracingAddon, which makes the concurrent trace exploration tool specific to the moccml engine at the moment. This is due to removing the operations to save/restore the state of the solver as an array of bytes from the ISolver common interface. I feel both these operations are too specific to ccsl at the moment − for instance I suppose that the Henshin engine cannot store its state as an array of bytes. Maybe we should provide a more generic way for any execution engine to store extra information in an execution trace?

[szschaler]

Hi,

I haven't looked at the store/restore API much yet, but I'm not sure it would be too difficult to store the Henshin-engine state in an array of bytes. Presumably, we would simply serialise the current in-memory model and restore that when needed. Am I missing anything else that would be considered part of the state? If not, then why would any of this be considered engine-specific?

Steffen

[combemale]

I suspect the way of saving the state (beyond the dynamic info stored in the runtime model itself) will be highly dependent on each approach, both on the content and the form it will be or can be saved. More importantly, what is needed to restore a state beyond the dynamic info is under the responsibility of each approach (i.e. provided by their own solver/virtual machine), and we should be inclusive such as anyone can easily integrate its own without having to adapt it to our assumption.

In conclusion, I'm wondering if it would be relevant to go beyond an interface (with a generic class reigying some "common" behavior)? but I'm also wondering why this interface would not be the same for all concurrent engines, but also the sequential ones???

Cheers,
benoit

[szschaler]

I agree there should be an interface. I suspect something like a Memento pattern should work, but could work in principle for all kinds of engines...

[ebousse]

I suspect the way of saving the state (beyond the dynamic info stored in the runtime model itself) will be highly dependent on each approach, both on the content and the form it will be or can be saved. More importantly, what is needed to restore a state beyond the dynamic info is under the responsibility of each approach (i.e. provided by their own solver/virtual machine), and we should be inclusive such as anyone can easily integrate its own without having to adapt it to our assumption.

fully agree!

In conclusion, I'm wondering if it would be relevant to go beyond an interface (with a generic class reigying some "common" behavior)?

For the concurrent engine you mean? Yes definitely, this PR is only a first step with the most "urgent" and not too difficult tasks, mostly API wise. Then I plan to focus on sharing code among concurrent engines, as I had done in the past for sequential engines.

but I'm also wondering why this interface would not be the same for all concurrent engines, but also the sequential ones???

I also fully agree! As I said in the PR, this PR is a first step, which should provide us a clearer vision of what exactly is the real minimal set of operations that define what is a concurrent engine at the moment, which will really help us unifying the engines in the framework later on.

I agree there should be an interface. I suspect something like a Memento pattern should work, but could work in principle for all kinds of engines...

Agreed, I think such memento could be achieved through an EReference (with containment=true) typed by EObject in the State concept of the execution trace metamodel, and to allow any metaprogramming approach to store there any relevant data as required. Then restoreState would always provide such data to the engine (if it is present), and the engine would know how to cast it and what to do with it. Not very pretty but could be a starting point.

[szschaler]

Why would the Memento need to be an EObject? Could it not just be a POJO implementing some suitable interface?

[ebousse]

We need to put the data in the trace, which is an EMF model, so my first thought was to remain in the EMF world. And if the interpreter of the considered metalanguage doesn't already have a well-defined format for memento-ing its execution state, I would not consider it to be a huge constraint for the person developing the integration into GEMOC to use Ecore to define how the memento is structured.

But there is the EJavaObject type in Ecore, which should allow the storage and serialization of POJOs within EMF models, but I have no experience with it. I suppose the stored POJO should be an instance of a class implementing java.io.Serializable? I'll look in to it. And if this is a valid technical choice, you would argue that it would be preferable @szschaler?

[szschaler]

I have no strong preference. I was just curious whether there was a need to go EMF here or whether POJO would do as it might occasionally make the engine implementation a bit easier.

[ebousse]

I have tested my changes with the SigML DSL and all seem to work well. The build on the CI is now passing as well after fixing the VCD addon: https://ci.eclipse.org/gemoc/job/gemoc-studio-integration/job/simplify-concurrent-api/.

Regarding the issue of saving/restoring the state of the solver, we would need to modify the concurrent trace metamodel, which is located in the execution framwork. Therefore this PR for now will not do better than making the concurrent tracer specific to the moccml engine, but let's keep this matter in mind for when we will unify all engines apis and all trace metamodels.

@jdeantoni when you have time to have a look, please do not hesitate to comment the changes I propose, and if you find the changes reasonable then I think you would be the most suited person to accept the PR :)

EDIT: wait in fact I just realized I haven't simplified IConcurrentExecutionContext and other such interfaces, I'll do a few more commits…

[ebousse]

After starting to work on interfaces and classes such as IConcurrentExecutionContext, I realized I needed to push the refactoring a bit deeper.

The PR now also :

• renames the concurrent engine into MoccmlExecutionEngine, and renames many other moccml-specific classes,
• introduces new execution platform / execution context interfaces and classes,
• introduce several new abstract classes, such as AbstractSolverCodeExecutorConcurrentEngine which contains all the (not moccml-specific) code aiming to drive a concurrent execution with a solver and a code executor. The moccml engine now extends this abstract class, and provide all the moccml specific logic. I will in another PR try to move code up from AbstractSolverCodeExecutorConcurrentEngine up to a AbstractConcurrentEngine class.
• many other things.

I still have to fix a few things to make concurrent executions work well, but I'm getting there.

[ebousse]

@jdeantoni Question: what is the difference between computePossibleLogicalSteps updatePossibleLogicalSteps? They eventually point to solverWrapper.updatePossibleLogicalSteps() and solverWrapper.computeAndGetPossibleLogicalSteps(), which is TimeSquare code.

My investivations led me into TimeSquare code, where I looked at fr.inria.aoste.timesquare.ccslkernel.solver.launch.CCSLKernelSolverWrapper. I find that is eventually calls TimeSquare's StepExecutor class, where both computeAndGetPossibleLogicalSteps updatePossibleLogicalSteps do the same things except that computeAndGetPossibleLogicalSteps also makes a call to computePossibleClockStates beforehand.

Unfortunately I still don't really get the difference :). Getting that would help me refactor the API, as I wonder whether having two different methods here is relevant for non-moccml engines.

[ebousse]

@jdeantoni Following up my previous comment/question: I found a solution by making this difference between "compute" and "update" specific to the moccml engine API, not to the generic API. Therefore now the concurrent engine generic public API only has computePossibleLogicalSteps.

[ebousse]

Update on the PR: I changed my mind and went as far as providing a real AbstractConcurrentExecutionEngine class that provides generic code for all concurrent engines. This helped me simplify much more the public API of concurrent engines, which is now as small as this:

public class SampleNewConcurrentEngine extends AbstractConcurrentExecutionEngine {

	@Override
	public String engineKindName() {
		// TODO mandatory method to provide the name of the engine
		return null;
	}

	@Override
	protected List<Step<?>> computePossibleLogicalSteps() {
		// TODO mandatory method to provide a list of possible next steps (most likely a
		// list of ParallelSteps, each containing a list of SmallSteps)
		return null;
	}

	@Override
	protected void executeSmallStep(SmallStep smallStep) throws CodeExecutionException {
		// TODO mandatory method to execute a small step (ie. a single method/rule call)
	}

	@Override
	protected void performInitialize(IExecutionContext executionContext) {
		// TODO optional method that gets called when the engine is initialized (eg. to
		// prepare its solver)
	}

	@Override
	protected void doAfterLogicalStepExecution(Step logicalStep) {
		// TODO optional method that gets called right after a logical step was executed
		// (eg. to notify its solver)
	}

	@Override
	protected void finishDispose() {
		// TODO optional method that gets called when the engine is disposed of (eg. to
		// clean its solver)
	}

}

I have updated the PR description accordingly, and I consider now that the PR is complete (although it can be fixed/changed based on feedback) and ready for a proper review and merge.

[szschaler]

Thanks, Erwan. Will this be merged in any time soon? I was hoping to work on updating my Henshin engine today, but may need to move to something else given the current download speed issues. However, in any case, merging this PR would help me loads in progressing this :-)

When we last spoke, you also mentioned you would provide a little more documentation on the other bits and pieces that a new concurrent engine would have to implement (similar to the mock up in the PR description). Would you be able to add this still?

[dvojtise]

right now, someone must first fix the compilation and test (CI doesn't pass yet https://ci.eclipse.org/gemoc/job/gemoc-studio-integration/job/simplify-concurrent-api/)

[szschaler]

Thanks. @ebousse the problem seems to be that the MoccML engine doesn't currently compile:

Compilation failure: 
gemoc-studio-execution-moccml/ccsljava_engine/plugins/org.eclipse.gemoc.execution.concurrent.ccsljavaengine/src/org/eclipse/gemoc/execution/concurrent/ccsljavaengine/commons/MoccmlModelExecutionContext.java:[55] 
MoccmlLanguageDefinitionExtension moccmlLangDef = (MoccmlLanguageDefinitionExtension) getLanguageDefinition();
^^^^^^^^^^^^^^^^^^^^^
The method getLanguageDefinition(String) in the type MoccmlModelExecutionContext is not applicable for the arguments ()

Could you take a look?

@dvojtise why is Github proudly saying that "All checks have passed", though?

[dvojtise]

@dvojtise why is Github proudly saying that "All checks have passed", though?

because the eclipse administrator have connected some validation ( an ECA checker) but github is not aware of the jenkins instance (and the correct job) that is in charge of validating the build (btw this is tricky since the build is validating the entire GEMOC project and not only the content of this repository)

The jenkins is triggered on every branches on the official repos

so the guy in charge of accepting the PR has to: 1/ if the contribution comes from an external repo: create a branch to enable the build on the CI from the PR, then 2/ verify that everything is OK before accepting the contribution

(this is more or less explained in https://github.com/eclipse/gemoc-studio/wiki/Official-commiter-memo)

[szschaler]

Thanks, Didier. @ebousse could you take a look? I was hoping to do something on generalising the Henshin engine today, but that depends on this PR being merged (or at least compiling :-) )...

[ebousse]

Sorry everyone, did not manage to get a second look at all that yet. My goal is to implement a DummyConcurrentEngine on my laptop to check that the new API can indeed be implemented for other cases than Moccml. And from there to fix what still needs to be fixed + to write a little documentation.

[ebousse]

I fixed the compilation problem, made some new improvements to the API, and also slightly changed the examples to make sure they work with the new API. Now the build is passing in Jenkins 🎉!

Moreover, I have implemented here a complete "dummy engine" that can be used a documentation or as a starter project to implement a concurrent engine using this API: https://github.com/ebousse/gemoc-dummy-concurrent-engine

It is entirely standalone, implements all required interfaces in a minimal way, and if used to run a model will simply create a sequence of 10 fake ParallelSteps.

@szschaler you can have a look at this "dummy engine" to have a better understanding of what this API provides and requires ; it is more detailed than the code snippet I provided in the PR description and covers all interfaces

@dvojtise I don't have the rights to move my gemoc-dummy-concurrent-engine project to the Github gemoc namespace, but if you agree we can move it there.

@jdeantoni and @dvojtise when you have time, this is (yet again) ready for review :)

[dvojtise]

looks good to me,
renaming the class with Moccl as a start is a good idea in order to plan for future refactoring
my local test passes and CI is OK too

[dvojtise]

I think that a future PR would go even further by moving some of the abstract classes and interfaces to https://github.com/eclipse/gemoc-studio-modeldebugging/tree/master/framework

this would help to get common interfaces for both concurrent and sequential engine (and coordinate them)

[ebousse]

Yes absolutely, moving code up into the framework will be the next step. We also need to remove the "sequential xdsml" extension point from the java engine API, as this EP is at the moment required by all engines.