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

Use of delayed and physical connections as provided by the C++ runtime #1583

Merged
merged 33 commits into from
Feb 16, 2023

Conversation

cmnrd
Copy link
Collaborator

@cmnrd cmnrd commented Feb 3, 2023

Currently, after delays are implemented as an AST transformation. With lf-lang/reactor-cpp#38, the C++ runtime provides a native solution for implementing delayed and physical connections. This PR disables the AST transformation in the C++ target and instead uses the native runtime implementations. It also adds a couple of tests for various corner cases that have not been tested before.

In short, the runtime provides to classes DelayedConnection and PhysicalConnection that are specializations of the Action class (which better should be called Trigger). By implementing Action, the connections may add new events to the event queue. At the same time, they provide API for binding to one upstream and multiple downstream ports. Due to a callback mechanism, the connection gets notified when the upstream port is set and it will schedule an event. When the scheduler retrieves the event from the event queue, its setup() function will also set all downstream ports and accordingly trigger downstream reactions.

This PR probably makes the C++ target the first target to correctly implement physical connections, as physical connections are currently not properly handled by the AST transformation (see #616)

Copy link
Member

@tanneberger tanneberger left a comment

Choose a reason for hiding this comment

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

I dont see any big mistakes

org.lflang/src/lib/cpp/lfutil.hh Outdated Show resolved Hide resolved
Copy link
Collaborator

@edwardalee edwardalee left a comment

Choose a reason for hiding this comment

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

Didn't check carefully, but LGTM!

@cmnrd
Copy link
Collaborator Author

cmnrd commented Feb 6, 2023

@petervdonovan Could you take a look at the failing LSP tests? If you have any tips on where I can start looking myself in the future (I already struggle with finding the actual error in the log), I would appreciate that. Or even better: it would be great if we could have some instructions on how to handle the LSP tests in the developer docs.

@petervdonovan
Copy link
Collaborator

petervdonovan commented Feb 6, 2023

The LSP tests appear to be passing

it would be great if we could have some instructions on how to handle the LSP tests in the developer docs.

I will try to do this tomorrow or the day after tomorrow

@cmnrd
Copy link
Collaborator Author

cmnrd commented Feb 7, 2023

The LSP tests appear to be passing

Oh, then I guess it was just one of the regular C++ tests also causing the LSP test failures.

I will try to do this tomorrow or the day after tomorrow

That would be great! Thanks!

@tanneberger tanneberger force-pushed the cpp-native-delayed-connections branch from 91ab496 to 4cb0758 Compare February 10, 2023 17:12
@tanneberger tanneberger added enhancement Enhancement of existing feature cpp Related to C++ target runtime Related to the runtime implementation federated epic labels Feb 10, 2023
Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

Looks good to me!

import org.lflang.lf.Reactor
import org.lflang.lf.VarRef

class CppConnectionGenerator(private val reactor: Reactor) {
Copy link
Member

Choose a reason for hiding this comment

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

This class needs some documentation...

@lhstrh lhstrh changed the title Use delayed and physical connections as provided by the C++ runtime Use of delayed and physical connections as provided by the C++ runtime Feb 16, 2023
@lhstrh lhstrh merged commit c42a5bc into master Feb 16, 2023
@lhstrh lhstrh deleted the cpp-native-delayed-connections branch February 16, 2023 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp Related to C++ target enhancement Enhancement of existing feature epic federated runtime Related to the runtime implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants