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

R shiny memory leak with observeEvent / flashing app #1591

Closed
markL0 opened this issue Feb 19, 2017 · 5 comments
Closed

R shiny memory leak with observeEvent / flashing app #1591

markL0 opened this issue Feb 19, 2017 · 5 comments

Comments

@markL0
Copy link

markL0 commented Feb 19, 2017

Hello, I want to plot 30 random time series when the variable doPlot is changed.
To reproduce my problem in the following code doPlot is set in the app while in my real app it will be changed by a server client architecture from outside.
Using observeEvent produces a memory leak and my alternative
flashes while calculating but has no memory leak.
How to combine no memory leak and no flashing?
Can anybody help?
Thank you!!!

library("shiny")
library("parallel")
library("pryr")
ui <-  basicPage(
   plotOutput('plot1')
  ,plotOutput('plot2')
  ,plotOutput('plot3')
  ,plotOutput('plot4')
  ,plotOutput('plot5')
  ,plotOutput('plot6')
  ,plotOutput('plot7')
  ,plotOutput('plot8')
  ,plotOutput('plot9')
  ,plotOutput('plot10')
  ,plotOutput('plot11')
  ,plotOutput('plot12')
  ,plotOutput('plot13')
  ,plotOutput('plot14')
  ,plotOutput('plot15')
  ,plotOutput('plot16')
  ,plotOutput('plot17')
  ,plotOutput('plot18')
  ,plotOutput('plot19')
  ,plotOutput('plot20')
  ,plotOutput('plot21')
  ,plotOutput('plot22')
  ,plotOutput('plot23')
  ,plotOutput('plot24')
  ,plotOutput('plot25')
  ,plotOutput('plot26')
  ,plotOutput('plot27')
  ,plotOutput('plot28')
  ,plotOutput('plot29')
  ,plotOutput('plot30')
)


server <- function(input, output) {
  
  values <- reactiveValues(a=1) 
  observe({
    invalidateLater(5000)
    
    doPlot<-rnorm(1)
    
    values$a <- doPlot
    print(mem_used())
  })
  
  observeEvent(values$a,{
    mclapply(1:30,function(i){
      output[[paste0("plot",i)]] <- renderPlot({plot(rnorm(50),main=i)})
    })  
  })
}

shinyApp(ui,server)


##################################

library("shiny")
library("parallel")
library("pryr")

ui <-  basicPage(
  plotOutput('plot1')
  ,plotOutput('plot2')
  ,plotOutput('plot3')
  ,plotOutput('plot4')
  ,plotOutput('plot5')
  ,plotOutput('plot6')
  ,plotOutput('plot7')
  ,plotOutput('plot8')
  ,plotOutput('plot9')
  ,plotOutput('plot10')
  ,plotOutput('plot11')
  ,plotOutput('plot12')
  ,plotOutput('plot13')
  ,plotOutput('plot14')
  ,plotOutput('plot15')
  ,plotOutput('plot16')
  ,plotOutput('plot17')
  ,plotOutput('plot18')
  ,plotOutput('plot19')
  ,plotOutput('plot20')
  ,plotOutput('plot21')
  ,plotOutput('plot22')
  ,plotOutput('plot23')
  ,plotOutput('plot24')
  ,plotOutput('plot25')
  ,plotOutput('plot26')
  ,plotOutput('plot27')
  ,plotOutput('plot28')
  ,plotOutput('plot29')
  ,plotOutput('plot30')
)


server <- function(input, output) {
  values <- reactiveValues(a=1) 
  
  observe({
    invalidateLater(5000)
    
    doPlot<-rnorm(1)
    
    values$a <- doPlot
    print(mem_used())
  })
  
  mclapply(1:30,function(i){
     output[[paste0("plot",i)]] <<- renderPlot({values$a
                                                plot(rnorm(50),main=i)
                                            })
  })
  
}
shinyApp(ui,server)
@bborgesr
Copy link
Contributor

TL;DR: tags$style(type="text/css", ".recalculating {opacity: 1.0;}")


The second way is definitely the right way to do it. If you want to disable the grey-on-invalidation mechanism, just add this line to your basicPage function, which injects the appropriate css into your app:

tags$style(type="text/css", ".recalculating {opacity: 1.0;}")

Although, the grey-on-invalidation mechanism (flashing) is actually meaningful. The plots become grey the moment they are invalidated (which in your case means when values$a has changed), so their current display is not up to date, but they also have not finished recalculating for the new value of values$a. You can speed up recalculation (as it seems you've already done by parallelizing your code), but never completely get rid of it. So removing this mechanism actually removes information from you/the end user, as you now don't have any way to know if the plot is still up to date or if it's recalculating for the next value... I understand the may be annoying, but I just want to make sure you understand why it's happening.

A couple of other things:

  • you don't need the super-assign operator (<<-); you can stick to <- for this (unless there's something about mcapply that necessitates this, but since you use <- in the first app, that doesn't seem to be the case).

  • I don't there is an actual memory leak: in the first app, you don't take a dependency on values$a inside the renderPlot code. So, Shiny never invalidates that expression. Instead, it keeps calculating plots over and over again, never freeing up the previous ones.

This becomes extremely obvious when you run the following app that makes the same mistake, and then look at its reactive log.


Here are the instruction for doing so:

  1. Restart your R session and then enter options(shiny.reactlog=TRUE)

  2. Run the following app:

    library(shiny)
    
    ui <- fluidPage(
      plotOutput('plt'),
      actionButton("go", "Go")
    )
    
    server <- function(input, output, session) {
      
      output$plt <- renderPlot({ plot(cars) })
      
      observeEvent(input$go,{
        output$plt <- renderPlot({ plot(rnorm(50)) })
      })
    }
    
    shinyApp(ui, server)

    I clicked the button 4 times times. I recommend you stick with an equally small number

  3. Exit the app.

  4. Enter showReactLog() at the console. This will open up a new tab in your browser and show you how each Shiny element relates to one another in your app as time moves forward (use the right arrow key for this). If you go through the trouble of dragging and dropping the nodes so that the graph is intelligible, you should end up with something like:

    reactlog

    As you can see there are FIVE output$plt objects, instead of just one. This is what is holding up the memory. You need to invalidate the previous object, rather than reassign something else to the same variable. You seem to be thinking in an imperative paradigm, while Shiny follows a reactive paradigm...

  5. Now if you run the correct version of the same app:

    library(shiny)
    
    ui <- fluidPage(
      plotOutput('plt'),
      actionButton("go", "Go")
    )
    
    server <- function(input, output, session) {
      
      output$plt <- renderPlot({ 
        if (input$go == 0) plot(cars)
        else plot(rnorm(50))
      })
    }
    
    shinyApp(ui, server)

    The you will see, that the reactlog is much neater, and in fact, you do only have one output$plt object, as you'd expect from the beginning.


Hope this was helpful, but I'd really recommend watching Joe's tutorial about Effective Reactive Programming to avoid getting bitten by this sort of thing in the future.

@wch
Copy link
Collaborator

wch commented Feb 20, 2017

One more important thing: I would not expect the use of mclapply to work correctly here. mclapply is supposed to only be used for its return value; it will not necessarily preserve side effects that occur in the parallelized function. In this case, one of those side effects is assignment to an output value.

In any case, the parallelization would only effect the assignment of output values, not the actual computation of new plots when those renderPlots are invalidated, so even if there wasn't the side-effect problem, you still would not get any performance increase.

You shouldn't be replacing the output values (like output$plot1) in the observeEvent. Instead, you should write the code inside of renderPlot to be invalidated when values$a changes.

@jcheng5
Copy link
Member

jcheng5 commented Feb 20, 2017

@bborgesr and @wch are both correct. However, I'm also surprised that redefining an output slot over and over would result in a memory leak; each time the old observer should be destroyed and garbage collected.

@wch wch reopened this Feb 22, 2017
@wch wch added the P2 label Feb 22, 2017
@bborgesr bborgesr self-assigned this Feb 22, 2017
@wch wch added the targeted label May 4, 2017
@wch wch removed P2 labels Jun 13, 2018
@awwsmm
Copy link

awwsmm commented Mar 21, 2019

Thank you for this! It's a livesaver:

tags$style(type="text/css", ".recalculating {opacity: 1.0;}")

In my opinion, this is one of the worst design decisions in R Shiny. "Blinking" plots, I think, are naturally assumed to be due to a memory leak / rendering issue, not an "invalid data" issue by the end-user. Users do not see a dimmed plot and assume the data is being updated, they see a dimmed plot and assume that you're plotting program has crashed / is eating too much memory to render the plot. This sends the wrong signal and should not be the default behaviour.

@alandipert
Copy link
Contributor

Closing because the original issue was worked around, and the memory leak @jcheng5 is surprised by looks like a dupe of #2321 (symbols aren't GC'd).

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

No branches or pull requests

6 participants