Skip to content
New issue

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

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

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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