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 sparse multiport iterator #29

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Use sparse multiport iterator #29

wants to merge 7 commits into from

Conversation

edwardalee
Copy link
Contributor

This makes use of the sparse iterator pull request 1298 of lingua-franca to get a performance improvement by using a sparse iterator over the input multiports.

@edwardalee edwardalee requested review from cmnrd and lhstrh July 18, 2022 18:40
@lhstrh lhstrh changed the title Use sparse multiport iterator. Use sparse multiport iterator Jul 23, 2022
@cmnrd
Copy link
Contributor

cmnrd commented Sep 25, 2023

Looks like we lost track of this PR. @edwardalee What should we do with this it? The @sparse annotation in Big does not compile anymore. Also, we have found a more general and more performant solution in the C++ target, that works on general multiports and does not require the @sparse annotation. The solution that Tassilo implemented in C++ (lf-lang/reactor-cpp#24) achieves more than 2x speedup in the Big benchmark. In comparison, the 15% improvement reported in lf-lang/lingua-franca#1298 seem marginal.

Should we drop this PR and also remove the @sparse annotation from LF. Or do you think it is worth fixing?

@edwardalee
Copy link
Contributor Author

I've lost track of this also. The sparse access technique in the C target is documented and tested. See https://www.lf-lang.org/reactor-c/da/d00/port_8h.html and https://github.com/lf-lang/lingua-franca/blob/master/test/C/src/multiport/Sparse.lf and https://github.com/lf-lang/lingua-franca/blob/master/test/C/src/multiport/SparseFallback.lf

These do not use the @sparse annotation. Apparently, that is obsolete.

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.

3 participants