Tim Armstrong has posted comments on this change. Change subject: IMPALA-5658: addtl. process/system-wide memory metrics ......................................................................
Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/7472/3/be/src/common/init.cc File be/src/common/init.cc: PS3, Line 132: v > nit extra . Done http://gerrit.cloudera.org:8080/#/c/7472/3/be/src/util/mem-info.cc File be/src/util/mem-info.cc: PS3, Line 132: } else if (name == "AnonHugePages") { : result.anon_huge_pages_kb += atol(value.c_str()); : } : } : return result; : } : : ThpConfig MemInfo::ParseThpConfig() { : Thp > I'm confused why this works for "Size: 1084 kB" - wouldn't the "kB" still b atol() only parses the leading digits of the string and ignores any following characters. http://gerrit.cloudera.org:8080/#/c/7472/2/be/src/util/mem-info.h File be/src/util/mem-info.h: PS2, Line 29: struct MappedMemInfo { : // Number of memory maps. : int64_t num_maps = 0; : // Total size of memory maps (i.e. virtual memory size) in kilobytes. : int64_t size_kb = 0; : // RSS in kilobytes : int64_t rss_kb = 0; : // Kilobytes of anonymous huge pages. : int64_t anon_huge_pages_kb = 0; > I agree testing will be hard, and probably this is fine, but that doesn't m That makes sense because that predates THP. It seems ok to assume that if it's not present there are 0 THPs. There seems to be a small range of Red Hat kernel versions with THP support but this interface not enabled, but the window is pretty small - this was added to smaps in early 2011 https://github.com/torvalds/linux/commit/4031a219d8913da40ade5a6e5b538cc61e975cc8 but Red Hat released RHEL 6.0 in late 2010 with THP support. It does appear to be the canonical linux interface for accessing this info. The pmap utility seems to do the same style of parsing as we're doing: https://gitlab.com/procps-ng/procps/blob/master/pmap.c#L314, so anything that broke this would probably break pmap too. -- To view, visit http://gerrit.cloudera.org:8080/7472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I13873e305ba464d11dea0d7244a29ff4f332f1a9 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
