Tim Armstrong has posted comments on this change.
Change subject: PREVIEW: Use mmap instead of malloc for buffer pool
......................................................................
Patch Set 1:
(2 comments)
I think by madvise-ing a particular region we only encourage the kernel to scan
those pages. I think either the kernel should successfully coalesce the pages
the first time it tries to scan the range we suggested.
We don't recommend running with automatic THP ("always" mode) but it should
work fine with "madvise" mode, where it only scans the pages we explicitly call
out.
I think we'd see that behaviour if we made bogus madvise calls (e.g. on ranges
too small to coalesce) or tried to unmap parts of the buffer, but we shouldn't
plan on doing that.
http://gerrit.cloudera.org:8080/#/c/3046/1/be/src/bufferpool/buffer-allocator.cc
File be/src/bufferpool/buffer-allocator.cc:
Line 58: int rc = madvise(mem, len, MADV_HUGEPAGE);
> Do you need to do this if the call on L41 succeeds? Wouldn't it already be
It already does this: it only goes down this path if mem == MAP_FAILED.
Line 69: int rc = munmap(buffer, len);
> After unmapping a huge page, would the kernel still treat that virtual addr
I think the madvise() info should be cleared when the memory is unmapped. The
docs that I can't find don't explicitly explain that situation but madvise() on
unmapped memory is a no-op so it would be strange if it somehow persisted after
unmapping.
"If there are some parts of the specified address range that are not mapped,
the Linux version of madvise() ignores them and applies the call to the rest
(but returns ENOMEM from the system call, as it should)."
--
To view, visit http://gerrit.cloudera.org:8080/3046
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbc748f74adcbbdcfa45f3ec7df98284925acbd6
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Jim Apple <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes
