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

micro-optimization: input vector for poly::get_variables() #571

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tueda
Copy link
Collaborator

@tueda tueda commented Nov 7, 2024

poly::get_variables() does not need a temporary copy of the input vector containing pointers to function arguments. Passing it by reference rather than by value avoids a heap allocation and a memory copy.

Pass the input vector by reference rather than by value.
@coveralls
Copy link

coveralls commented Nov 7, 2024

Coverage Status

coverage: 49.988% (+0.002%) from 49.986%
when pulling 506e5d9 on tueda:improve-get_variables
into 61ecfdd on vermaseren:master.

Reserving capacity for the input vector with a fixed size to be passed
to get_variables() prevents unnecessary reallocations during push_back
operations.
@tueda tueda changed the title micro-optimization: stop copying input vector for poly::get_variables() micro-optimization: input vector for poly::get_variables() Nov 7, 2024
@tueda
Copy link
Collaborator Author

tueda commented Nov 7, 2024

I've reserved capacity for vectors passed to poly::get_variables() when they are known to have a fixed size. Without reserving capacity, for example,

vector<WORD *> e;
e.push_back(a);
e.push_back(b);
poly::get_variables(BHEAD e, false, false);

calls new and delete twice each, because typically the capacity grows as 0, 1, 2, 4, ..., so a reallocation is needed.

If we can use C++11, then this could be rewritten with std::array and by templatizing:

    template <typename Container>
    static void get_variables (PHEAD const Container &, bool, bool);

(Optionally, with_arghead and bool sort_vars can be converted to template parameters, which allows more compiler optimization.)

@jodavies
Copy link
Collaborator

jodavies commented Nov 8, 2024

Could you measure any improvement from these in poly benches? #297 could go in with this also in principle?

@tueda
Copy link
Collaborator Author

tueda commented Nov 9, 2024

The improvement in speed is negligible. So, practically, the difference is only in writing code in a more proper style.

  Revision: v5.0.0-beta.1-71-g61ecfdd (master)
  Command being timed: "/home/tueda/work/form/build/sources/form ratfuntest.frm"
  n = 10                                            Mean (          SD)            Min            Max
  User time (seconds):                            14.139 (       0.028)          14.08          14.17
  System time (seconds):                           0.508 (       0.021)           0.48           0.55

  Revision: v5.0.0-beta.1-71-g61ecfdd (master)
  Command being timed: "/home/tueda/work/form/build/sources/form test-mbox1l.frm"
  n = 10                                            Mean (          SD)            Min            Max
  User time (seconds):                            42.063 (       0.018)          42.04           42.1
  System time (seconds):                           0.030 (       0.011)           0.01           0.05

vs.

  Revision: v5.0.0-beta.1-73-g506e5d9 (improve-get_variables)
  Command being timed: "/home/tueda/work/form/build/sources/form ratfuntest.frm"
  n = 10                                            Mean (          SD)            Min            Max
  User time (seconds):                            14.170 (       0.028)          14.13          14.21
  System time (seconds):                           0.513 (       0.025)           0.47           0.56

  Revision: v5.0.0-beta.1-73-g506e5d9 (improve-get_variables)
  Command being timed: "/home/tueda/work/form/build/sources/form test-mbox1l.frm"
  n = 10                                            Mean (          SD)            Min            Max
  User time (seconds):                            42.142 (       0.021)          42.11          42.17
  System time (seconds):                           0.024 (       0.010)           0.01           0.04

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