Support for basic scheduling enclaves in the C++ target #1513
Conversation
| private boolean isOptional() { | ||
| return defaultValue == null; | ||
| } | ||
| record AttrParamSpec(String name, AttrParamType type, boolean isOptional) { |
There was a problem hiding this comment.
I don't think this change is very sensible. Previously isOptional was implemented in terms of the default value. With your change we can't add back default values later, as changing the constructor back to take an Object as third param would make all existing constructor calls have a default value of true...
If you want to increase readability of the ctor calls then a better way IMO would be to have static factory methods newRequiredParam(String name, AttrParamType type), newParamWithDefault(String name, AttrParamType type, Object defaultValue) that delegate to the constructor.
There was a problem hiding this comment.
The default value was not used anywhere and since this class only works as a spec for the verifier I would argue that the default value is not needed anyway. It would only make sense to keep it if we use this spec in `AttributUtils' to retrieve the value of the parameter, but this would be a larger change.
| List.of(new AttrParamSpec(VALUE_ATTR, AttrParamType.STRING, true)) | ||
| )); | ||
| // @enclave(indivdual=boolean) | ||
| ATTRIBUTE_SPECS_BY_NAME.put("enclave", new AttributeSpec( |
There was a problem hiding this comment.
If this individual attribute is optional, then it should mention a default value
There was a problem hiding this comment.
Since we don't manage default values centrally at the moment, I don't think it should. I agree that we should provide a better infrastructure for attribute parameters, but this should be done in another PR.
There was a problem hiding this comment.
It looks like the term "enclave" is getting established. Did we decide the terminology collision with Intel SGX doesn't matter?
|
I am moving @edwardalee's comment out of the code discussion for better visibility.
I think enclave is the best option currently on the table and I went with it. I wouldn't consider the terminology final, though. But personally I think the feature is conceptually sufficiently distinct from Intel SGX. |
cmnrd
force-pushed
the
cpp-affiliated
branch
from
3f3a1bb
to
f9c288b
Compare
|
The discussion in today's meeting concluded that we should use the term "scheduling enclave" in our documentation to disambiguate from secure enclaves. |
|
I hope the tests are fixed for good now. As far as I am concerned, this is ready to be merged. |
|
I agree that it looks ready, but I left a suggestion for a syntax change... |
lhstrh
changed the title
lhstrh
changed the title
This PR introduces the
@enclaveattribute which allows to nest reactor programs such they can execute independently (similar to federates) while sharing the same address space. Any reactor instance annotated with@enclavewill create its own execution environment and scheduler (the enclave), and act as the top-level reactor in this enclave. This also works for banks. By default, a bank annotated with@enclavewill create a single enclave where all bank instances are top-level reactors. If instead@enclave(individual=true)is used, this will createnenclaves for a bank of widthnwhere each bank instance is the top-level reactor in one of the enclaves.This pulls in lf-lang/reactor-cpp#37
Missing features that will be added in later PRs:
Closes #1383