Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/14804 )
Change subject: IMPALA-9174: Speedup allocations of ORC Scanner ...................................................................... Patch Set 2: Code-Review+1 (2 comments) http://gerrit.cloudera.org:8080/#/c/14804/1/be/src/exec/hdfs-orc-scanner.cc File be/src/exec/hdfs-orc-scanner.cc: http://gerrit.cloudera.org:8080/#/c/14804/1/be/src/exec/hdfs-orc-scanner.cc@74 PS1, Line 74: return addr + HEADER_SIZE; > Thanks for noting it. Currently the ORC lib doesn't assume 16 byte alignmen nitpicking: "it still uses less memory than the Base version" is not necessarily true - probably it is better for small allocations but can be worse for large ones. Adding even 1 byte to the allocations can move to a different size class, e.g. allocating 2049 instead of 2048 bytes will need 256 more bytes according to http://goog-perftools.sourceforge.net/doc/tcmalloc.html http://gerrit.cloudera.org:8080/#/c/14804/1/be/src/exec/hdfs-orc-scanner.cc@80 PS1, Line 80: SizeType size = *reinterpret_cast<SizeType*>(p); > Thanks for the pointer, although I'm not sure if it worth it, I'd rather ke It doesn't matter AFAIK - size is rounder up to certain sizes (size classes), which will be the allocated size. So the original and allocated size should have the same size class, which is needed during free(). Found an issue that suggests that we don't win too much with using the size hint: https://github.com/gperftools/gperftools/issues/1096 Maybe a TODO can be added to give a hint if further optimization will be needed in the future. -- To view, visit http://gerrit.cloudera.org:8080/14804 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia09e746883176d6f955c1718267bf55e2abb239b Gerrit-Change-Number: 14804 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 28 Nov 2019 15:42:28 +0000 Gerrit-HasComments: Yes
