From 4d681bf03b866526af5af579c58d0e0c8eb6545c Mon Sep 17 00:00:00 2001 From: Sebastian Fischer Date: Mon, 30 Oct 2023 11:53:20 +0100 Subject: [PATCH 1/3] feat: add .compile argument to crate function R disables jit-compilation of functions when changing their environment. Resolves Issue #94 --- NEWS.md | 7 ++++++- R/crate.R | 9 +++++++-- man/crate.Rd | 7 +++++-- tests/testthat/test_crate.R | 5 +++++ 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/NEWS.md b/NEWS.md index 8b778372..a1ef3a38 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,9 @@ -# mlr3misc 0.12.0-9000 +# mlr3misc 0.13.0-9000 + +* Added argument `.compile` to function `crate()` because R disables byte-code +compilation of functions when changing their enclosing environment + +# mlr3misc 0.13.0 * Updated default environment for `crate()` to `topenv()` (#86). * Added safe methods for dictionary retrieval (#83) diff --git a/R/crate.R b/R/crate.R index 9725efe6..9c950056 100644 --- a/R/crate.R +++ b/R/crate.R @@ -9,6 +9,8 @@ #' The objects, which should be visible inside `.fn`. #' @param .parent (`environment`)\cr #' Parent environment to look up names. Default to [topenv()]. +#' @param .compile (`logical(1)`)\cr +#' Whether to jit-compile the function. #' #' @export #' @examples @@ -24,8 +26,11 @@ #' z = 300 #' f = meta_f(1) #' f() -crate = function(.fn, ..., .parent = topenv()) { +crate = function(.fn, ..., .parent = topenv(), .compile = TRUE) { nn = map_chr(substitute(list(...)), as.character)[-1L] environment(.fn) = list2env(setNames(list(...), nn), parent = .parent) - .fn + if (.compile) { + .fn = compiler::cmpfun(.fn) + } + return(.fn) } diff --git a/man/crate.Rd b/man/crate.Rd index 013aa07d..d06c672e 100644 --- a/man/crate.Rd +++ b/man/crate.Rd @@ -4,7 +4,7 @@ \alias{crate} \title{Isolate a Function from its Environment} \usage{ -crate(.fn, ..., .parent = .GlobalEnv) +crate(.fn, ..., .parent = topenv(), .compile = TRUE) } \arguments{ \item{.fn}{(\verb{function()})\cr @@ -14,7 +14,10 @@ function to crate} The objects, which should be visible inside \code{.fn}.} \item{.parent}{(\code{environment})\cr -Parent environment to look up names. Default so the global environment.} +Parent environment to look up names. Default to \code{\link[=topenv]{topenv()}}.} + +\item{.compile}{(\code{logical(1)})\cr +Whether to jit-compile the function.} } \description{ Put a function in a "lean" environment that does not carry unnecessary baggage with it (e.g. references to datasets). diff --git a/tests/testthat/test_crate.R b/tests/testthat/test_crate.R index f0c5005d..4e692543 100644 --- a/tests/testthat/test_crate.R +++ b/tests/testthat/test_crate.R @@ -12,3 +12,8 @@ test_that("crate", { f = meta_f(1) expect_equal(f(), c(1, 200, 300)) }) + +test_that("compilation works", { + expect_error(compiler::disassemble(crate(function() NULL, .compile = FALSE)), regexp = "function is not compiled") + expect_error(compiler::disassemble(crate(function() NULL, .compile = TRUE)), regexp = NA) +}) From 0cbc314e746420de0affe35afa2d8f8a96c0785d Mon Sep 17 00:00:00 2001 From: Sebastian Fischer Date: Tue, 31 Oct 2023 18:34:16 +0100 Subject: [PATCH 2/3] crate always compiles when input function was compiled --- R/crate.R | 12 +++++++++++- man/crate.Rd | 4 +++- tests/testthat/test_crate.R | 5 +++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/R/crate.R b/R/crate.R index 9c950056..59a21186 100644 --- a/R/crate.R +++ b/R/crate.R @@ -11,6 +11,8 @@ #' Parent environment to look up names. Default to [topenv()]. #' @param .compile (`logical(1)`)\cr #' Whether to jit-compile the function. +#' In case the function is already compiled. +#' If the input function `.fn` is compiled, this has no effect, and the output function will always be compiled. #' #' @export #' @examples @@ -27,10 +29,18 @@ #' f = meta_f(1) #' f() crate = function(.fn, ..., .parent = topenv(), .compile = TRUE) { + assert_flag(.compile) nn = map_chr(substitute(list(...)), as.character)[-1L] environment(.fn) = list2env(setNames(list(...), nn), parent = .parent) - if (.compile) { + if (.compile || is_compiled(.fn)) { .fn = compiler::cmpfun(.fn) } return(.fn) } + +is_compiled = function(x) { + tryCatch({ + capture.output(compiler::disassemble(x)) + TRUE + }, error = function(e) FALSE) +} diff --git a/man/crate.Rd b/man/crate.Rd index d06c672e..a84039ff 100644 --- a/man/crate.Rd +++ b/man/crate.Rd @@ -17,7 +17,9 @@ The objects, which should be visible inside \code{.fn}.} Parent environment to look up names. Default to \code{\link[=topenv]{topenv()}}.} \item{.compile}{(\code{logical(1)})\cr -Whether to jit-compile the function.} +Whether to jit-compile the function. +In case the function is already compiled. +If the input function \code{.fn} is compiled, this has no effect, and the output function will always be compiled.} } \description{ Put a function in a "lean" environment that does not carry unnecessary baggage with it (e.g. references to datasets). diff --git a/tests/testthat/test_crate.R b/tests/testthat/test_crate.R index 4e692543..d0bbb90e 100644 --- a/tests/testthat/test_crate.R +++ b/tests/testthat/test_crate.R @@ -16,4 +16,9 @@ test_that("crate", { test_that("compilation works", { expect_error(compiler::disassemble(crate(function() NULL, .compile = FALSE)), regexp = "function is not compiled") expect_error(compiler::disassemble(crate(function() NULL, .compile = TRUE)), regexp = NA) + + f = function() NULL + fc = compiler::cmpfun(f) + + expect_true(is_compiled(fc)) }) From 691186c02415338315906fa7cb951beb73a51e45 Mon Sep 17 00:00:00 2001 From: Sebastian Fischer Date: Wed, 17 Jan 2024 15:52:46 +0100 Subject: [PATCH 3/3] fix crate --- R/crate.R | 3 ++- tests/testthat/test_crate.R | 10 +++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/R/crate.R b/R/crate.R index 59a21186..aaf9cddc 100644 --- a/R/crate.R +++ b/R/crate.R @@ -30,9 +30,10 @@ #' f() crate = function(.fn, ..., .parent = topenv(), .compile = TRUE) { assert_flag(.compile) + .compile = .compile || is_compiled(.fn) nn = map_chr(substitute(list(...)), as.character)[-1L] environment(.fn) = list2env(setNames(list(...), nn), parent = .parent) - if (.compile || is_compiled(.fn)) { + if (.compile) { .fn = compiler::cmpfun(.fn) } return(.fn) diff --git a/tests/testthat/test_crate.R b/tests/testthat/test_crate.R index d0bbb90e..7fd8addd 100644 --- a/tests/testthat/test_crate.R +++ b/tests/testthat/test_crate.R @@ -14,11 +14,7 @@ test_that("crate", { }) test_that("compilation works", { - expect_error(compiler::disassemble(crate(function() NULL, .compile = FALSE)), regexp = "function is not compiled") - expect_error(compiler::disassemble(crate(function() NULL, .compile = TRUE)), regexp = NA) - - f = function() NULL - fc = compiler::cmpfun(f) - - expect_true(is_compiled(fc)) + expect_false(is_compiled(crate(function() NULL, .compile = FALSE))) + expect_true(is_compiled(crate(function() NULL, .compile = TRUE))) + expect_true(is_compiled(crate(compiler::cmpfun(function() NULL), .compile = FALSE))) })