Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12657 )
Change subject: KUDU-2721: support ranges in CPU lists ...................................................................... Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/12657/6/src/kudu/gutil/sysinfo.cc File src/kudu/gutil/sysinfo.cc: http://gerrit.cloudera.org:8080/#/c/12657/6/src/kudu/gutil/sysinfo.cc@168 PS6, Line 168: CHECK(SlurpSmallTextFile("/sys/devices/system/cpu/possible", buf, arraysize(buf))); interestingly, on the machines I have access to, the number in 'possible' is much higher than the number in 'present'. For example, on a 48-core E5-2680v3, 'possible' returns 0-143, and on an 88-core E5-2699v4, I get 0-191. Although I see that we might have some trouble with hotplug, I'm also nervous about the memory blowup on any structures which are "percpu". Looking through the kudu code I don't see too many that have more than thousands of possible instances -- so this would only represent a few MB of memory blowup. Do you have any usage of percpu_rwlock in Impala? http://gerrit.cloudera.org:8080/#/c/12657/6/src/kudu/gutil/sysinfo.cc@174 PS6, Line 174: int ParseMaxCpuIndex(const char* str) { should probably be static http://gerrit.cloudera.org:8080/#/c/12657/6/src/kudu/gutil/sysinfo.cc@183 PS6, Line 183: // newline or comma if the input is valid. Instead of the hand-rolled parsing code, maybe it's easier to just move the MaxCPUIndex initialization code outside of the InitializeSystemInfo path and have its own lazy-init path using its own 'once'? Then you can just use normal Split/StrToInt calls etc to make this a lot simpler -- To view, visit http://gerrit.cloudera.org:8080/12657 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I97311bfbcf70bea069e941b6e7f4f015fb781b3f Gerrit-Change-Number: 12657 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Tue, 05 Mar 2019 17:18:42 +0000 Gerrit-HasComments: Yes
