Very nice! Will take a look at this ASAP and write back.

just pushed update

• rebased to recent master (previous inline comments are gone, but all were addressed)
• uses memmove instead of memcpy
• fixed compiler warning
• proper way of nested parallelism
• now parallel will work with verbose=T when there is no print inside parallel region
• reorg code for less repetition, C level wrappers
• some segfault fixes

code for benchmarking landed in https://gist.github.com/jangorecki/96d4956dbf9cf0de118655ecbaca9b73
may require change from exact=TRUE to algo="exact"

Codecov Report

❗ No coverage uploaded for pull request base (master@c434610). Click here to learn what that means.
The diff coverage is 95.85%.

@@            Coverage Diff            @@
##             master    #2961   +/-   ##
=========================================
  Coverage          ?   93.69%           
=========================================
  Files             ?       65           
  Lines             ?    11949           
  Branches          ?        0           
=========================================
  Hits              ?    11196           
  Misses            ?      753           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c434610...31dca17. Read the comment docs.

Starting to look through this. Just quick initial comments :
Can't see a NEWS item.
Suggest adding "experimental function names and arguments; new in v1.12.0" to the top of the manual pages so that it's clear that it's new and might change.
I saw two malloc calls that don't have return value checked. Those need if (==NULL) error("alloc failed...) adding, or change to R_alloc() is preferable where possible.

@mattdowle Thanks for looking into this. NEWS entry added, also checks for malloc. I did not use R_alloc because those functions are attempting to be R agnostic.

As of now, it might happened that error("could not allocate...") will be raise from parallel region. The one defined in frollfunR when user provides 2+ columns to roll on AND/OR 2+ window sizes for rolling. There is nested parallel region in fadaptiverollmeanFast function where allocation happens but allocation is made before parallel region is entered there. Still it might be in parallel region of parent function frollfunR. Do I understand correctly this is not safe? If so I had to either

• move those allocations to frollfunR before entering topmost parallel region. One of the allocation occurs only if NAs were detected so might ended up unnecessarily allocating extra space that won't be used.
• implement error handling from parallel region by returning some kind of ans_t struct storing status, error message, and results (when no exception occurred).

I think the latter one is way to go, but it will be definitely more complex change comparing to just moving allocation. Looking forward for your feedback.

Also, before merging we may want to move tests from own test file to tests.Rraw.

Yes, the malloc itself is thread-safe (thread lock inside it). It's just the R API error() that isn't. Allocating up front before parallel region is usually best. Even a large malloc() will be almost instant so there's no time lost if it's not used; just higher memory usage and the possibility of failing to allocate something that isn't needed. Only at the point of touching allocated memory does anything happen (e.g. calloc vs malloc).

I went for the second option. First option would be easier but it would be polluting code as those two mallocs were specific to the algorithm (as of now only 1 out of 4 rolling statistics algos in this PR). Same frollfunR is going to be used for other rolling statistics so polluting it with pointers needed for specific algos in specific statistics won't scale.

Added exception handling so fails of malloc are handled using this exception handling mechanism. Additionally one type of console output (before printed with verbose) is now warning. It should be warning but I simplified it to not need to handle exceptions. Unit test for that verbose output had comment # this could be warning but no warns from plain C code. Because now we have exception handling mechanism I turned those console outputs into warnings. There is a limit on how long error/warning string can be, set to 255, which is safe because that string does not depend on user input.
Build and install using gcc-7 going perfectly well. When using cc(F) with same gcc I am getting following warning: SOLVED

Also checked that wrapping double vector for answer into struct does not degrade performance.