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

Memory map: mmap+memcopy instead pread+pwrite #11484

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

monkuta
Copy link

@monkuta monkuta commented Jun 27, 2024

Allows for only-in-memory caching for usecases where such behavior is needed (i.e. Live Video caching). It maps storage directly into user space memory, so reads and writes are being done by a memcpy (without calls to io).

To build:
cmake -B build -DENABLE_HWLOC=ON -DENABLE_MMAP=ON -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=OFF
cmake --build build -j
cmake --install build

@JosiahWI
Copy link
Contributor

JosiahWI commented Jun 27, 2024

Thanks for making the PR against the master branch. Should #10832 be closed?

@monkuta
Copy link
Author

monkuta commented Jun 27, 2024

Yes #10832 should be closed. As per the comment, it has been merged into the master branch.

@bneradt
Copy link
Contributor

bneradt commented Jun 27, 2024

[approve ci]

@JosiahWI
Copy link
Contributor

JosiahWI commented Jun 27, 2024

@monkuta Please run cmake --build build --target format.

@monkuta
Copy link
Author

monkuta commented Jun 28, 2024

Changes have been made. (6df2474)

@bryancall
Copy link
Contributor

@monkuta Do you have benchmarking number without and with this change?

@bryancall bryancall requested a review from moonchen July 1, 2024 22:15
@moleksy
Copy link

moleksy commented Jul 4, 2024

@bryancall
For 1MB objects, 100% cache hit ratio mean latency drops from 800ms to 23ms @ 33.27GiB/s transfer rate. The achievable transfer rate for 50kB objects at 100% cache hit ratio is 4.84GiB/s compared to 0.94GiB/s for baseline at comparable latencies.

Copy link
Contributor

@moonchen moonchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for making this PR. Why is it that the stripe mutex is needed for mmap I/O when it's not needed for syscall I/O?

src/iocore/aio/AIO.cc Outdated Show resolved Hide resolved
#endif
} else {
#ifdef AIO_FAULT_INJECTION
err = aioFaultInjection.pwrite(a->aio_fildes, (static_cast<char *>(a->aio_buf)) + res, a->aio_nbytes - res,
a->aio_offset + res);
#else
#if TS_USE_MMAP
ink_assert((static_cast<char const *>(a->aio_fildes.first) + a->aio_offset + a->aio_nbytes) <= a->aio_fildes.last);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto here

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

Successfully merging this pull request may close these issues.

6 participants