Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14804 )

Change subject: IMPALA-9174: Speedup allocations of ORC Scanner
......................................................................


Patch Set 1:

(1 comment)

The solution makes sense to me. I originally was thinking that you could have 
used the malloc extensions (we have that dependency anyway), but I think that 
gets complicated because they don't return the exact size that you allocated.

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/gperftools/gperftools/blob/f47a52ce85c3d8d559aaae7b7a426c359fbca225/src/gperftools/tcmalloc.h.in#L118

There's also an equivalent for the sanitizer malloc - 
https://github.com/llvm-mirror/compiler-rt/blob/master/include/sanitizer/allocator_interface.h#L31

We already use both these different extensions, e.g. in 
be/src/util/memory-metrics.cc, so it's not a new dependency.

I guess the sizes are a little different because they include fragmentation 
overhead. Oh right, and we need to consume the memory before allocating anyway, 
so we don't know the allocated size when we call Consume(). So this idea 
probably doesn't work out.



--
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: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Wed, 27 Nov 2019 17:20:57 +0000
Gerrit-HasComments: Yes

Reply via email to