Skip to content

Commit

Permalink
Don't emit the record separator when testing mark_() directly (#133)
Browse files Browse the repository at this point in the history
Because we aren't capturing the emitted messages anywhere (unlike in `mark()` with `with_gcinfo()`), which results in the record separators showing up the next time output is sent to the screen, which is generally when `devtools::test()` is running
  • Loading branch information
DavisVaughan committed May 3, 2023
1 parent bb69d15 commit f938a66
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 11 deletions.
2 changes: 1 addition & 1 deletion R/mark.R
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ mark <- function(..., min_time = .5, iterations = NULL, min_iterations = 1,
error <- NULL
gc_msg <- with_gcinfo({
tryCatch(error = function(e) { e$call <- NULL; error <<- e},
res <- .Call(mark_, exprs[[i]], env, min_time, as.integer(min_iterations), as.integer(max_iterations))
res <- .Call(mark_, exprs[[i]], env, min_time, as.integer(min_iterations), as.integer(max_iterations), TRUE)
)
})
if (!is.null(error)) {
Expand Down
13 changes: 9 additions & 4 deletions src/mark.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ double get_overhead(SEXP env) {
return overhead;
}

SEXP mark_(SEXP expr, SEXP env, SEXP min_time, SEXP min_itr, SEXP max_itr) {
SEXP mark_(SEXP expr, SEXP env, SEXP min_time, SEXP min_itr, SEXP max_itr, SEXP gcinfo) {
R_xlen_t min_itr_ = INTEGER(min_itr)[0];
R_xlen_t max_itr_ = INTEGER(max_itr)[0];
double min_time_ = REAL(min_time)[0];
Rboolean gcinfo_ = LOGICAL(gcinfo)[0];

SEXP out = PROTECT(Rf_allocVector(REALSXP, max_itr_));

Expand All @@ -39,8 +40,12 @@ SEXP mark_(SEXP expr, SEXP env, SEXP min_time, SEXP min_itr, SEXP max_itr) {

long double elapsed = expr_elapsed_time(expr, env);

// 1E is record separator
REprintf("\x1E");
if (gcinfo_) {
// We don't emit the separator during low level testing of `mark_()`
// 1E is record separator
REprintf("\x1E");
}

REAL(out)[i] = elapsed - overhead;
total+=elapsed;

Expand Down Expand Up @@ -119,7 +124,7 @@ extern SEXP bench_process_memory_(void);
extern SEXP bench_load_average_(void);

static const R_CallMethodDef CallEntries[] = {
{"mark_", (DL_FUNC) &mark_, 5},
{"mark_", (DL_FUNC) &mark_, 6},
{"system_time_", (DL_FUNC) &system_time_, 2},
{"bench_process_memory_", (DL_FUNC) &bench_process_memory_, 0},
{"bench_load_average_", (DL_FUNC) &bench_load_average_, 0},
Expand Down
12 changes: 6 additions & 6 deletions tests/testthat/test-mark.R
Original file line number Diff line number Diff line change
@@ -1,30 +1,30 @@
describe("mark_", {
it("If min_time is Inf, runs for max_iterations", {
res <- .Call(mark_, quote(1), new.env(), Inf, as.integer(0), as.integer(10))
res <- .Call(mark_, quote(1), new.env(), Inf, as.integer(0), as.integer(10), FALSE)
expect_equal(length(res), 10)

res <- .Call(mark_, quote(1), new.env(), Inf, as.integer(0), as.integer(20))
res <- .Call(mark_, quote(1), new.env(), Inf, as.integer(0), as.integer(20), FALSE)
expect_equal(length(res), 20)
})

it("If min_time is 0, runs for min_iterations", {
res <- .Call(mark_, quote(1), new.env(), 0, as.integer(1), as.integer(10))
res <- .Call(mark_, quote(1), new.env(), 0, as.integer(1), as.integer(10), FALSE)
expect_equal(length(res), 1)

res <- .Call(mark_, quote(1), new.env(), 0, as.integer(5), as.integer(10))
res <- .Call(mark_, quote(1), new.env(), 0, as.integer(5), as.integer(10), FALSE)
expect_equal(length(res), 5)
})

it("If min_time is 0, runs for min_iterations", {
res <- .Call(mark_, quote({i <- 1; while(i < 10000) i <- i + 1}), new.env(), .1, as.integer(1), as.integer(1000))
res <- .Call(mark_, quote({i <- 1; while(i < 10000) i <- i + 1}), new.env(), .1, as.integer(1), as.integer(1000), FALSE)

expect_gte(length(res), 1)
expect_lte(length(res), 1000)
})

it("Evaluates code in the environment", {
e <- new.env(parent = baseenv())
res <- .Call(mark_, quote({a <- 42}), e, Inf, as.integer(1), as.integer(1))
res <- .Call(mark_, quote({a <- 42}), e, Inf, as.integer(1), as.integer(1), FALSE)
expect_equal(e[["a"]], 42)
})
})
Expand Down

0 comments on commit f938a66

Please sign in to comment.