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

Saving compiled code #16

Merged
merged 25 commits into from
Nov 26, 2020
Merged

Conversation

jranke
Copy link
Contributor

@jranke jranke commented Nov 26, 2020

This PR adds functionality for writing CFunc objects. The previously existing code defining writeDynLib and readDynLib was not functional for several reasons. Therefore, a new approach was developed (see commit messages for details), working in two steps.

A precondition for this to work is that the finalizer created by cfunction does not remove the DLL that libLFile in the environment of the function points to. Therefore, cfunction was modified to only remove libLFile_orig upon garbage collection or session termination, but to leave libLFile which will point to a different location after using moveDLL.

moveDLL moves the DLL to a user defined location for an existing CFunc object, i.e. it modifies its argument. Having a separate step for this allows for explicit control over unloading and overwriting of any pre-existing DLLs. Care is taken that no loaded DLL gets overwritten, which would cause a segfault.

writeCFunc writes a CFunc object, but takes care that the DLL has been moved already, as otherwise it would be garbage collected, making the operation pointless. Side note: We could also recompile when reading...

readCFunc reads a CFunc object stored using writeCFunc and restores the pointer (symbol info) in the function body.

cfunction also gains an argument name that will be used as the function name in the source code if the signature is not specified as a named list (sorry for not splitting out this as a separate issue with a separate PR).

And please excuse the whitespace changes in the docs (man/utilities.R). I have a hard time checking .Rd files when indentation and whitespace is too random.

All the new functionality is tested in example code and tiny tests.

Closes #13.

Instead of the argument "file", there are now two arguments "bname" and
"directory" (see updated documentation in man/utilities.Rd).

However, we still have remainders of the old DLL locations and names in
after writing and reading, see eddelbuettel#13.
cfunction and R/utilities.R:
- use DLLpath instead of DLLname if it is a path

cfunction:
- Add 'name' argument to make it possible to specify a function name
  that is used in the source code of the function
- In the finalizer, remove only 'libLFile_orig' which always points to
  the original DLL file, while 'libLFile' can be changed by
  'writeDynLib' to a user defined location

R/utilities.R (writeDynLib and readDynLib):
- Do not use additional attributes for DLL path, DLL name and function
  name, but only modify this information in the function environment
  in 'writeDynLib', and restore this information in 'readDynLib'
- Remove code that was meant to support 'CFuncList' objects in
  'writeDynLib' and 'readDynLib'
Unfortunately compiling cfunction objects does not work in the R
testing environment. The error message upon compiling the first
cfunction in tests/cfunction.R:

```
* checking tests ...
  Running ‘cfunction.R’
 ERROR
Running the tests in ‘tests/cfunction.R’ failed.
Last 13 lines of output:
    4:       DOUBLE PRECISION x(*)
    5:
    6:       integer i
    7:       do 1 i=1, n(1)
    8:     1 x(i) = x(i)**3
    9:
   10:       RETURN
   11:       END
   12:

  Compilation ERROR, function(s)/method(s) not created!
  Error in compileCode(f, code, language, verbose) :
    Error in file(filename, "r", encoding = encoding) :   cannot open the connectionCalls: local ... eval.parent -> eval -> eval -> eval -> eval -> source -> fileIn addition: Warning message:In file(filename, "r", encoding = encoding) :  cannot open file 'startup.Rs': No such file or directoryExecution halted
  Calls: cfunction -> compileCode
  Execution halted
```
Instead of the argument "file", there are now two arguments "bname" and
"directory" (see updated documentation in man/utilities.Rd).

However, we still have remainders of the old DLL locations and names in
after writing and reading, see eddelbuettel#13.
cfunction and R/utilities.R:
- use DLLpath instead of DLLname if it is a path

cfunction:
- Add 'name' argument to make it possible to specify a function name
  that is used in the source code of the function
- In the finalizer, remove only 'libLFile_orig' which always points to
  the original DLL file, while 'libLFile' can be changed by
  'writeDynLib' to a user defined location

R/utilities.R (writeDynLib and readDynLib):
- Do not use additional attributes for DLL path, DLL name and function
  name, but only modify this information in the function environment
  in 'writeDynLib', and restore this information in 'readDynLib'
- Remove code that was meant to support 'CFuncList' objects in
  'writeDynLib' and 'readDynLib'
Unfortunately compiling cfunction objects does not work in the R
testing environment. The error message upon compiling the first
cfunction in tests/cfunction.R:

```
* checking tests ...
  Running ‘cfunction.R’
 ERROR
Running the tests in ‘tests/cfunction.R’ failed.
Last 13 lines of output:
    4:       DOUBLE PRECISION x(*)
    5:
    6:       integer i
    7:       do 1 i=1, n(1)
    8:     1 x(i) = x(i)**3
    9:
   10:       RETURN
   11:       END
   12:

  Compilation ERROR, function(s)/method(s) not created!
  Error in compileCode(f, code, language, verbose) :
    Error in file(filename, "r", encoding = encoding) :   cannot open the connectionCalls: local ... eval.parent -> eval -> eval -> eval -> eval -> source -> fileIn addition: Warning message:In file(filename, "r", encoding = encoding) :  cannot open file 'startup.Rs': No such file or directoryExecution halted
  Calls: cfunction -> compileCode
  Execution halted
```
This came back with the merge commit and I didn't notice
By separating the process, it is easier to catch potential errors when
moving the DLL around (now done in moveDLL()). Also, the files can get
independent file names in this way. Only writing CFunc objects is
currently supported.
@eddelbuettel
Copy link
Owner

eddelbuettel commented Nov 26, 2020

I cannot merge this as is as it changes published interfaces.

My bad. Apparently only internal functions.

Would you consider rebasing or would you mind squashing? This is one PR. I am not sure we need 30 or so commits here.

@jranke
Copy link
Contributor Author

jranke commented Nov 26, 2020

I don't mind squashing at all.

Copy link
Owner

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

Nice and careful work. Good to have the tests.

DESCRIPTION Show resolved Hide resolved
DESCRIPTION Show resolved Hide resolved
"registerPlugin",
"rcpp"
"rcpp",
"writeCFunc"
Copy link
Owner

Choose a reason for hiding this comment

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

Do these actually persist C-level functions across sessions now? We were never clear if that could be done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's exactly the point. I just ran

x <- as.numeric(1:10)
n <- as.integer(10)

code <- "
      integer i
      do 1 i=1, n(1)
    1 x(i) = x(i)**3
"
cubefn <- cfunction(signature(n="integer", x="numeric"), code,
  convention=".Fortran")
code(cubefn)

cubefn(n, x)$x

moveDLL(cubefn, name = "cubefn", directory = "~/dll")
path <- file.path("~/dll", "cubefn.rda")
writeCFunc(cubefn, path)

in one session and

path <- file.path("~/dll", "cubefn.rda")
cfn <- readCFunc(path)
cfn(3, 1:3)$x

and got

> cfn(3, 1:3)$x
[1]  1  8 27

using 0.3.16.2 on Linux

\o/

@eddelbuettel eddelbuettel merged commit 184abe8 into eddelbuettel:master Nov 26, 2020
@eddelbuettel
Copy link
Owner

I added one quick commit to master to update NEWS (and roll the minor). If you have more work planned please pull and rebase as needed.

eddelbuettel added a commit that referenced this pull request Nov 26, 2020
@eddelbuettel
Copy link
Owner

(And two more)

@jranke jranke deleted the saving_compiled_code branch November 27, 2020 19:58
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 this pull request may close these issues.

Saving compiled code from cfunction objects in user defined locations is not supported
2 participants