From b087c19b522a3530c736f5588b9ed5080ad35364 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 8 May 2019 14:08:22 -0500 Subject: [PATCH 01/13] Use fastmap as backing store for Map class --- DESCRIPTION | 5 +- R/globals.R | 6 ++ R/map.R | 42 ++++---- R/server-input-handlers.R | 221 +++++++++++++++++++------------------- R/server.R | 6 +- R/shiny.R | 3 +- R/timer.R | 3 +- 7 files changed, 149 insertions(+), 137 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 22f439f0a..8d6e8a6d7 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -77,7 +77,8 @@ Imports: promises (>= 1.0.1), tools, crayon, - rlang + rlang, + fastmap Suggests: datasets, Cairo (>= 1.5-5), @@ -88,6 +89,8 @@ Suggests: ggplot2, reactlog (>= 1.0.0), magrittr +Remotes: + wch/fastmap URL: http://shiny.rstudio.com BugReports: https://github.com/rstudio/shiny/issues Collate: diff --git a/R/globals.R b/R/globals.R index 9edfc8b60..444796571 100644 --- a/R/globals.R +++ b/R/globals.R @@ -6,6 +6,12 @@ # package itself, making our PRNG completely deterministic. This line resets # the private seed during load. withPrivateSeed(set.seed(NULL)) + + appsByToken <<- Map$new() + appsNeedingFlush <<- Map$new() + timerCallbacks <<- TimerCallbacks$new() + initializeInputHandlers() + .globals$onStopCallbacks <<- Callbacks$new() } .onAttach <- function(libname, pkgname) { diff --git a/R/map.R b/R/map.R index 91790b8b0..5a555c75a 100644 --- a/R/map.R +++ b/R/map.R @@ -9,63 +9,57 @@ # Remove of unknown key does nothing # Setting a key twice always results in last-one-wins # /TESTS + +# Note that Map objects can't be saved in one R session and restored in +# another, because they are based on fastmap, which uses an external pointer, +# and external pointers can't be saved and restored in another session. Map <- R6Class( 'Map', portable = FALSE, public = list( initialize = function() { - private$env <- new.env(parent=emptyenv()) + private$map <<- fastmap::fastmap() }, get = function(key) { - env[[key]] + map$get(key) }, set = function(key, value) { - env[[key]] <- value + map$set(key, value) value }, mget = function(keys) { - base::mget(keys, env) + map$mget(keys) }, mset = function(...) { - args <- list(...) - if (length(args) == 0) - return() - - arg_names <- names(args) - if (is.null(arg_names) || any(!nzchar(arg_names))) - stop("All elements must be named") - - list2env(args, envir = env) + map$mset(...) }, remove = function(key) { - if (!self$containsKey(key)) + if (!map$exists(key)) return(NULL) - result <- env[[key]] - rm(list=key, envir=env, inherits=FALSE) + result <- map$get(key) + map$remove(key) result }, containsKey = function(key) { - exists(key, envir=env, inherits=FALSE) + map$exists(key) }, keys = function() { - # Sadly, this is much faster than ls(), because it doesn't sort the keys. - names(as.list(env, all.names=TRUE)) + map$keys() }, values = function() { - as.list(env, all.names=TRUE) + map$as_list() }, clear = function() { - private$env <- new.env(parent=emptyenv()) - invisible(NULL) + map$reset() }, size = function() { - length(env) + map$size() } ), private = list( - env = 'environment' + map = NULL ) ) diff --git a/R/server-input-handlers.R b/R/server-input-handlers.R index e1fab4431..3ed22afd5 100644 --- a/R/server-input-handlers.R +++ b/R/server-input-handlers.R @@ -1,5 +1,6 @@ -# Create a map for input handlers and register the defaults. -inputHandlers <- Map$new() +# Create a map for input handlers and register the defaults. The Map object is +# initialized in initializeInputHandlers, which is called from .onLoad(). +inputHandlers <- NULL #' Register an Input Handler #' @@ -128,114 +129,118 @@ applyInputHandlers <- function(inputs, shinysession = getDefaultReactiveDomain() } -# Takes a list-of-lists and returns a matrix. The lists -# must all be the same length. NULL is replaced by NA. -registerInputHandler("shiny.matrix", function(data, ...) { - if (length(data) == 0) - return(matrix(nrow=0, ncol=0)) +initializeInputHandlers <- function() { + inputHandlers <<- Map$new() - m <- matrix(unlist(lapply(data, function(x) { - sapply(x, function(y) { - ifelse(is.null(y), NA, y) - }) - })), nrow = length(data[[1]]), ncol = length(data)) - return(m) -}) + # Takes a list-of-lists and returns a matrix. The lists + # must all be the same length. NULL is replaced by NA. + registerInputHandler("shiny.matrix", function(data, ...) { + if (length(data) == 0) + return(matrix(nrow=0, ncol=0)) - -registerInputHandler("shiny.number", function(val, ...){ - ifelse(is.null(val), NA, val) -}) - -registerInputHandler("shiny.password", function(val, shinysession, name) { - # Mark passwords as not serializable - setSerializer(name, serializerUnserializable) - val -}) - -registerInputHandler("shiny.date", function(val, ...){ - # First replace NULLs with NA, then convert to Date vector - datelist <- ifelse(lapply(val, is.null), NA, val) - - res <- NULL - tryCatch({ - res <- as.Date(unlist(datelist)) - }, - error = function(e) { - # It's possible for client to send a string like "99999-01-01", which - # as.Date can't handle. - warning(e$message) - res <<- as.Date(rep(NA, length(datelist))) - } - ) - - res -}) - -registerInputHandler("shiny.datetime", function(val, ...){ - # First replace NULLs with NA, then convert to POSIXct vector - times <- lapply(val, function(x) { - if (is.null(x)) NA - else x + m <- matrix(unlist(lapply(data, function(x) { + sapply(x, function(y) { + ifelse(is.null(y), NA, y) + }) + })), nrow = length(data[[1]]), ncol = length(data)) + return(m) }) - as.POSIXct(unlist(times), origin = "1970-01-01", tz = "UTC") -}) - -registerInputHandler("shiny.action", function(val, shinysession, name) { - # mark up the action button value with a special class so we can recognize it later - class(val) <- c(class(val), "shinyActionButtonValue") - val -}) - -registerInputHandler("shiny.file", function(val, shinysession, name) { - # This function is only used when restoring a Shiny fileInput. When a file is - # uploaded the usual way, it takes a different code path and won't hit this - # function. - if (is.null(val)) - return(NULL) - - # The data will be a named list of lists; convert to a data frame. - val <- as.data.frame(lapply(val, unlist), stringsAsFactors = FALSE) - - # `val$datapath` should be a filename without a path, for security reasons. - if (basename(val$datapath) != val$datapath) { - stop("Invalid '/' found in file input path.") - } - - # Prepend the persistent dir - oldfile <- file.path(getCurrentRestoreContext()$dir, val$datapath) - - # Copy the original file to a new temp dir, so that a restored session can't - # modify the original. - newdir <- file.path(tempdir(), createUniqueId(12)) - dir.create(newdir) - val$datapath <- file.path(newdir, val$datapath) - file.copy(oldfile, val$datapath) - - # Need to mark this input value with the correct serializer. When a file is - # uploaded the usual way (instead of being restored), this occurs in - # session$`@uploadEnd`. - setSerializer(name, serializerFileInput) - - snapshotPreprocessInput(name, snapshotPreprocessorFileInput) - - val -}) -# to be used with !!!answer -registerInputHandler("shiny.symbolList", function(val, ...) { - if (is.null(val)) { - list() - } else { - lapply(val, as.symbol) - } -}) -# to be used with !!answer -registerInputHandler("shiny.symbol", function(val, ...) { - if (is.null(val) || identical(val, "")) { - NULL - } else { - as.symbol(val) - } -}) + registerInputHandler("shiny.number", function(val, ...){ + ifelse(is.null(val), NA, val) + }) + + registerInputHandler("shiny.password", function(val, shinysession, name) { + # Mark passwords as not serializable + setSerializer(name, serializerUnserializable) + val + }) + + registerInputHandler("shiny.date", function(val, ...){ + # First replace NULLs with NA, then convert to Date vector + datelist <- ifelse(lapply(val, is.null), NA, val) + + res <- NULL + tryCatch({ + res <- as.Date(unlist(datelist)) + }, + error = function(e) { + # It's possible for client to send a string like "99999-01-01", which + # as.Date can't handle. + warning(e$message) + res <<- as.Date(rep(NA, length(datelist))) + } + ) + + res + }) + + registerInputHandler("shiny.datetime", function(val, ...){ + # First replace NULLs with NA, then convert to POSIXct vector + times <- lapply(val, function(x) { + if (is.null(x)) NA + else x + }) + as.POSIXct(unlist(times), origin = "1970-01-01", tz = "UTC") + }) + + registerInputHandler("shiny.action", function(val, shinysession, name) { + # mark up the action button value with a special class so we can recognize it later + class(val) <- c(class(val), "shinyActionButtonValue") + val + }) + + registerInputHandler("shiny.file", function(val, shinysession, name) { + # This function is only used when restoring a Shiny fileInput. When a file is + # uploaded the usual way, it takes a different code path and won't hit this + # function. + if (is.null(val)) + return(NULL) + + # The data will be a named list of lists; convert to a data frame. + val <- as.data.frame(lapply(val, unlist), stringsAsFactors = FALSE) + + # `val$datapath` should be a filename without a path, for security reasons. + if (basename(val$datapath) != val$datapath) { + stop("Invalid '/' found in file input path.") + } + + # Prepend the persistent dir + oldfile <- file.path(getCurrentRestoreContext()$dir, val$datapath) + + # Copy the original file to a new temp dir, so that a restored session can't + # modify the original. + newdir <- file.path(tempdir(), createUniqueId(12)) + dir.create(newdir) + val$datapath <- file.path(newdir, val$datapath) + file.copy(oldfile, val$datapath) + + # Need to mark this input value with the correct serializer. When a file is + # uploaded the usual way (instead of being restored), this occurs in + # session$`@uploadEnd`. + setSerializer(name, serializerFileInput) + + snapshotPreprocessInput(name, snapshotPreprocessorFileInput) + + val + }) + + + # to be used with !!!answer + registerInputHandler("shiny.symbolList", function(val, ...) { + if (is.null(val)) { + list() + } else { + lapply(val, as.symbol) + } + }) + # to be used with !!answer + registerInputHandler("shiny.symbol", function(val, ...) { + if (is.null(val) || identical(val, "")) { + NULL + } else { + as.symbol(val) + } + }) +} diff --git a/R/server.R b/R/server.R index c7ef319d9..a82bcb7b6 100644 --- a/R/server.R +++ b/R/server.R @@ -1,7 +1,9 @@ #' @include server-input-handlers.R -appsByToken <- Map$new() -appsNeedingFlush <- Map$new() +# These Map objects are initialized in .onLoad() because Maps based on fastmap +# can't be saved in one R session and loaded in another. +appsByToken <- NULL +appsNeedingFlush <- NULL # Provide a character representation of the WS that can be used # as a key in a Map. diff --git a/R/shiny.R b/R/shiny.R index ba94c1b8c..de5454e01 100644 --- a/R/shiny.R +++ b/R/shiny.R @@ -2220,7 +2220,8 @@ flushPendingSessions <- function() { }) } -.globals$onStopCallbacks <- Callbacks$new() +# Initialized in .onLoad +.globals$onStopCallbacks <- NULL #' Run code after an application or session ends #' diff --git a/R/timer.R b/R/timer.R index e3d1f4b9e..61395f11f 100644 --- a/R/timer.R +++ b/R/timer.R @@ -86,7 +86,8 @@ TimerCallbacks <- R6Class( ) ) -timerCallbacks <- TimerCallbacks$new() +# Initialized in onLoad() because TimerCallbacks uses Map. +timerCallbacks <- NULL scheduleTask <- function(millis, callback) { cancelled <- FALSE From 2a6f21870000a3ad2b01d6dbd72ed9cabd445aed Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 8 May 2019 15:41:03 -0500 Subject: [PATCH 02/13] Convert ReactiveValues$.dependents to use Map --- R/reactives.R | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/R/reactives.R b/R/reactives.R index c1e2e1872..6e8e60dd6 100644 --- a/R/reactives.R +++ b/R/reactives.R @@ -294,7 +294,7 @@ ReactiveValues <- R6Class( .label = character(0), .values = 'environment', .metadata = 'environment', - .dependents = 'environment', + .dependents = 'Map', # Dependents for the list of all names, including hidden .namesDeps = 'Dependents', # Dependents for all values, including hidden @@ -314,7 +314,7 @@ ReactiveValues <- R6Class( .label <<- label .values <<- new.env(parent=emptyenv()) .metadata <<- new.env(parent=emptyenv()) - .dependents <<- new.env(parent=emptyenv()) + .dependents <<- Map$new() .hasRetrieved <<- list(names = FALSE, asListAll = FALSE, asList = FALSE, keys = list()) .namesDeps <<- Dependents$new(reactId = rLog$namesIdStr(.reactId)) .allValuesDeps <<- Dependents$new(reactId = rLog$asListAllIdStr(.reactId)) @@ -333,7 +333,7 @@ ReactiveValues <- R6Class( # that we know to invalidate them when this value changes. ctx <- getCurrentContext() dep.key <- paste(key, ':', ctx$id, sep='') - if (!exists(dep.key, envir=.dependents, inherits=FALSE)) { + if (!.dependents$containsKey(dep.key)) { reactKeyId <- rLog$keyIdStr(.reactId, key) if (!isTRUE(.hasRetrieved$keys[[key]])) { @@ -341,10 +341,10 @@ ReactiveValues <- R6Class( .hasRetrieved$keys[[key]] <<- TRUE } rLog$dependsOnKey(ctx$.reactId, .reactId, key, ctx$id, ctx$.domain) - .dependents[[dep.key]] <- ctx + .dependents$set(dep.key, ctx) ctx$onInvalidate(function() { rLog$dependsOnKeyRemove(ctx$.reactId, .reactId, key, ctx$id, ctx$.domain) - rm(list=dep.key, envir=.dependents, inherits=FALSE) + .dependents$remove(dep.key) }) } @@ -425,13 +425,10 @@ ReactiveValues <- R6Class( } } - dep.keys <- objects( - envir=.dependents, - pattern=paste('^\\Q', key, ':', '\\E', '\\d+$', sep=''), - all.names=TRUE - ) + dep.keys <- .dependents$keys() + dep.keys <- dep.keys[grepl(paste('^\\Q', key, ':', '\\E', '\\d+$', sep=''), dep.keys)] lapply( - mget(dep.keys, envir=.dependents), + .dependents$mget(dep.keys), function(ctx) { ctx$invalidate() NULL From 555ede03ed1fc86117c5a9258c35bc2f2e5712c7 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 8 May 2019 16:37:02 -0500 Subject: [PATCH 03/13] Convert ReactiveValues$.values to use Map --- DESCRIPTION | 2 +- R/reactives.R | 32 +++++++++++++++++--------------- R/shiny.R | 5 ++--- tests/testthat/test-reactivity.r | 8 ++++---- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 8d6e8a6d7..5ee64b350 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -82,7 +82,7 @@ Imports: Suggests: datasets, Cairo (>= 1.5-5), - testthat, + testthat (>= 2.2.1), knitr (>= 1.6), markdown, rmarkdown, diff --git a/R/reactives.R b/R/reactives.R index 6e8e60dd6..83bafd951 100644 --- a/R/reactives.R +++ b/R/reactives.R @@ -292,7 +292,7 @@ ReactiveValues <- R6Class( # For debug purposes .reactId = character(0), .label = character(0), - .values = 'environment', + .values = 'Map', .metadata = 'environment', .dependents = 'Map', # Dependents for the list of all names, including hidden @@ -312,7 +312,7 @@ ReactiveValues <- R6Class( ) { .reactId <<- nextGlobalReactId() .label <<- label - .values <<- new.env(parent=emptyenv()) + .values <<- Map$new() .metadata <<- new.env(parent=emptyenv()) .dependents <<- Map$new() .hasRetrieved <<- list(names = FALSE, asListAll = FALSE, asList = FALSE, keys = list()) @@ -324,10 +324,7 @@ ReactiveValues <- R6Class( get = function(key) { # get value right away to use for logging - if (!exists(key, envir=.values, inherits=FALSE)) - keyValue <- NULL - else - keyValue <- .values[[key]] + keyValue <- .values$get(key) # Register the "downstream" reactive which is accessing this value, so # that we know to invalidate them when this value changes. @@ -384,16 +381,16 @@ ReactiveValues <- R6Class( domain <- getDefaultReactiveDomain() hidden <- substr(key, 1, 1) == "." - key_exists <- exists(key, envir=.values, inherits=FALSE) + key_exists <- .values$containsKey(key) if (key_exists) { - if (.dedupe && identical(.values[[key]], value)) { + if (.dedupe && identical(.values$get(key), value)) { return(invisible()) } } # set the value for better logging - .values[[key]] <- value + .values$set(key, value) # key has been depended upon if (isTRUE(.hasRetrieved$keys[[key]])) { @@ -408,19 +405,21 @@ ReactiveValues <- R6Class( # only invalidate if there are deps if (!key_exists && isTRUE(.hasRetrieved$names)) { - rLog$valueChangeNames(.reactId, ls(.values, all.names = TRUE), domain) + rLog$valueChangeNames(.reactId, .values$keys(), domain) .namesDeps$invalidate() } if (hidden) { if (isTRUE(.hasRetrieved$asListAll)) { - rLog$valueChangeAsListAll(.reactId, as.list(.values, all.names = TRUE), domain) + rLog$valueChangeAsListAll(.reactId, .values$values(), domain) .allValuesDeps$invalidate() } } else { if (isTRUE(.hasRetrieved$asList)) { + react_vals <- .values$values() + react_vals <- react_vals[!grepl("^\\.", base::names(react_vals))] # leave as is. both object would be registered to the listening object - rLog$valueChangeAsList(.reactId, as.list(.values, all.names = FALSE), domain) + rLog$valueChangeAsList(.reactId, react_vals, domain) .valuesDeps$invalidate() } } @@ -445,7 +444,7 @@ ReactiveValues <- R6Class( }, names = function() { - nameValues <- ls(.values, all.names=TRUE) + nameValues <- .values$keys() if (!isTRUE(.hasRetrieved$names)) { domain <- getDefaultReactiveDomain() rLog$defineNames(.reactId, nameValues, .label, domain) @@ -493,7 +492,10 @@ ReactiveValues <- R6Class( }, toList = function(all.names=FALSE) { - listValue <- as.list(.values, all.names=all.names) + listValue <- .values$values() + if (!all.names) { + listValue <- listValue[!grepl("^\\.", base::names(listValue))] + } if (all.names) { if (!isTRUE(.hasRetrieved$asListAll)) { domain <- getDefaultReactiveDomain() @@ -506,7 +508,7 @@ ReactiveValues <- R6Class( if (!isTRUE(.hasRetrieved$asList)) { domain <- getDefaultReactiveDomain() # making sure the value being recorded is with `all.names = FALSE` - rLog$defineAsList(.reactId, as.list(.values, all.names=FALSE), .label, domain) + rLog$defineAsList(.reactId, listValue[!grepl("^\\.", base::names(listValue))], .label, domain) .hasRetrieved$asList <<- TRUE } .valuesDeps$register() diff --git a/R/shiny.R b/R/shiny.R index de5454e01..624ca404e 100644 --- a/R/shiny.R +++ b/R/shiny.R @@ -513,8 +513,7 @@ ShinySession <- R6Class( # in the web page; in these cases, there's no output_foo_hidden flag, # and hidden should be TRUE. In other words, NULL and TRUE should map to # TRUE, FALSE should map to FALSE. - hidden <- private$.clientData$.values[[paste("output_", name, "_hidden", - sep="")]] + hidden <- private$.clientData$.values$get(paste0("output_", name, "_hidden")) if (is.null(hidden)) hidden <- TRUE return(hidden && private$getOutputOption(name, 'suspendWhenHidden', TRUE)) @@ -1917,7 +1916,7 @@ ShinySession <- R6Class( fileData <- readBin(file, 'raw', n=bytes) - if (isTRUE(private$.clientData$.values$allowDataUriScheme)) { + if (isTRUE(private$.clientData$.values$get("allowDataUriScheme"))) { b64 <- rawToBase64(fileData) return(paste('data:', contentType, ';base64,', b64, sep='')) } else { diff --git a/tests/testthat/test-reactivity.r b/tests/testthat/test-reactivity.r index ff24b144a..f126da324 100644 --- a/tests/testthat/test-reactivity.r +++ b/tests/testthat/test-reactivity.r @@ -521,8 +521,8 @@ test_that("names() and reactiveValuesToList()", { # Assigning names fails expect_error(isolate(names(v) <- c('x', 'y'))) - expect_equal(isolate(reactiveValuesToList(values)), list(A=1)) - expect_equal(isolate(reactiveValuesToList(values, all.names=TRUE)), list(A=1, .B=2)) + expect_mapequal(isolate(reactiveValuesToList(values)), list(A=1)) + expect_mapequal(isolate(reactiveValuesToList(values, all.names=TRUE)), list(A=1, .B=2)) flushReact() @@ -1137,10 +1137,10 @@ test_that("reactive domain works across async handlers", { ~{hasReactiveDomain <<- identical(getDefaultReactiveDomain(), obj)} ) }) - + while (is.null(hasReactiveDomain) && !later::loop_empty()) { later::run_now() } - + testthat::expect_true(hasReactiveDomain) }) From 32c2bff6eb76c221f8471c73313ced99591e18fd Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 8 May 2019 19:54:32 -0500 Subject: [PATCH 04/13] Convert ReactiveValues$.metadata to use Map --- R/reactives.R | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/R/reactives.R b/R/reactives.R index 83bafd951..667f181e4 100644 --- a/R/reactives.R +++ b/R/reactives.R @@ -293,7 +293,7 @@ ReactiveValues <- R6Class( .reactId = character(0), .label = character(0), .values = 'Map', - .metadata = 'environment', + .metadata = 'Map', .dependents = 'Map', # Dependents for the list of all names, including hidden .namesDeps = 'Dependents', @@ -313,7 +313,7 @@ ReactiveValues <- R6Class( .reactId <<- nextGlobalReactId() .label <<- label .values <<- Map$new() - .metadata <<- new.env(parent=emptyenv()) + .metadata <<- Map$new() .dependents <<- Map$new() .hasRetrieved <<- list(names = FALSE, asListAll = FALSE, asList = FALSE, keys = list()) .namesDeps <<- Dependents$new(reactId = rLog$namesIdStr(.reactId)) @@ -458,7 +458,7 @@ ReactiveValues <- R6Class( getMeta = function(key, metaKey) { # Make sure to use named (not numeric) indexing into list. metaKey <- as.character(metaKey) - .metadata[[key]][[metaKey]] + .metadata$get("key")[[metaKey]] }, # Set a metadata value. Does not trigger reactivity. @@ -466,11 +466,13 @@ ReactiveValues <- R6Class( # Make sure to use named (not numeric) indexing into list. metaKey <- as.character(metaKey) - if (!exists(key, envir = .metadata, inherits = FALSE)) { - .metadata[[key]] <<- list() + if (!.metadata$containsKey(key)) { + .metadata$set(key, list()) } - .metadata[[key]][[metaKey]] <<- value + m <- .metadata$get(key) + m[[metaKey]] <- value + .metadata$set(key, m) }, # Mark a value as frozen If accessed while frozen, a shiny.silent.error will From d7718991a6b8610d2c19096ed997ecd09156f53d Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 8 May 2019 21:03:50 -0500 Subject: [PATCH 05/13] Import fastmap::fastmap --- NAMESPACE | 1 + R/map.R | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NAMESPACE b/NAMESPACE index 174f2dce4..ce7de6ef8 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -301,5 +301,6 @@ import(httpuv) import(methods) import(mime) import(xtable) +importFrom(fastmap,fastmap) importFrom(grDevices,dev.cur) importFrom(grDevices,dev.set) diff --git a/R/map.R b/R/map.R index 5a555c75a..33ab49ff6 100644 --- a/R/map.R +++ b/R/map.R @@ -13,12 +13,13 @@ # Note that Map objects can't be saved in one R session and restored in # another, because they are based on fastmap, which uses an external pointer, # and external pointers can't be saved and restored in another session. +#' @importFrom fastmap fastmap Map <- R6Class( 'Map', portable = FALSE, public = list( initialize = function() { - private$map <<- fastmap::fastmap() + private$map <<- fastmap() }, get = function(key) { map$get(key) From 38d2809131342edb9bfc9f5b46a57087bfbea0a2 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Wed, 8 May 2019 21:16:32 -0500 Subject: [PATCH 06/13] Convert MemoryCache to use fastmap --- R/cache-memory.R | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/R/cache-memory.R b/R/cache-memory.R index c7f1b5036..6fc843dd8 100644 --- a/R/cache-memory.R +++ b/R/cache-memory.R @@ -179,7 +179,7 @@ MemoryCache <- R6Class("MemoryCache", if (!is.numeric(max_size)) stop("max_size must be a number. Use `Inf` for no limit.") if (!is.numeric(max_age)) stop("max_age must be a number. Use `Inf` for no limit.") if (!is.numeric(max_n)) stop("max_n must be a number. Use `Inf` for no limit.") - private$cache <- new.env(parent = emptyenv()) + private$cache <- fastmap() private$max_size <- max_size private$max_age <- max_age private$max_n <- max_n @@ -208,7 +208,7 @@ MemoryCache <- R6Class("MemoryCache", } private$log(paste0('get: key "', key, '" found')) - value <- private$cache[[key]]$value + value <- private$cache$get(key)$value value }, @@ -226,37 +226,36 @@ MemoryCache <- R6Class("MemoryCache", size <- NULL } - private$cache[[key]] <- list( + private$cache$set(key, list( key = key, value = value, size = size, mtime = time, atime = time - ) + )) self$prune() invisible(self) }, exists = function(key) { validate_key(key) - # Faster than `exists(key, envir = private$cache, inherits = FALSE) - !is.null(private$cache[[key]]) + private$cache$exists(key) }, keys = function() { - ls(private$cache, sorted = FALSE) # Faster with sorted=FALSE + private$cache$keys() }, remove = function(key) { private$log(paste0('remove: key "', key, '"')) validate_key(key) - rm(list = key, envir = private$cache) + private$cache$remove(key) invisible(self) }, reset = function() { private$log(paste0('reset')) - rm(list = self$keys(), envir = private$cache) + private$cache$reset() invisible(self) }, @@ -271,7 +270,7 @@ MemoryCache <- R6Class("MemoryCache", rm_idx <- timediff > private$max_age if (any(rm_idx)) { private$log(paste0("prune max_age: Removing ", paste(info$key[rm_idx], collapse = ", "))) - rm(list = info$key[rm_idx], envir = private$cache) + private$cache$remove(info$key[rm_idx]) info <- info[!rm_idx, ] } } @@ -298,7 +297,7 @@ MemoryCache <- R6Class("MemoryCache", ensure_info_is_sorted() rm_idx <- seq_len(nrow(info)) > private$max_n private$log(paste0("prune max_n: Removing ", paste(info$key[rm_idx], collapse = ", "))) - rm(list = info$key[rm_idx], envir = private$cache) + private$cache$remove(info$key[rm_idx]) info <- info[!rm_idx, ] } @@ -308,7 +307,7 @@ MemoryCache <- R6Class("MemoryCache", cum_size <- cumsum(info$size) rm_idx <- cum_size > private$max_size private$log(paste0("prune max_size: Removing ", paste(info$key[rm_idx], collapse = ", "))) - rm(list = info$key[rm_idx], envir = private$cache) + private$cache$remove(info$key[rm_idx]) info <- info[!rm_idx, ] } @@ -335,23 +334,23 @@ MemoryCache <- R6Class("MemoryCache", maybe_prune_single = function(key) { if (!is.finite(private$max_age)) return() - obj <- private$cache[[key]] + obj <- private$cache$get(key) if (is.null(obj)) return() timediff <- as.numeric(Sys.time()) - obj$mtime if (timediff > private$max_age) { private$log(paste0("pruning single object exceeding max_age: Removing ", key)) - rm(list = key, envir = private$cache) + private$cache$remove(key) } }, object_info = function() { - keys <- ls(private$cache, sorted = FALSE) + keys <- private$cache$keys() data.frame( key = keys, - size = vapply(keys, function(key) private$cache[[key]]$size, 0), - mtime = vapply(keys, function(key) private$cache[[key]]$mtime, 0), - atime = vapply(keys, function(key) private$cache[[key]]$atime, 0), + size = vapply(keys, function(key) private$cache$get(key)$size, 0), + mtime = vapply(keys, function(key) private$cache$get(key)$mtime, 0), + atime = vapply(keys, function(key) private$cache$get(key)$atime, 0), stringsAsFactors = FALSE ) }, From 0747b2a72a78958706b160bc2c49f28589159e23 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Mon, 13 May 2019 15:31:19 -0500 Subject: [PATCH 07/13] fastmap: exists() was renamed to has() --- R/cache-memory.R | 2 +- R/map.R | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/R/cache-memory.R b/R/cache-memory.R index 6fc843dd8..0d6c22426 100644 --- a/R/cache-memory.R +++ b/R/cache-memory.R @@ -239,7 +239,7 @@ MemoryCache <- R6Class("MemoryCache", exists = function(key) { validate_key(key) - private$cache$exists(key) + private$cache$has(key) }, keys = function() { diff --git a/R/map.R b/R/map.R index 33ab49ff6..c435b70ff 100644 --- a/R/map.R +++ b/R/map.R @@ -35,7 +35,7 @@ Map <- R6Class( map$mset(...) }, remove = function(key) { - if (!map$exists(key)) + if (!map$has(key)) return(NULL) result <- map$get(key) @@ -43,7 +43,7 @@ Map <- R6Class( result }, containsKey = function(key) { - map$exists(key) + map$has(key) }, keys = function() { map$keys() From 1ef2074a10872ea2692b1b13928305eeaf7ab330 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Tue, 14 May 2019 10:36:37 -0500 Subject: [PATCH 08/13] Fastmap objects can now be saved and loaded --- R/globals.R | 6 -- R/server-input-handlers.R | 207 +++++++++++++++++++------------------- R/server.R | 6 +- R/shiny.R | 3 +- R/timer.R | 3 +- 5 files changed, 105 insertions(+), 120 deletions(-) diff --git a/R/globals.R b/R/globals.R index 444796571..9edfc8b60 100644 --- a/R/globals.R +++ b/R/globals.R @@ -6,12 +6,6 @@ # package itself, making our PRNG completely deterministic. This line resets # the private seed during load. withPrivateSeed(set.seed(NULL)) - - appsByToken <<- Map$new() - appsNeedingFlush <<- Map$new() - timerCallbacks <<- TimerCallbacks$new() - initializeInputHandlers() - .globals$onStopCallbacks <<- Callbacks$new() } .onAttach <- function(libname, pkgname) { diff --git a/R/server-input-handlers.R b/R/server-input-handlers.R index 3ed22afd5..e1fab4431 100644 --- a/R/server-input-handlers.R +++ b/R/server-input-handlers.R @@ -1,6 +1,5 @@ -# Create a map for input handlers and register the defaults. The Map object is -# initialized in initializeInputHandlers, which is called from .onLoad(). -inputHandlers <- NULL +# Create a map for input handlers and register the defaults. +inputHandlers <- Map$new() #' Register an Input Handler #' @@ -129,118 +128,114 @@ applyInputHandlers <- function(inputs, shinysession = getDefaultReactiveDomain() } -initializeInputHandlers <- function() { - inputHandlers <<- Map$new() +# Takes a list-of-lists and returns a matrix. The lists +# must all be the same length. NULL is replaced by NA. +registerInputHandler("shiny.matrix", function(data, ...) { + if (length(data) == 0) + return(matrix(nrow=0, ncol=0)) - # Takes a list-of-lists and returns a matrix. The lists - # must all be the same length. NULL is replaced by NA. - registerInputHandler("shiny.matrix", function(data, ...) { - if (length(data) == 0) - return(matrix(nrow=0, ncol=0)) - - m <- matrix(unlist(lapply(data, function(x) { - sapply(x, function(y) { - ifelse(is.null(y), NA, y) - }) - })), nrow = length(data[[1]]), ncol = length(data)) - return(m) - }) - - - registerInputHandler("shiny.number", function(val, ...){ - ifelse(is.null(val), NA, val) - }) - - registerInputHandler("shiny.password", function(val, shinysession, name) { - # Mark passwords as not serializable - setSerializer(name, serializerUnserializable) - val - }) - - registerInputHandler("shiny.date", function(val, ...){ - # First replace NULLs with NA, then convert to Date vector - datelist <- ifelse(lapply(val, is.null), NA, val) - - res <- NULL - tryCatch({ - res <- as.Date(unlist(datelist)) - }, - error = function(e) { - # It's possible for client to send a string like "99999-01-01", which - # as.Date can't handle. - warning(e$message) - res <<- as.Date(rep(NA, length(datelist))) - } - ) - - res - }) - - registerInputHandler("shiny.datetime", function(val, ...){ - # First replace NULLs with NA, then convert to POSIXct vector - times <- lapply(val, function(x) { - if (is.null(x)) NA - else x + m <- matrix(unlist(lapply(data, function(x) { + sapply(x, function(y) { + ifelse(is.null(y), NA, y) }) - as.POSIXct(unlist(times), origin = "1970-01-01", tz = "UTC") - }) + })), nrow = length(data[[1]]), ncol = length(data)) + return(m) +}) - registerInputHandler("shiny.action", function(val, shinysession, name) { - # mark up the action button value with a special class so we can recognize it later - class(val) <- c(class(val), "shinyActionButtonValue") - val - }) - registerInputHandler("shiny.file", function(val, shinysession, name) { - # This function is only used when restoring a Shiny fileInput. When a file is - # uploaded the usual way, it takes a different code path and won't hit this - # function. - if (is.null(val)) - return(NULL) +registerInputHandler("shiny.number", function(val, ...){ + ifelse(is.null(val), NA, val) +}) - # The data will be a named list of lists; convert to a data frame. - val <- as.data.frame(lapply(val, unlist), stringsAsFactors = FALSE) +registerInputHandler("shiny.password", function(val, shinysession, name) { + # Mark passwords as not serializable + setSerializer(name, serializerUnserializable) + val +}) - # `val$datapath` should be a filename without a path, for security reasons. - if (basename(val$datapath) != val$datapath) { - stop("Invalid '/' found in file input path.") +registerInputHandler("shiny.date", function(val, ...){ + # First replace NULLs with NA, then convert to Date vector + datelist <- ifelse(lapply(val, is.null), NA, val) + + res <- NULL + tryCatch({ + res <- as.Date(unlist(datelist)) + }, + error = function(e) { + # It's possible for client to send a string like "99999-01-01", which + # as.Date can't handle. + warning(e$message) + res <<- as.Date(rep(NA, length(datelist))) } + ) - # Prepend the persistent dir - oldfile <- file.path(getCurrentRestoreContext()$dir, val$datapath) + res +}) - # Copy the original file to a new temp dir, so that a restored session can't - # modify the original. - newdir <- file.path(tempdir(), createUniqueId(12)) - dir.create(newdir) - val$datapath <- file.path(newdir, val$datapath) - file.copy(oldfile, val$datapath) - - # Need to mark this input value with the correct serializer. When a file is - # uploaded the usual way (instead of being restored), this occurs in - # session$`@uploadEnd`. - setSerializer(name, serializerFileInput) - - snapshotPreprocessInput(name, snapshotPreprocessorFileInput) - - val +registerInputHandler("shiny.datetime", function(val, ...){ + # First replace NULLs with NA, then convert to POSIXct vector + times <- lapply(val, function(x) { + if (is.null(x)) NA + else x }) + as.POSIXct(unlist(times), origin = "1970-01-01", tz = "UTC") +}) + +registerInputHandler("shiny.action", function(val, shinysession, name) { + # mark up the action button value with a special class so we can recognize it later + class(val) <- c(class(val), "shinyActionButtonValue") + val +}) + +registerInputHandler("shiny.file", function(val, shinysession, name) { + # This function is only used when restoring a Shiny fileInput. When a file is + # uploaded the usual way, it takes a different code path and won't hit this + # function. + if (is.null(val)) + return(NULL) + + # The data will be a named list of lists; convert to a data frame. + val <- as.data.frame(lapply(val, unlist), stringsAsFactors = FALSE) + + # `val$datapath` should be a filename without a path, for security reasons. + if (basename(val$datapath) != val$datapath) { + stop("Invalid '/' found in file input path.") + } + + # Prepend the persistent dir + oldfile <- file.path(getCurrentRestoreContext()$dir, val$datapath) + + # Copy the original file to a new temp dir, so that a restored session can't + # modify the original. + newdir <- file.path(tempdir(), createUniqueId(12)) + dir.create(newdir) + val$datapath <- file.path(newdir, val$datapath) + file.copy(oldfile, val$datapath) + + # Need to mark this input value with the correct serializer. When a file is + # uploaded the usual way (instead of being restored), this occurs in + # session$`@uploadEnd`. + setSerializer(name, serializerFileInput) + + snapshotPreprocessInput(name, snapshotPreprocessorFileInput) + + val +}) - # to be used with !!!answer - registerInputHandler("shiny.symbolList", function(val, ...) { - if (is.null(val)) { - list() - } else { - lapply(val, as.symbol) - } - }) - # to be used with !!answer - registerInputHandler("shiny.symbol", function(val, ...) { - if (is.null(val) || identical(val, "")) { - NULL - } else { - as.symbol(val) - } - }) -} +# to be used with !!!answer +registerInputHandler("shiny.symbolList", function(val, ...) { + if (is.null(val)) { + list() + } else { + lapply(val, as.symbol) + } +}) +# to be used with !!answer +registerInputHandler("shiny.symbol", function(val, ...) { + if (is.null(val) || identical(val, "")) { + NULL + } else { + as.symbol(val) + } +}) diff --git a/R/server.R b/R/server.R index a82bcb7b6..c7ef319d9 100644 --- a/R/server.R +++ b/R/server.R @@ -1,9 +1,7 @@ #' @include server-input-handlers.R -# These Map objects are initialized in .onLoad() because Maps based on fastmap -# can't be saved in one R session and loaded in another. -appsByToken <- NULL -appsNeedingFlush <- NULL +appsByToken <- Map$new() +appsNeedingFlush <- Map$new() # Provide a character representation of the WS that can be used # as a key in a Map. diff --git a/R/shiny.R b/R/shiny.R index 624ca404e..96d285012 100644 --- a/R/shiny.R +++ b/R/shiny.R @@ -2219,8 +2219,7 @@ flushPendingSessions <- function() { }) } -# Initialized in .onLoad -.globals$onStopCallbacks <- NULL +.globals$onStopCallbacks <- Callbacks$new() #' Run code after an application or session ends #' diff --git a/R/timer.R b/R/timer.R index 61395f11f..e3d1f4b9e 100644 --- a/R/timer.R +++ b/R/timer.R @@ -86,8 +86,7 @@ TimerCallbacks <- R6Class( ) ) -# Initialized in onLoad() because TimerCallbacks uses Map. -timerCallbacks <- NULL +timerCallbacks <- TimerCallbacks$new() scheduleTask <- function(millis, callback) { cancelled <- FALSE From 7bc0a0ca39a0994fb6e30c3e230135663068a332 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Tue, 14 May 2019 10:41:06 -0500 Subject: [PATCH 09/13] Fix tests that assumed names in a specific order --- tests/testthat/test-reactivity.r | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/testthat/test-reactivity.r b/tests/testthat/test-reactivity.r index f126da324..1c4190187 100644 --- a/tests/testthat/test-reactivity.r +++ b/tests/testthat/test-reactivity.r @@ -94,12 +94,12 @@ test_that("ReactiveValues", { # Initializing with NULL value values <- reactiveValues(a=NULL, b=2) # a should exist and be NULL - expect_equal(isolate(names(values)), c("a", "b")) + expect_setequal(isolate(names(values)), c("a", "b")) expect_true(is.null(isolate(values$a))) # Assigning NULL should keep object (not delete it), and set value to NULL values$b <- NULL - expect_equal(isolate(names(values)), c("a", "b")) + expect_setequal(isolate(names(values)), c("a", "b")) expect_true(is.null(isolate(values$b))) @@ -517,7 +517,7 @@ test_that("names() and reactiveValuesToList()", { }) # names() returns all names - expect_equal(sort(isolate(names(values))), sort(c(".B", "A"))) + expect_setequal(isolate(names(values)), c(".B", "A")) # Assigning names fails expect_error(isolate(names(v) <- c('x', 'y'))) From 0b6cdcc826d6d2cc97521ad36dceac5986930445 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Tue, 14 May 2019 11:46:57 -0500 Subject: [PATCH 10/13] fastmap moved to r-lib --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 5ee64b350..56e11a502 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -90,7 +90,7 @@ Suggests: reactlog (>= 1.0.0), magrittr Remotes: - wch/fastmap + r-lib/fastmap URL: http://shiny.rstudio.com BugReports: https://github.com/rstudio/shiny/issues Collate: From 0b46c63c317dd473531f1318339c9e902f8a9473 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 16 May 2019 16:43:01 -0500 Subject: [PATCH 11/13] Fix testthat version number --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 56e11a502..0377e9c17 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -82,7 +82,7 @@ Imports: Suggests: datasets, Cairo (>= 1.5-5), - testthat (>= 2.2.1), + testthat (>= 2.1.1), knitr (>= 1.6), markdown, rmarkdown, From 5cd4588ef284fa2a2e01d3e515522817de015eb3 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 30 May 2019 14:38:05 -0500 Subject: [PATCH 12/13] Use grep(value=TRUE) --- R/reactives.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/R/reactives.R b/R/reactives.R index 667f181e4..7b51aa7e5 100644 --- a/R/reactives.R +++ b/R/reactives.R @@ -425,7 +425,9 @@ ReactiveValues <- R6Class( } dep.keys <- .dependents$keys() - dep.keys <- dep.keys[grepl(paste('^\\Q', key, ':', '\\E', '\\d+$', sep=''), dep.keys)] + dep.keys <- grep( + paste('^\\Q', key, ':', '\\E', '\\d+$', sep=''), dep.keys, value = TRUE + ) lapply( .dependents$mget(dep.keys), function(ctx) { From 8f24d667d650eb50808f32742242f2e1d5360152 Mon Sep 17 00:00:00 2001 From: Winston Chang Date: Thu, 30 May 2019 15:12:15 -0500 Subject: [PATCH 13/13] Unquote key --- R/reactives.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/reactives.R b/R/reactives.R index 7b51aa7e5..ff9c11fdd 100644 --- a/R/reactives.R +++ b/R/reactives.R @@ -460,7 +460,7 @@ ReactiveValues <- R6Class( getMeta = function(key, metaKey) { # Make sure to use named (not numeric) indexing into list. metaKey <- as.character(metaKey) - .metadata$get("key")[[metaKey]] + .metadata$get(key)[[metaKey]] }, # Set a metadata value. Does not trigger reactivity.