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

Optimize after delays on broadcast connections and fix them for C #593

Merged
merged 4 commits into from
Oct 11, 2021

Conversation

cmnrd
Copy link
Collaborator

@cmnrd cmnrd commented Oct 11, 2021

#553 fixed after delays on broadcast connections in C++, but did so quite inefficiently. Given a broadcast connection with N left ports and M right ports (N<=M), #553 would create M delay reactors and broadcast from the left ports to the delay instances. However, it would be much more efficient to only create N delay reactors and broadcast their output to the right ports. This optimization is implemented in this PR. The PR also includes test cases for C and C++.

Interestingly, this change also happens to fix #554. It looks like the C generator expected the broadcast to occur on the right side of the delay reactors. This is probably the reason why #553 did not fix the issue for C.

Given a broadcast connection with N ports on the left an M ports on the right,
we currently instantiate M delay reactors and broadcast to the delays. However,
it would be more efficient to only instantiate N delay reactors and broadcast
the output of the delays. This change implements this optimization and also
adds a new testcase.
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.

Looks good! Nice comprehensive tests. Thanks for dealing with this.

@edwardalee edwardalee merged commit d6e4160 into master Oct 11, 2021
@edwardalee edwardalee deleted the broadcast-after branch October 11, 2021 20:19
Soroosh129 added a commit that referenced this pull request Oct 12, 2021
cmnrd pushed a commit that referenced this pull request Oct 12, 2021
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.

After delays do not work on broadcast connections in the C target
2 participants