r-spatial / leafgl

R package for fast web gl rendering for leaflet
Other
261 stars 31 forks source link

Possible memory leak ? #49

Open vlarmet opened 3 years ago

vlarmet commented 3 years ago

Hi,

First of all, thank you for this great package ! I want to use leafgl to display a large shapefile (~35000 polygons) in Shiny. The user can choose a variable to map and the map is updated through leafletProxy. However, I notice that memory usage increase each time the user choose a new variable, here is a reproductible example.

library(shiny)
library(leaflet)
library(leafgl)
library(raster)
library(sf)

# Create 50000 pixels raster
r <- raster(extent(matrix( c(-90, -50, 90,  50), nrow=2)), nrow=500, ncol=100, 
            crs = "+proj=longlat +datum=WGS84 +no_defs +ellps=WGS84 +towgs84=0,0,0") 

Extent <- extent(r)

# Convert to sf shapefile
shp <- rasterToPolygons(r)
shp <- st_as_sf(shp)

## UI ##########
ui <- fluidPage(
  actionButton("go","go"),
  leafglOutput("map")
)

## SERVER ##########
server <- function(input, output) {
  output$map <- renderLeaflet({
    leaflet()  %>% 
      addTiles() %>% 
      fitBounds(lng1 = Extent[1],lat1 = Extent[3], lng2 = Extent[2], lat2 = Extent[4])
  })

  observeEvent(input$go,{
    random <- sample(1:100,50000,replace = T)
    pal = colorQuantile(rev(viridis::viridis(10)), random , n = 10)

    leafletProxy("map") %>%
      leafgl::removeGlPolygons(layerId = "polygons") %>%
      leafgl::addGlPolygons(layerId = "polygons",data = shp, color = ~pal(random))
  })
}

shinyApp(ui, server)

Each time I click go button, memory increase by about 100Mo.

trafficonese commented 3 years ago

I think that problem might come from the upstream repo, which is not fully removing the glify instances. (see this issue) In the browser console you can see that L.glify.instances increases with every click on go.

vlarmet commented 3 years ago

Thanks for the reply. So there is no way to fix that issue at the moment ?

trafficonese commented 3 years ago

If those are the only polygons you are plotting, you could remove the instances yourself with shinyjs.

In the ui you need to include useShinyjs() and after the leafletProxy call you can run runjs("L.glify.Shapes.instances.splice(0, 1);"). Maybe that would work as a workaround?

tim-salabim commented 3 years ago

Thanks @trafficonese for chiming in here. I have sent you an invite with write access to this repo. I figure you're contributing so much here, you might as well have write access...

vlarmet commented 3 years ago

Wow thank you very much @trafficonese ! It work like a charm.

edit : here is the solution

library(shiny)
library(leaflet)
library(leafgl)
library(raster)
library(sf)
library(shinyjs)
library(jsonify)
library(geojsonsf)

# Create 50000 pixels raster
r <- raster(extent(matrix( c(-90, -50, 90,  50), nrow=2)), nrow=500, ncol=100, 
            crs = "+proj=longlat +datum=WGS84 +no_defs +ellps=WGS84 +towgs84=0,0,0") 

Extent <- extent(r)

# Convert to sf shapefile
shp <- rasterToPolygons(r)
shp <- st_as_sf(shp)

## UI ##########
ui <- fluidPage(
  useShinyjs(),
  actionButton("go","go"),
  leafglOutput("map")
)

## SERVER ##########
server <- function(input, output) {
  output$map <- renderLeaflet({
    leaflet()  %>% 
      addTiles() %>% 
      fitBounds(lng1 = Extent[1],lat1 = Extent[3], lng2 = Extent[2], lat2 = Extent[4])
  })

  observeEvent(input$go,{
    random <- sample(1:100,50000,replace = T)
    pal = colorQuantile(rev(viridis::viridis(10)), random , n = 10)

    runjs("L.glify.Shapes.instances.splice(0, 1);")

    leafletProxy("map") %>%
      leafgl::removeGlPolygons(layerId = "polygons") %>%
      leafgl::addGlPolygons(layerId = "polygons",data = shp, color = ~pal(random))
  })
}

shinyApp(ui, server)
trafficonese commented 3 years ago

I would keep this issue open as this is more of a dirty hack than a proper fix. If multiple layers are included and the one to be removed is not the first in the array, the solution will not work. So, I think this should actually be handled in the underlying library.

Thanks @tim-salabim, I really appreciate it!

h-a-graham commented 2 years ago

Hey all, I'm coming up against this issue using addGlPolylines() and varSelectInput() - If Ichange the column input to the polylines more than ~3 times the app runs out of memory. I have tried the suggested workaround here along with the following (and some variations around it):

runjs("L.glify.Lines.instances.splice(0, 1);")

Neither seem to solve the problem - just wondering if there is an updated workaround or alternative that I can consider? many thanks, Hugh

trafficonese commented 1 month ago

The issue is documented here https://github.com/robertleeplummerjr/Leaflet.glify/issues/129#issue-1260671418

I just pushed a PR to the upstream repo to address it. https://github.com/robertleeplummerjr/Leaflet.glify/pull/153#issue-2330622238

In my opinion this is currently the biggest issue for using the library in an interactive context (e.g. Shiny), where one needs to remove/update a layer many times. Otherwise we just add new WebGL contexts and at some point the browser will tell us that the oldest WebGL contexts are lost and the old layers are gone..

❗ When this gets merged and we add it here, the old workaround using runjs("L.glify.Lines.instances.splice(0, 1);") needs to get removed.

Also @tim-salabim, since you updated to the new Leaflet.glify version (which btw. is not registered in Github as a release ❓ ), we should expose the methods update/remove/insert for the same reasons.

I will try to find some time at the end of this week to open a new PR, if thats ok with you?

tim-salabim commented 1 month ago

Thanks @trafficonese ! That's fine. I'll take a look at your PR as soon as it's ready