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 1: (3 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@67 PS1, Line 67: char* addr = static_cast<char*>(std::malloc(sizeof(SizeType) + size)); > TCMalloc has an API to get the size of an allocation: https://github.com/gp Yeah, this seems a hard question. We could also TryConsume() the original size, then check the actual allocated size with tc_malloc, and then TryConsume() the difference again (and free + fail if that is not successful?). This solution would be probably much slower, but could avoid the space overhead and the possible alignment issue. http://gerrit.cloudera.org:8080/#/c/14804/1/be/src/exec/hdfs-orc-scanner.cc@78 PS1, Line 78: p -= sizeof(SizeType); : SizeType size = *reinterpret_cast<SizeType*>(p); The orc library probably doesn't rely on this, but it is generally assumed that calling free with nullptr is noop. http://gerrit.cloudera.org:8080/#/c/14804/1/be/src/exec/hdfs-orc-scanner.cc@80 PS1, Line 80: std::free(p); Not sure if this worth the added dependency, but tc_malloc has a function called tc_free_sized() which uses the size as a hint to speed up free: https://github.com/gperftools/gperftools/blob/f47a52ce85c3d8d559aaae7b7a426c359fbca225/src/gperftools/tcmalloc.h.in#L126 -- 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: 1 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-Comment-Date: Thu, 28 Nov 2019 12:50:59 +0000 Gerrit-HasComments: Yes
