Baike Xia has posted comments on this change. ( http://gerrit.cloudera.org:8080/18987 )
Change subject: IMPALA-11564: For Agg/Scan nodes, increase the Cache of regular expressions to speed up ...................................................................... Patch Set 4: (3 comments) > Patch Set 3: > > (4 comments) http://gerrit.cloudera.org:8080/#/c/18987/3/be/src/exec/exec-node-thread-cache.h File be/src/exec/exec-node-thread-cache.h: http://gerrit.cloudera.org:8080/#/c/18987/3/be/src/exec/exec-node-thread-cache.h@37 PS3, Line 37: boost::unordered_map<std::string, int> regex_cache_key_map_; > map internally is a BST while unordered_map is a hash table, since we care OK. http://gerrit.cloudera.org:8080/#/c/18987/3/be/src/exec/exec-node-thread-cache.h@40 PS3, Line 40: // 0 indicates the initial status; : // 1 indicates successful matching; : // 2 indicates failure matching; : // 3 indicates null; > how about using an enum to represent these states? These states are only used internally and are not exposed to the outside world, so there is no need to use enumerations. http://gerrit.cloudera.org:8080/#/c/18987/3/be/src/exec/exec-node-thread-cache.h@76 PS3, Line 76: int cache_status = regex_cache_[cache_id]; > how about directly using the string `cache_key` to get the cache value? you In the current implementation, result state can be saved(init, successful, failure, null), and as of now, callers only have LikePredicate. The complexity of using unordered_map is O(1). So I thought, do we need to change it? -- To view, visit http://gerrit.cloudera.org:8080/18987 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I68f37303aee4b6a28e560f27548c31472b82048b Gerrit-Change-Number: 18987 Gerrit-PatchSet: 4 Gerrit-Owner: Baike Xia <[email protected]> Gerrit-Reviewer: Baike Xia <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jian Zhang <[email protected]> Gerrit-Comment-Date: Mon, 10 Oct 2022 06:35:12 +0000 Gerrit-HasComments: Yes
