Skip to content

Conversation

@jangorecki
Copy link
Member

@jangorecki jangorecki commented Jun 18, 2020

timings in #4200 (comment)
probably qualifies to close #4200

src/subset.c Outdated
#define PARLOOP(_NAVAL_) \
if (anyNA) { \
_Pragma("omp parallel for num_threads(getDTthreads(n, true))") \
_Pragma("omp parallel for num_threads(nth)") \
Copy link
Member Author

@jangorecki jangorecki Jun 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • tried turning it into omp parallel for if (nth > 1) num_threads... but it makes no difference.
  • tried using if (nth > 1) _Pragma("...") but it makes result a garbage

so I ended up duplicating whole macro for single threaded mode

@codecov
Copy link

codecov bot commented Jun 18, 2020

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.61%. Comparing base (6f360be) to head (20d4855).
Report is 1591 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4558   +/-   ##
=======================================
  Coverage   99.61%   99.61%           
=======================================
  Files          73       73           
  Lines       14118    14119    +1     
=======================================
+ Hits        14063    14064    +1     
  Misses         55       55           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jangorecki jangorecki requested a review from mattdowle June 18, 2020 17:27
@jangorecki
Copy link
Member Author

jangorecki commented Jun 18, 2020

I also tried simpler approach, eliminating _Pragma("...") in favor of real #pragma but that does not help. See https://github.com/Rdatatable/data.table/compare/dogroups-overhead2?expand=1
So the issue is not about _Pragma() but an overhead of just single threaded openmp, see related issue #4527.

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

Labels

atime Requests related to adding/improving/monitoring performance regression tests via atime.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows 10 is faster with -fno-openmp than setDTthreads(1) on many repeated calls groupby with dogroups (R expression) performance regression

3 participants