Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Base implementation of Dynamix, Tim #111

Open
wants to merge 88 commits into
base: develop
Choose a base branch
from

Conversation

molenzwiebel
Copy link
Contributor

This PR contains the following changes:

  • A new metalang, tim, which is the result of running a Dynamix specification on an input AST. This is a CPS-based intermediate representation language. The tim metalang includes a simple Stratego interpreter, but the intention is that this is replaced with a more performant optimizing compiler in a follow-up master thesis.
  • The dynamix metalang has been split into dynamix (grammar, static analysis, "compilation"/spec merging) and dynamix_runtime (stratego interpreter for running compiled/merged specifications).
  • Enable multi-file analysis for dynamix, and support running the statix spec inside the concurrent interpreter.
  • Tasks/menus for compiling the dynamix specification in a project to an ATerm representation, and using the dynamix_runtime for executing the spec on an input AST.
  • All languages (tim/dynamix/dynamix_runtime) have been namespaced into folders, their sort/constructor names prefixed (T/M respectively) and stratego rules prefixed (tim-, dx- respectively).
  • The tim/dynamix/dynamix_runtime spoofax2 projects use source imports to depend on each other.
  • Actual signature generation for dynamix is implemented, instead of simple stubs.

Notes/issues that would be nice for you to take a look at:

  • The ReadDynamixSpecTaskDef is possibly a bit over the top and could be changed to just passing a ResourcePath, and having the reading be done in the dynamix runtime instead, but I wasn't entirely sure if that would work properly with PIE.
  • Menu items are included for running the Dynamix specification. However, the continuous versions of these seem to not work. Possibly due to the fact that the output is written to a file.
  • Embedding dynamix_runtime should be complete (largely copied from the rv32im code), but I'm not entirely certain if is correct. I am not sure whether a standalone eclipse build will work.
  • Passing options to the DynamixRuntime is currently done through createDynamixRuntimeConfig in ExecuteDynamixSpecificationTaskDef.java.mustache. This works, but there might be a more idiomatic approach?
  • The dynamix_runtime "language" is currently in the metalang folder, but it might be more appropriate to put it elsewhere (next to rv32im?).
  • The tim stratego interpreter is currently bundled with the metalang (and not split into tim_runtime or something similar). Currently done because it works and as mentioned this shouldn't be the long-term solution for running tim files.
  • Source imports for ESV files don't seem to work. Try uncommenting the tim/colors import in the dynamix Main.esv.
  • (Spoofax 2) versions of all the languages are set to 0.1.0-SNAPSHOT. Is that fine?
  • All stratego files have commented out type signatures for str2. Ideally the files are ported over the str2, but I had some issues with getting Stratego 2 to work properly in Spoofax 2 so we're back on the old stratego for now.

Other notes/TODOs:

  • Spoofax 3 doesn't yet support configuring the Statix solver mode. @AZWN, I see that Incremental Statix #108 is open for that?
  • Current statix spec becomes very slow very fast, big culprit here is the checking for overlaps in rule patterns. This is partially on my end, and partially something which might be due to an issue in the Statix solver?
  • The Statix spec seems to work for the concurrent solver (tested in Spoofax 2), but the concurrent solver is significantly slower. The incremental solver seems to fail. I sent a bundled version to @AZWN, hopefully he has some time to take a look at some point.


@DynamixRuntimeScope
public class DynamixRuntimePrettyPrint implements TaskDef<IStrategoTerm, Result<String, ?>> {
private final Provider<StrategoRuntime> strategoRuntimeProvider;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using DynamixRuntimeGetStrategoRuntimeProvider here results in an exception in PIE on every build after the first, due to reasons I don't fully understand. For now this is acceptable, since the stratego ctree/jar of the runtime will be bundled with Eclipse anyway, but we should likely get a proper dependency on it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a TODO in the code here that we should switch to DynamixRuntimeGetStrategoRuntimeProvider if possible and the reason this was not done?

Copy link
Member

@Gohla Gohla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool, and also very quickly done!

Are there any examples and/or regression tests for dynamix, to ensure that dynamix will not suddenly break? If not, it would be good to at least have some simple smoke tests.

Answers to general questions:

The ReadDynamixSpecTaskDef is possibly a bit over the top and could be changed to just passing a ResourcePath, and having the reading be done in the dynamix runtime instead, but I wasn't entirely sure if that would work properly with PIE.

Maybe the reading can be done by the dynamix runtime instead of outputting an AST from a task (since reading is probably so fast that caching may not be useful?). However, for incrementality you must still tell PIE about the read (require) dependency, otherwise changing the specification will not result in rerunning the dynamix runtime.

Menu items are included for running the Dynamix specification. However, the continuous versions of these seem to not work. Possibly due to the fact that the output is written to a file.

Hmm, this could be a bug, can you make a bug report so I can look into this sometime?

Embedding dynamix_runtime should be complete (largely copied from the rv32im code), but I'm not entirely certain if is correct. I am not sure whether a standalone eclipse build will work.

From a glance it looks ok, but can you test this? If you run gradle buildSpoofax3LwbEclipseInstallation in devenv it will create a standalone installation somewhere in spoofax.pie/lwb.distrib/spoofax.lwb.eclipse.repository/build/ (either in eclipse-<os>-<arch> or in the distributions directory. If there are problems we can look into it.

Passing options to the DynamixRuntime is currently done through createDynamixRuntimeConfig in ExecuteDynamixSpecificationTaskDef.java.mustache. This works, but there might be a more idiomatic approach?

That's what we do in other places too, it's easy and works. The downside is that changing configuration requires recompilation of the Java classes. You could also write the options to a Java properties file during language compilation (be sure to use mb.common.util.Properties#storeWithoutDate to ensure no date is written to make it deterministic) and require and read that in the task.

The dynamix_runtime "language" is currently in the metalang folder, but it might be more appropriate to put it elsewhere (next to rv32im?).

It needs to be part of the lwb composite build dependency wise, so it should stay there. Maybe there could be another metaruntime directory for projects like that.

The tim stratego interpreter is currently bundled with the metalang (and not split into tim_runtime or something similar). Currently done because it works and as mentioned this shouldn't be the long-term solution for running tim files.

I guess that is fine.

Source imports for ESV files don't seem to work. Try uncommenting the tim/colors import in the dynamix Main.esv.

Can you make a bug report for this as well, so I can have a look at it sometime?

(Spoofax 2) versions of all the languages are set to 0.1.0-SNAPSHOT. Is that fine?

I think the Spoofax 2 Gradle plugin will override that with the version from Gradle, so that should be fine.

All stratego files have commented out type signatures for str2. Ideally the files are ported over the str2, but I had some issues with getting Stratego 2 to work properly in Spoofax 2 so we're back on the old stratego for now.

That is too bad, but fine solution for now. Did you contact Jeff or make a bug report?


@DynamixRuntimeScope
public class DynamixRuntimePrettyPrint implements TaskDef<IStrategoTerm, Result<String, ?>> {
private final Provider<StrategoRuntime> strategoRuntimeProvider;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a TODO in the code here that we should switch to DynamixRuntimeGetStrategoRuntimeProvider if possible and the reason this was not done?

specAst = readTerm(inputStream);
}
return Result.ofOk(specAst);
} catch(Exception ex) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catching all Exceptions is bad practice, as it also catches all RuntimeExceptions indicating bugs such as NullPointerException which should just crash the build, as well as catching InterruptedExceptions which should cancel the build, etc.

Can you catch a more specific exception? If not, can you rethrow RuntimeException and InterruptedException?

try {
return reader.parseFromStream(stream);
} catch(IOException e) {
throw new IllegalStateException("Loading ATerm from stream failed unexpectedly", e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the IOException be handled in a nicer way? Either return the exception as part of the Result of the task, or just let the IOException bubble up?

@AZWN
Copy link
Contributor

AZWN commented May 13, 2022

To quickly react to your last points involving the concurrent solver: When your specification semantically works with the concurrent solver, and there is no type-directed transformation, I think it is fine to use the traditional solver for now, and switch to the concurrent solver later. I'll add a TODO to that PR.

@AZWN AZWN mentioned this pull request May 13, 2022
2 tasks
Called when an object is no longer in accessible memory
Compiling to LLVM in Spoofax 3 is not yet functional
* Records were handled incorrectly by gc
* Function type was not converted to closure type everywhere just yet
In some cases free variables might be added after processing a function
@AZWN
Copy link
Contributor

AZWN commented Apr 13, 2023

Exactly 11 months ago, I posted a comment about enabling the concurrent solver. I don't remember the discussion fully anymore, but the referred PR has landed, so technically, enabling the concurrent solver might be possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants