Update Synthesiser To Throw Exception When Calling setNumThreads With Threading Disabled

[cwarden]

[codecov[bot]]

Codecov Report

Merging #2133 (2a0bef1) into master (caea609) will increase coverage by 0.00%. The diff coverage is 91.68%.

@@           Coverage Diff           @@
##           master    #2133   +/-   ##
=======================================
  Coverage   75.26%   75.26%           
=======================================
  Files         452      450    -2     
  Lines       27357    27281   -76     
=======================================
- Hits        20589    20533   -56     
+ Misses       6768     6748   -20     

Impacted Files	Coverage Δ
src/ast/analysis/typesystem/Type.cpp	45.84% <0.00%> (-0.10%)	:arrow_down:
src/ast/transform/FoldAnonymousRecords.h	50.00% <ø> (ø)
src/ast/transform/MagicSet.h	63.26% <ø> (ø)
src/ast/utility/SipsMetric.cpp	15.60% <0.00%> (+0.15%)	:arrow_up:
src/ast/utility/SipsMetric.h	13.33% <0.00%> (ø)
src/ast/utility/Utils.h	100.00% <ø> (ø)
src/ast2ram/utility/TranslatorContext.h	100.00% <ø> (ø)
src/include/souffle/utility/FunctionalUtil.h	100.00% <ø> (ø)
src/include/souffle/utility/span.h	100.00% <ø> (ø)
src/ast/transform/ReorderLiterals.cpp	71.42% <50.00%> (+1.15%)	:arrow_up:
... and 44 more

[cwarden]

When the c++ program is generated without parallelism enabled, calling setNumThreads doesn't do anything, afaict. This change makes it fail by throwing an exception. Are there any downsides to this approach?

[quentin]

Isn't it possible to build Souffle with OpenMP disabled. Then use that Souffle to produce c++ code with all the necessary openmp directives. And finally build the generated code with OpenMP enabled and run it with parallel jobs enabled.

[cwarden]

Isn't it possible to build Souffle with OpenMP disabled. Then use that Souffle to produce c++ code with all the necessary openmp directives. And finally build the generated code with OpenMP enabled and run it with parallel jobs enabled.

I'm not sure. I'm trying to address the case where souffle is used to generate c++ without threading enabled, and the generated code is then compiled with OpenMP. If you call setNumThreads, then getNumThreads, it looks like the souffle program will be using multiple threads. But only a single thread is actually being used so I'd like it to fail to more noisily, instead of just taking a lot longer to run.

[quentin]

In that case the condition should be in the generated code and evaluated by the generated code, instead of being evaluated at synthesizing time. That would be easy using the #ifndef _OPENMP in the generated code as in:

https://github.com/souffle-lang/souffle/blob/dc7d595e60d7fc397175c7df5d59e33f18a034a1/src/synthesiser/Synthesiser.cpp#L2668-L2670

Or here:

https://github.com/souffle-lang/souffle/blob/dc7d595e60d7fc397175c7df5d59e33f18a034a1/src/synthesiser/Synthesiser.cpp#L2963-L2965

[cwarden]

Synthesizing time is when the generated code will either support parallelism or not so I think we need to evaluate the condition at that time. Checking whether _OPENMP is defined when generated code is compiled doesn't work because the code can be compiled with OpenMP support, but the generated code won't take advantage of it.

[b-scholz]

I believe we need further investigation. What is wrong with the current behaviour?

Ideally, we enclose OpenMP operations with #ifdefs, checking whether OpenMP is enabled in the compilation environment. If not, the users should not be able to specify the -j option. We have not looked into the whole stack (runtime and interpreter) for a while.

[cwarden]

The issue that I've run into is that code generated with souffle -g provides setNumThreads whether -j is specified or not. But if -j is not used at synthesis time and you call setNumThreads to use more than one thread, it silently fails. You can call getNumThreads after setNumThreads, and it will return the number of threads that you passed to setNumThreads, but the application will still be using a single thread.

This pull request offers a possible solution, failing noisily if you call setNumThreads when the program was synthesized without -j.

[quentin]

I am still not convinced the suggested change is appropriate. Maybe adding a few test cases would help.

[quentin]

Hi, I am closing this this pull request because it has been inactive for over 3 years.