Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17592 )
Change subject: IMPALA-7635: Reducing HashTable size by packing it's buckets efficiently. ...................................................................... Patch Set 12: (1 comment) Looks great! http://gerrit.cloudera.org:8080/#/c/17592/12/be/src/exec/hash-table.h File be/src/exec/hash-table.h: http://gerrit.cloudera.org:8080/#/c/17592/12/be/src/exec/hash-table.h@705 PS12, Line 705: If bucket doesn't have tag fields set, TAGGED can : /// be set to 'false' to avoid extra bit operations. nit. I think the following may describe the semantics better: When 'data' may contain tagging bits, use TAGGED being equal true version of this method. This comment probably should become item 3. of the description for this class (e.g., at line 697) as there are several methods with the TAGGED argument. I also wonder if we can improve the performance a bit by splitting the method into two to avoid the IF test 1. SetBucketDataUnTagged() (untagged version) 2. SetBucketDataTagged() (tagged version) Another option would be to add UNLIKELY() for untagged branch. -- To view, visit http://gerrit.cloudera.org:8080/17592 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I72912ae9353b0d567a976ca712d2d193e035df9b Gerrit-Change-Number: 17592 Gerrit-PatchSet: 12 Gerrit-Owner: Amogh Margoor <amarg...@gmail.com> Gerrit-Reviewer: Amogh Margoor <amarg...@gmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 24 Aug 2021 13:37:33 +0000 Gerrit-HasComments: Yes
