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, 01 Nov 2017 01:07:09 GMT
Pranay Singh has posted comments on this change. ( )

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

Patch Set 12:

File be/src/exec/hdfs-parquet-scanner.h:
PS12, Line 352:   MemTracker* dict_mem_tracker() { return scan_node_->mem_tracker(); }
> See comment in HdfsParquetTableWriter. Also, see how we pass the mem_tracke
File be/src/exec/hdfs-parquet-table-writer.h:
PS12, Line 81:   /// Memory tracker to track the memory used by encoder.
             :   MemTracker* dict_mem_tracker();
> When I'm reading the code, I'm thinking that this is hiding more than it is
Removed the function made it in line where the memtracker is used so it's more explicit
File be/src/exec/
PS12, Line 179:       dict_encoder_base_->ClearIndices();
              :       dict_encoder_base_->Close();
> When Close() does a call to ClearIndices(), this can be simplified to just 
implemented the Close() logic inside ClearIndices()
PS12, Line 322:                            plain_encoded_value_size_,
              :                            parent_->dict_mem_tracker()));
> Nit: Indentation in this case should be even with the "D" in new DictEncode
aligned the indentation with D in next line.
File be/src/util/dict-encoding.h:
PS12, Line 63:   void Close() {
             :     ReleaseBytes();
> I think Close() should do ClearIndices().
implemented the logic of Close() inside ClearIndices()

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: 12
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bikramjeet Vig <>
Gerrit-Reviewer: Joe McDonnell <>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Comment-Date: Wed, 01 Nov 2017 01:07:09 +0000
Gerrit-HasComments: Yes

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