Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14804 )

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


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14804/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/14804/4//COMMIT_MSG@19
PS4, Line 19: malloc already stores the size of the allocated bytes in front of 
the
            : allocated bytes. That's why 'free()' doesn't take a 'size' 
parameter,
            : it just searches for the size information next to the pointed 
bytes.
            : TC Malloc provides a function called 'tc_malloc_size()' that 
reveals
            : that information programmatically, so instead of the hash table we
            : can just use this function to retrieve the size.
> This is not exactly how tc malloc stores sizes. There is an explanation in
Thanks for the clarification. I changed the wording.


http://gerrit.cloudera.org:8080/#/c/14804/4/be/src/exec/hdfs-orc-scanner.cc
File be/src/exec/hdfs-orc-scanner.cc:

http://gerrit.cloudera.org:8080/#/c/14804/4/be/src/exec/hdfs-orc-scanner.cc@63
PS4, Line 63: malloc
> I think it would be better to move most of this functionality into MemTrack
Currently MemTracker is only used to do the bookeeping of allocated memory, it 
doesn't allocate/release.
Objects of MemPool and ObjectPool are used for allocations, they have an 
internal MemTracker to check memory limits for TryAllocate*() functions. 
However, they cannot release arbitrary memory, they can only release all 
allocated memory at once.

I'd rather have this logic only here, since later we'll probably need to get 
rid of this OrcMemPool anyway, and switch to buffer-pool-based allocations.



--
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: 4
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: Wed, 11 Dec 2019 12:53:16 +0000
Gerrit-HasComments: Yes

Reply via email to