impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bikramjeet Vig (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5522:Use tracked memory for DictDecoder and DictEncoder
Date Fri, 29 Sep 2017 01:16:40 GMT
Bikramjeet Vig has posted comments on this change. ( )

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

Patch Set 3:

File be/src/util/dict-encoding.h:
PS3, Line 412: ConsumeBytes(sizeof(value));
> The parquet DictionaryPageHeader contains a num_values field. Look where we
using num_values * size of type might not work when dealing with variable sized type like

I would recommend keeping count of the bytes used in a local variable and then do a ConsumeBytes
when we exit the loop. This is because mtrackp loops on all its parent trackers to update
mem usage every time Consume(num_bytes) is called. Not a huge optimization but I think it
might be worth avoiding another loop inside the hot path.
File be/src/util/
PS3, Line 39: tracker
can you add test cases that verify that the encoder/decoder is keeping track correctly.
You can do this by using tracker.consumption() to get the num of bytes consumed and compare
it to the expected size you calculate separately.

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: 3
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bikramjeet Vig <>
Gerrit-Reviewer: Joe McDonnell <>
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Fri, 29 Sep 2017 01:16:40 +0000
Gerrit-HasComments: Yes

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