Tim Armstrong has posted comments on this change. Change subject: IMPALA-5073: Use mmap() instead of malloc() for buffer pool ......................................................................
Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/6474/7/be/src/runtime/bufferpool/system-allocator.cc File be/src/runtime/bufferpool/system-allocator.cc: Line 81: munmap(mem, fixup); > Regarding not calling from the system too often -- didn't you have to make I think we may need additional tuning, but I don't think we should tackle that in this patch. Allocating multiple buffers at once makes sense. http://gerrit.cloudera.org:8080/#/c/6474/7/be/src/util/memory-metrics.h File be/src/util/memory-metrics.h: PS7, Line 39: in use > what does that mean? i.e .does it include buffers in the free lists and TCM Rewrote the comment to be more concrete. PS7, Line 158: Total amount of buffer memory currently allocated. > it's hard to tell from this comment (and naming) if this is the total alloc Made it clearer it the amount allocated from the system. PS7, Line 159: buffer memory reserved for buffers > not sure what that means. isn't all "buffer memory" reserved for buffers? Improved the comment and clarified the relationship with the other metrics. -- To view, visit http://gerrit.cloudera.org:8080/6474 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifbc748f74adcbbdcfa45f3ec7df98284925acbd6 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
