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

Stress-test threading/batchSize is always on #3205

Closed
2 tasks done
mattdowle opened this issue Dec 11, 2018 · 4 comments · Fixed by #4484
Closed
2 tasks done

Stress-test threading/batchSize is always on #3205

mattdowle opened this issue Dec 11, 2018 · 4 comments · Fixed by #4484
Labels
Milestone

Comments

@mattdowle
Copy link
Member

@mattdowle mattdowle commented Dec 11, 2018

Two current limitations to overcome :

  • getDTthreads() doesn't return more than omp_get_max_threads(). This prevents stress testing logic and bounds on small data by increasing number of threads. I wasn't able to reproduce #3204 until I mimicked getDTthreads() returning 20 (greater than omp_get_max_threads()) on my machine.
> setDTthreads(20)
> getDTthreads(verbose=TRUE)
omp_get_max_threads() = 8
omp_get_thread_limit() = 2147483647
DTthreads = 20
[1] 8
  • I'm currently deliberately allowing very small batch sizes (sometimes 2 rows) in order to stress-test the test-suite. Otherwise we have to add a lot of large data tests before threading kicks in. This is good, and #3204 quickly came to light because of this good strategy. However, this stress-testing is left in for release. So it could be that overhead on small data is larger. Need to add a flag that is on for stress-testing in the test-suite, but turned off by default in release. It may be that overhead is not actually increased on small data and that it can be left on -- need to test.
    Update: throttle added in #4484 solves this 2nd part.
@jangorecki
Copy link
Member

@jangorecki jangorecki commented Jan 22, 2019

was it switched off before 1.12.0?

@mattdowle
Copy link
Member Author

@mattdowle mattdowle commented Jan 22, 2019

Not as far as I know. I just tested now and I still see getDTthreads(verbose=TRUE) returning the same output as above and returning 8. I'll follow up to your findings in the other issue: #3300 (comment)

@jangorecki jangorecki added this to the 1.12.2 milestone Mar 5, 2019
@mattdowle
Copy link
Member Author

@mattdowle mattdowle commented Mar 5, 2019

PR #3435 changed setDTthreads() to allow up to omp_get_num_procs() which is the number of logical CPUs on the machine. On my laptop with 4 cores, this limit is 8 for example. This should be plenty to do the types of stress-mentioned in this issue.

@mattdowle mattdowle removed this from the 1.12.2 milestone Mar 5, 2019
@jangorecki jangorecki added this to the 1.12.4 milestone Jul 31, 2019
@mattdowle
Copy link
Member Author

@mattdowle mattdowle commented Sep 19, 2019

The 2nd part needs a flag adding, or a wrapper, to turn off openMP for small data. Related to #3438 and #3743.

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

Successfully merging a pull request may close this issue.

2 participants