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

simulation timeout thread is started unconditionally, leading to unnecessary overhead #253

Closed
wookietreiber opened this issue Sep 7, 2020 · 2 comments · Fixed by #254
Closed

Comments

@wookietreiber
Copy link
Contributor

We realized that for very short (few seconds) simulations, there is considerable overhead:

$ time strace -c -f -p 5634
...
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 65.13    0.006429           2      2997       699 futex
 32.42    0.003200          33        96           madvise
  1.62    0.000160           1        96           clone
  0.83    0.000082           0        96           set_robust_list
  0.00    0.000000           0         2           write
------ ----------- ----------- --------- --------- ----------------
100.00    0.009871                  3287       699 total

real    0m3.984s
user    0m0.027s
sys    0m0.070s

There's a lot going on in thread creation (clone) and coordination (futex) that is not necessary, especially not for a single-core run. This is due to def simulate in spotpy/algorithms/_algorithm.py.

My proposal is to only spawn the thread if self.sim_timeout is set to avoid the above-mentioned overhead. I'll prepare a pull request.

@thouska
Copy link
Owner

thouska commented Sep 7, 2020

Thanks for testing this! Indeed this whole thread creation section might be not necessary anymore, as it was only included as a workaround for a python2/3 compability issue. Your proposal sounds good, I would highly appreciate your pull-request on this.

@wookietreiber
Copy link
Contributor Author

@thouska Great, please review #254.

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 a pull request may close this issue.

2 participants