Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3546: don't die if sysconf() reports bogus cache info
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3110/1/be/src/util/cpu-info.cc
File be/src/util/cpu-info.cc:

Line 192:   string L1 = Substitute("L1 Cache: $0 (Line: $1)",
        :       PrettyPrinter::Print(CacheSize(L1_CACHE), TUnit::BYTES),
        :       PrettyPrinter::Print(CacheLineSize(L1_CACHE), TUnit::BYTES));
        :   string L2 = Substitute("L1 Cache: $0 (Line: $1)",
        :       PrettyPrinter::Print(CacheSize(L2_CACHE), TUnit::BYTES),
        :       PrettyPrinter::Print(CacheLineSize(L2_CACHE), TUnit::BYTES));
        :   string L3 = Substitute("L1 Cache: $0 (Line: $1)",
        :       PrettyPrinter::Print(CacheSize(L3_CACHE), TUnit::BYTES),
        :       PrettyPrinter::Print(CacheLineSize(L3_CACHE), TUnit::BYTES));
Since the cache info is never used anywhere else (and I guess we can't rely on 
it anyway), it seems like a shame to bother storing it at all. Consider 
removing the methods & members and instead just getting the info right here to 
print it. If we need to get this in ASAP I'm fine with this for now...


-- 
To view, visit http://gerrit.cloudera.org:8080/3110
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4d61b05fe213028a7e9aaabe98adc2792b90e07
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Casey Ching <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-HasComments: Yes

Reply via email to