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

Minimize imports in an effort to lower simulation setup time #992

Merged
merged 1 commit into from
Mar 4, 2024

Conversation

LukasVik
Copy link
Contributor

@LukasVik LukasVik commented Feb 29, 2024

These changes represent a 21% reduction of execution time for the 460 test cases in hdl-modules repo: https://github.com/hdl-modules/hdl-modules For some small testbenches, the reduction is as much as 42%. Note that this repo contains only small to medium sized testbenches (1-10 seconds, with most around 1-4 seconds).

This is all using the GHDL simulator with GCC backend on Linux. With ModelSim Intel FPGA starter edition 2020.1, the reductions is negligible (1-2%).

This change:

  1. Stop using "context" in all VUnit packages. In most packages, importing with context includes a lot more than what is actually used.

  2. Stop using "use work.x_pkg.all" in cases where only 1-3 things are used from the package. Instead, one explicit "use" clause for the things that are actually used.

    • Not done for the most common packages (run_pkg, check_pkg, logger_pkg).

    • Not done for includes from very small packages.

  3. For packages that have head and body in different files, move all imports to the head file. As they were, split between the files, there was a lot of overlap of imports, and it what hard to get an overview of what is actually imported and what is actually used.

The first point represents roughly 80% of the performance gain.

Note that in order to reach the performance gains listed above, the corresponding changes have to be made to all testbenches and simulation code in the user repo also. If the testbench includes "vunit_context", "com_context" etc, the performance gains will not be realized since everything will be imported anyway.

@LarsAsplund
Copy link
Collaborator

  1. Do you have some tool support for finding out what includes are used and what are not? Or is it brute force?

  2. Generally speaking I would prefer explicit imports if VHDL had a more compact syntax for importing a list of items from the same package. Just like Python has for x import a, b, c

  3. That is probably the easiest approach. However, there can be "private" packages that only need to be included in the body and shouldn't be visible in user facing head.

I will accept the PR when it is green

These changes represent a 21% reduction of execution time for the 460 test cases in hdl-modules repo: https://github.com/hdl-modules/hdl-modules
For some small testbenches, the reduction is as much as 42%.
Note that this repo contains only small to medium sized testbenches (1-10 seconds, with most around 1-4 seconds).

This is all using the GHDL simulator with GCC backend on Linux. With ModelSim Intel FPGA starter edition 2020.1, the reductions is negligible (1-2%).

This change:

1. Stop using "context" in all VUnit packages.
   In most packages, importing with context includes a lot more than what is actually used.

2. Stop using "use work.x_pkg.all" in cases where only 1-3 things are used from the package.
   Instead, one explicit "use" clause for the things that are actually used.

   * Not done for the most common packages (run_pkg, check_pkg, logger_pkg).

   * Not done for includes from very small packages.

3. For packages that have head and body in different files, move all imports to the head file.
   As they were, split between the files, there was a lot of overlap of imports, and it what hard to get an overview of what is actually imported and what is actually used.

The first point represents roughly 80% of the performance gain.

Note that in order to reach the performance gains listed above, the corresponding changes have to be made to all testbenches and simulation code in the user repo also.
If the testbench includes "vunit_context", "com_context" etc, the performance gains will not be realized since everything will be imported anyway.
@LukasVik
Copy link
Contributor Author

LukasVik commented Mar 4, 2024

Hello Lars!

  1. I do not, I did it all by hand. It took a while 🙈. Though I would love to have such a tool available. Something like isort in Python that groups and sorts the import, and some pylint equivalent that checks for unused imports. That would be lovely. Could probably be built on top of rust_hdl if someone wants to put in the work.
  2. I agree fully. I think in general in programming keeping the scope clean, i.e. importing as little as possible, is a good thing. So the programming language should help with that. Would be great if VHDL "use" could be done in the compact way that you suggest.
  3. Yes that is true. I see no change like this in my commit, but we can keep it in mind for the future!

Thank you!

@LukasVik
Copy link
Contributor Author

LukasVik commented Mar 4, 2024

I have pushed a fix so that it is now green!

@LarsAsplund LarsAsplund merged commit 594a2c0 into VUnit:master Mar 4, 2024
14 checks passed
@umarcor
Copy link
Member

umarcor commented Mar 11, 2024

See #997.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants