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

Modules (form) #430

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Modules (form) #430

wants to merge 1 commit into from

Conversation

burrbull
Copy link
Member

Closes #424 .

r? @therealprof

Speed tests are needed

@burrbull burrbull requested a review from a team as a code owner January 14, 2020 07:58
@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Jan 14, 2020
@therealprof
Copy link
Contributor

That's very nice! Need to have a closer look this evening.

@burrbull
Copy link
Member Author

@therealprof you promised to look

@therealprof
Copy link
Contributor

Didn't get to it, sorry. Maybe I can squeeze it in later today.

@therealprof
Copy link
Contributor

I had a look. svd2rust runtime goes through the roof with the -foption, it takes 10x as long in my tests to dump the code. There's no change in build/check times of the generated code.

It would be great to see some noticeable benefit somewhere.

@therealprof
Copy link
Contributor

The Module is a rather weird implementation causing quite a few clippy lints and warnings in case the superfluous #[allow(dead_code)] are removed.

We should also get rid of all the panics and instead implement proper error handling.

@burrbull burrbull marked this pull request as draft April 17, 2021 04:06
@duskmoon314
Copy link
Contributor

Any plan on this?

Recently I tried the logic of xtask in esp-pacs on d1-pac, which first generates lib.rs and then transforms it into modules in src. Much simpler than this implementation in my mind.
esp-pacs
d1-pac

it takes 10x as long in my tests to dump the code.

I am not familiar with improving performance. Taking a simple look at the flame graph of executing xtask gen d1, I found that form::create_directory_structure is indeed costly. But I don't feel it annoying since I always use form after generating code with svd2rust.

The flame graph:
flamegraph

@rmsyn
Copy link
Contributor

rmsyn commented Jan 29, 2025

@burrbull you mentioned that you would like me to take a look at bringing this PR over the finish line.

Happy to do so. Would you mind rebasing on the latest HEAD to make finishing the work a bit easier?

@burrbull
Copy link
Member Author

@burrbull you mentioned that you would like me to take a look at bringing this PR over the finish line.

Happy to do so. Would you mind rebasing on the latest HEAD to make finishing the work a bit easier?

I need to remember what I've tried to do and try to solve perfomance issues.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate form
5 participants