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

Don't mmap datasets #61

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Don't mmap datasets #61

wants to merge 1 commit into from

Conversation

kousu
Copy link

@kousu kousu commented Jun 6, 2022

nibabel is sort of convoluted, but you can see here it has this mmap parameter:

        mmap : {True, False, 'c', 'r'}, optional, keyword only
            `mmap` controls the use of numpy memory mapping for reading image
            array data.  If False, do not try numpy ``memmap`` for data array.
            If one of {'c', 'r'}, try numpy memmap with ``mode=mmap``.  A
            `mmap` value of True gives the same behavior as ``mmap='c'``.  If
            image data file cannot be memory-mapped, ignore `mmap` value and
            read array from file.)

It looks like they turned it on by default, with the assumption that it was a performance boost, but I think on the NVMe drive on romane it might be holding us back by triggering thousands of tiny I/Os in parallel. The NVMe drive apparently can handle 400MB/s (maybe more), but not when we force it to give us individual values in the order we ask for them, which I suspect mmap() is probably implicitly doing.

https://stackoverflow.com/questions/9817233/why-mmap-is-faster-than-sequential-io

It can be - there are pros and cons, listed below. When you really have reason to care, always benchmark both.

This is all a big guess but it is an attempt to fix https://github.com/neuropoly/computers/issues/371.

I don't know if this should become a permanent part of ivadomed, but I would like you to test to see if it improves performance on our hardware.

This is an attempt to fix neuropoly/computers#371.
@kousu kousu requested a review from naga-karthik June 6, 2022 20:55
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

Successfully merging this pull request may close these issues.

1 participant