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

openslide memory leak #24

Open
gabricampanella opened this issue Aug 25, 2017 · 6 comments
Open

openslide memory leak #24

gabricampanella opened this issue Aug 25, 2017 · 6 comments

Comments

@gabricampanella
Copy link

Context

Issue type (bug report):
Operating system (CentOS 7.3.1611):
Platform (x86_64 GNU/Linux):
OpenSlide Python version (1.1.0):
Slide format (SVS):

Details

Dear openslide team,
Thank you for making this software available. It greatly facilitates my research. I am building large-scale pipelines for digital pathology that rely on extracting tiles from thousands of slides on the fly. To achieve this I create all openslide objects at the beginning and append them to a list. I fetch tiles by looping through a list of pixel coordinates and slide index. The loop somehow leaks memory. The memory used keeps increasing until out of memory. The script below reproduces this behavior. You only need to change the path to a slide to make it work.

import openslide
import random
import sys
import numpy as np

class library(object):
    def __init__(self, slides, idx, grid, size):
        sl = []
        for i,slide in enumerate(slides):
            sl.append(openslide.OpenSlide(slide))
            sys.stdout.write("{}/{} \r".format(i+1,len(slides)))
            sys.stdout.flush()
        tup = zip(grid,idx)
        self.size = size
        self.slides = slides
        self.tup = tup
        self.sl = sl

    def make_list_of_objs(self):
        sl = []
        for i,slide in enumerate(self.slides):
            sl.append(openslide.OpenSlide(slide))
            sys.stdout.write("{}/{} \r".format(i+1,len(self.slides)))
            sys.stdout.flush()
        self.sl = sl

    def __getitem__(self, index):
        tup = self.tup[index]
        img = self.sl[tup[1]].read_region(tup[0],0,(self.size,self.size)).convert('RGB')
        return img

    def __len__(self):
        return len(self.tup)

class batch_loader(object):
    def __init__(self,dset,n):
        batches = []
        for i in range(0,dset.__len__(),n):
            d = min(n,dset.__len__()-i)
            batches.append(range(i,i+d))
        self.batches = batches

#MAKE DATA
slides = ['/scratch/gabriele/prostate_dict/389973.svs']*2000 #CHANGE SLIDE NAME
idx = list(np.random.choice(len(slides),2000000,replace=True))
gridx = list(np.random.choice(10000,2000000,replace=True))
gridy = list(np.random.choice(10000,2000000,replace=True))
grid = zip(gridx,gridy)

#SET UP LOADER
dset = library(slides,idx,grid,224)
loader = batch_loader(dset,512)

#LEAKING LOOP
for ii,batch in enumerate(loader.batches):
    imgs = []
    for i,idx in enumerate(batch):
        img = np.array(dset.__getitem__(idx))
        imgs.append(img)
    sys.stdout.write("{}/{} \r".format(ii+1,len(loader.batches)))
    sys.stdout.flush()

To circumvent this one can delete the list of openslide objects and recreate it every few iterations. This stabilizes memory footprint but it is an unfortunate work-around that wastes a lot of time reopening openslide objects:

import openslide
import random
import sys
import numpy as np

class library(object):
    def __init__(self, slides, idx, grid, size):
        sl = []
        for i,slide in enumerate(slides):
            sl.append(openslide.OpenSlide(slide))
            sys.stdout.write("{}/{} \r".format(i+1,len(slides)))
            sys.stdout.flush()
        tup = zip(grid,idx)
        self.size = size
        self.slides = slides
        self.tup = tup
        self.sl = sl
        self.idx = 0

    def make_list_of_objs(self):
        sl = []
        for i,slide in enumerate(self.slides):
            sl.append(openslide.OpenSlide(slide))
            sys.stdout.write("{}/{} \r".format(i+1,len(self.slides)))
            sys.stdout.flush()
        self.sl = sl

    def __getitem__(self, index):
        if self.idx > 5120:
            self.idx = 0
            self.make_list_of_objs()
        self.idx += 1
        tup = self.tup[index]
        img = self.sl[tup[1]].read_region(tup[0],0,(self.size,self.size)).convert('RGB')
        return img

    def __len__(self):
        return len(self.tup)

class batch_loader(object):
    def __init__(self,dset,n):
        batches = []
        for i in range(0,dset.__len__(),n):
            d = min(n,dset.__len__()-i)
            batches.append(range(i,i+d))
        self.batches = batches

#MAKE DATA
slides = ['/scratch/gabriele/prostate_dict/389973.svs']*2000 #CHANGE SLIDE NAME
idx = list(np.random.choice(len(slides),2000000,replace=True))
gridx = list(np.random.choice(10000,2000000,replace=True))
gridy = list(np.random.choice(10000,2000000,replace=True))
grid = zip(gridx,gridy)

#SET UP LOADER
dset = library(slides,idx,grid,224)
loader = batch_loader(dset,512)

#LEAKING LOOP
for ii,batch in enumerate(loader.batches):
    imgs = []
    for i,idx in enumerate(batch):
        img = np.array(dset.__getitem__(idx))
        imgs.append(img)
    sys.stdout.write("{}/{} \r".format(ii+1,len(loader.batches)))
    sys.stdout.flush()

Thanks for your patience.

@bgilbert
Copy link
Member

Hi @gabricampanella. Each OpenSlide object has an internal cache of decoded pixel data that can grow to 32 MiB. There's currently no API to configure the size of that cache (openslide/openslide#38).

My usual recommendation is to keep a fixed-length cache of OpenSlide handles, closing old ones as necessary. That works reasonably well if the workload exhibits locality, such as in the backend of a web-based slide viewer, but of course it's not great for truly random workloads. Do you have any opportunity to coalesce multiple accesses to a slide?

@gabricampanella
Copy link
Author

Thanks for your reply. If I understand correctly, if each handle can grow to 32 MiB then if I open 2000 of them, they should grow no more than 64 GiB. But I observe a memory footprint that goes well beyond that.
I believe that in a supervised learning framework the fact that samples are drawn independently from each other is important. I think complete randomness when extracting tiles is more correct, but practically it could be that good learning is achieved also by having batches of tiles all coming from the same slide. For now I am more inclined to use the hack than to eliminate random sampling.

@bgilbert
Copy link
Member

For what it's worth, you're also storing at least 373 GiB of pixel data in imgs.

If you have a fixed set of samples to collect, as in your reproduction code, you could do it in two passes: first pick a set of samples, then sort by source slide, extract the pixel data, and re-sort into the original order. That way you'd only need to have one slide open at a time.

@gabricampanella
Copy link
Author

imgs should contain only 512 images of size 3x224x224 (In other words 77M floats). It gets rewritten at every step of the loop. It definitely shouldn't grow to 370GiB.

I see what you are saying. In that case If I am unlucky I would need to open 512 slides per batch (if the batch size is 512). In the best case scenario I only get to open 1. It is an interesting idea. I'll think about it.

@bgilbert
Copy link
Member

Whoops, you're right about imgs, I misread.

@happyalfred2016
Copy link

I have a similar issue. I am trying to load patches from slides in the training of deep learning model. But even though I closed the slides (OpenSlide.close()) after each patch loading, the memory still grow heavily. Do you have any suggestions or workaround?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants