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

Simpler approaching to removing call suffix #172

Merged
merged 5 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# profvis (development version)

* New technique for trimming uninteresting frames from the stack (#130). This requires a new evaluationg model where the code you supply to `profvis()` is turned into the body of a zero-argument anonymous function that is then called by profvis. This subtly changes the semantics of evaluation, but it's very unlikely to affect the type of code that you are typically profiling.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* New technique for trimming uninteresting frames from the stack (#130). This requires a new evaluationg model where the code you supply to `profvis()` is turned into the body of a zero-argument anonymous function that is then called by profvis. This subtly changes the semantics of evaluation, but it's very unlikely to affect the type of code that you are typically profiling.
* New technique for trimming uninteresting frames from the stack (#130). This requires a new evaluation model where the code you supply to `profvis()` is turned into the body of a zero-argument anonymous function that is then called by profvis. This subtly changes the semantics of evaluation, but it's very unlikely to affect the type of code that you are typically profiling.

* Bundled `highlight.js` updated to the latest version 11.10.0.
* The CSS for profvis code is scoped so that it does not affect other blocks of code, such as those from RMarkdown or Quarto (@wch, #140).
* profvis now relies on R 4.0.0.
Expand Down
31 changes: 20 additions & 11 deletions R/profvis.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,15 @@
#' corresponding data file as the `prof_input` argument to
#' `profvis()`.
#'
#' @param expr Expression to profile. Not compatible with `prof_input`.
#' @param expr Expression to profile. The expression will be turned into the
#' body of a zero-argument anonymous function which is then called repeatedly
#' as needed.
#'
#' The expression is repeatedly evaluated until `Rprof()` produces
#' an output. It can _be_ a quosure injected with [rlang::inject()] but
#' it cannot _contain_ injected quosures.
#'
#' Not compatible with `prof_input`.
#' @param interval Interval for profiling samples, in seconds. Values less than
#' 0.005 (5 ms) will probably not result in accurate timings
#' @param prof_output Name of an Rprof output file or directory in which to save
Expand Down Expand Up @@ -171,12 +176,16 @@
if (remove_on_exit) {
on.exit(unlink(prof_output), add = TRUE)
}
repeat {
# Work around https://github.com/r-lib/rlang/issues/1749
eval(substitute(delayedAssign("expr", expr_q, eval.env = env)))

# We call the quoted expression directly inside a function to make it
# easy to detect in both raw and simplified stack traces. The simplified
# case is particularly tricky because evaluating a promise fails to create
# a call on the trailing edges of the tree returned by simplification
`__profvis_execute__` <- new_function(list(), expr_q, env)

Check warning on line 184 in R/profvis.R

View check run for this annotation

Codecov / codecov/patch

R/profvis.R#L184

Added line #L184 was not covered by tests

repeat {

Check warning on line 186 in R/profvis.R

View check run for this annotation

Codecov / codecov/patch

R/profvis.R#L186

Added line #L186 was not covered by tests
inject(Rprof(prof_output, !!!rprof_args))
cnd <- with_profvis_handlers(expr)
cnd <- with_profvis_handlers(`__profvis_execute__`())

Check warning on line 188 in R/profvis.R

View check run for this annotation

Codecov / codecov/patch

R/profvis.R#L188

Added line #L188 was not covered by tests
Rprof(NULL)

lines <- readLines(prof_output)
Expand All @@ -188,12 +197,7 @@
}
}

# Must be in the same handler context as `expr` above to get the
# full stack suffix
with_profvis_handlers({
suffix <- rprof_current_suffix(env, simplify)
lines <- gsub(suffix, "", lines)
})
lines <- gsub('"__profvis_execute__".*$', "", lines)

Check warning on line 200 in R/profvis.R

View check run for this annotation

Codecov / codecov/patch

R/profvis.R#L200

Added line #L200 was not covered by tests
} else {
# If we got here, we were provided a prof_input file instead of expr
expr_source <- NULL
Expand Down Expand Up @@ -308,3 +312,8 @@
if (!quoted) { expr <- substitute(expr) } # force quoted
shinyRenderWidget(expr, profvisOutput, env, quoted = TRUE)
}


has_event <- function() {
getRversion() >= "4.4.0"

Check warning on line 318 in R/profvis.R

View check run for this annotation

Codecov / codecov/patch

R/profvis.R#L318

Added line #L318 was not covered by tests
}
135 changes: 0 additions & 135 deletions R/rprof.R

This file was deleted.

9 changes: 7 additions & 2 deletions man/profvis.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

42 changes: 0 additions & 42 deletions tests/testthat/_snaps/rprof.md

This file was deleted.

22 changes: 6 additions & 16 deletions tests/testthat/helper-profvis.R
Original file line number Diff line number Diff line change
@@ -1,23 +1,13 @@

TEST_PAUSE_TIME <- 0.050

cat_rprof <- function(expr, ..., rerun = "pause") {
out <- inject(rprof_lines({{ expr }}, ..., rerun = rerun))
out <- modal_value0(out)

if (is_null(out)) {
abort("Unexpected profile")
}

cat(paste0(out, "\n"))
}

repro_profvis <- function(expr, ..., rerun = "pause", interval = 0.010) {
inject(profvis({{ expr }}, ..., rerun = rerun, interval = interval))
call_stacks <- function(x) {
prof <- x$x$message$prof
stacks <- split(prof$label, prof$time)
vapply(stacks, paste, "", collapse = " ")
}

zap_trailing_space <- function(lines) {
gsub(" $", "", lines)
modal_call <- function(x) {
modal_value0(call_stacks(x))
}

profile_calls <- function(x) {
Expand Down
38 changes: 32 additions & 6 deletions tests/testthat/test-profvis.R
Original file line number Diff line number Diff line change
@@ -1,16 +1,39 @@
test_that("Irrelevant stack is trimmed from profiles (#123)", {
test_that("irrelevant stack trimmed from function calls (#123)", {
skip_on_cran()
skip_on_covr()

f <- function() pause(TEST_PAUSE_TIME)
g <- function() f()

out <- repro_profvis(f(), simplify = FALSE)
expect_equal(profile_mode(out), "pause f")
out <- profvis(g(), simplify = TRUE, rerun = "pause")
expect_equal(profile_mode(out), "pause f g")

out <- profvis(g(), simplify = FALSE, rerun = "pause")
expect_equal(profile_mode(out), "pause f g")
})

test_that("irrelevant stack trimmed from inlined code (#130)", {
skip_on_cran()
skip_on_covr()

out <- profvis(for (i in 1:1e4) rnorm(100), simplify = TRUE, rerun = "rnorm")
expect_equal(profile_mode(out), "rnorm")

out <- profvis(for (i in 1:1e4) rnorm(100), simplify = FALSE, rerun = "rnorm")
expect_equal(profile_mode(out), "rnorm")
})

out <- profvis(f(), simplify = TRUE, rerun = "pause", interval = 0.005)
test_that("strips stack above profvis", {
skip_on_cran()
skip_on_covr()

f <- function() pause(TEST_PAUSE_TIME)
profvis_wrap <- function(...) profvis(...)

out <- profvis_wrap(f(), simplify = TRUE, rerun = "pause")
expect_equal(profile_mode(out), "pause f")

out <- repro_profvis(f(), simplify = TRUE)
out <- profvis_wrap(f(), simplify = FALSE, rerun = "pause")
expect_equal(profile_mode(out), "pause f")
})

Expand All @@ -21,7 +44,7 @@ test_that("defaults to elapsed timing", {

f <- function() Sys.sleep(TEST_PAUSE_TIME)

out <- repro_profvis(f(), rerun = "Sys.sleep")
out <- profvis(f(), rerun = "Sys.sleep")
expect_equal(profile_mode(out), "Sys.sleep f")
})

Expand All @@ -30,6 +53,9 @@ test_that("expr and prof_input are mutually exclusive", {
})

test_that("can capture profile of code with error", {
skip_on_cran()
skip_on_covr()

f <- function() {
pause(TEST_PAUSE_TIME)
stop("error")
Expand Down
Loading
Loading