Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/24761 )
Change subject: IMPALA-15316: Support huge pages with aggressive decommit off ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/24761/3/be/src/runtime/bufferpool/buffer-allocator-test.cc File be/src/runtime/bufferpool/buffer-allocator-test.cc: http://gerrit.cloudera.org:8080/#/c/24761/3/be/src/runtime/bufferpool/buffer-allocator-test.cc@188 PS3, Line 188: ASSERT_OK(allocator.Allocate(len, &buffer, &mmapped_bytes_)); Might make sense to assert the value of mmapped_bytes_. http://gerrit.cloudera.org:8080/#/c/24761/3/be/src/runtime/bufferpool/system-allocator.cc File be/src/runtime/bufferpool/system-allocator.cc: http://gerrit.cloudera.org:8080/#/c/24761/3/be/src/runtime/bufferpool/system-allocator.cc@140 PS3, Line 140: malloc_huge_page_support_ != MallocUtil::HugePageSupport::MADVISE_UNNECESSARY) { Logically I think this is a little clearer as malloc_huge_page_support_ == MallocUtil::HugePageSupport::MADVISE_COMPATIBLE. The DCHECK_NE can live anywhere in this function. http://gerrit.cloudera.org:8080/#/c/24761/3/be/src/runtime/bufferpool/system-allocator.cc@159 PS3, Line 159: bool mmapped_huge_page = huge_page_sized && FLAGS_madvise_huge_pages && Maybe this should be a helper function since we use it in two places. -- To view, visit http://gerrit.cloudera.org:8080/24761 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I13ac5f065104d5cf35efb5029ba3c4d482904096 Gerrit-Change-Number: 24761 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Fri, 28 Aug 2026 17:21:43 +0000 Gerrit-HasComments: Yes
