impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pranay Singh (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder
Date Wed, 18 Oct 2017 05:28:47 GMT
Pranay Singh has posted comments on this change. ( )

Change subject: IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder

Patch Set 9:

File be/src/exec/
PS8, Line 348: f the column.
> We should be using Close() here instead of unregistering every tracker (thi
Removed the Close() since I'm using the MemTracker of 
scan_node_->mem_tracker so won't be using 1000's of memtrackers now
File be/src/util/dict-encoding.h:
PS8, Line 133:     di
> nit: inline here and nearby isn't necessary for a couple of reason. First, 
Removed inline
PS8, Line 184:     Node(const T& v, const NodeIndex& n) : value(v), next(n) { }
> Nit: check isn't necessary - Release() does this for you.
Removed the check
PS8, Line 194:     enum { INVALID_INDEX = 40000 };
> The TODO should be to use TryConsume(). The asynchronous checks are not the
Changed the TODO to use TryConsume()
PS8, Line 251: / This function
> I agree with Joe's comment. All codepaths should call Close(), so we can ju
Added a DCHECK found during the testing that some of the column readers do not call Close,
so I've added ReleaseBytes() so that accounting of memory used for dictionary is release when
DictDecoder is destroyed.

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I02a3b54f6c107d19b62ad9e1c49df94175964299
Gerrit-Change-Number: 8034
Gerrit-PatchSet: 9
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bikramjeet Vig <>
Gerrit-Reviewer: Joe McDonnell <>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Comment-Date: Wed, 18 Oct 2017 05:28:47 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message