impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) PREVIEW: Use mmap instead of malloc for buffer pool
Date Fri, 13 May 2016 23:40:34 GMT
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 <tarmstrong@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message