[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Impala Public Jenkins has submitted this change and it was merged. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. IMPALA-5116: Remove deprecated hash_* types in gutil The following class templates are substituted from C++11 standard. __gnu_cxx::hash_map => std::unordered_map __gnu_cxx::hash_set => std::unordered_set __gnu_cxx::hash => std::hash Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Reviewed-on: http://gerrit.cloudera.org:8080/7414 Reviewed-by: Jim Apple Tested-by: Impala Public Jenkins --- M be/src/gutil/hash/hash.cc M be/src/gutil/hash/hash.h M be/src/gutil/strings/join.h M be/src/gutil/strings/serialize.cc M be/src/gutil/strings/serialize.h M be/src/gutil/strings/split.cc M be/src/gutil/strings/split.h 7 files changed, 37 insertions(+), 185 deletions(-) Approvals: Impala Public Jenkins: Verified Jim Apple: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Impala Public Jenkins has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1058/ -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Jim Apple has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Jim Apple has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 6: Thanks for fixing those last comments! I'm starting the pre-merge job now; it takes roughly four hours, and it will commit this patch if it passes all of the tests. -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Jim Apple has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Kim Jin Chul has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 5: Code-Review+1 (5 comments) http://gerrit.cloudera.org:8080/#/c/7414/5/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: Line 101 Unnecessary include because of std::hash Line 117 MSVC stuff is removed Line 242 MSVC stuff is removed Line 305 MSVC stuff is removed Line 318 MSVC stuff is removed -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7414 to look at the new patch set (#5). Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. IMPALA-5116: Remove deprecated hash_* types in gutil The following class templates are substituted from C++11 standard. __gnu_cxx::hash_map => std::unordered_map __gnu_cxx::hash_set => std::unordered_set __gnu_cxx::hash => std::hash Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 --- M be/src/gutil/hash/hash.cc M be/src/gutil/hash/hash.h M be/src/gutil/strings/join.h M be/src/gutil/strings/serialize.cc M be/src/gutil/strings/serialize.h M be/src/gutil/strings/split.cc M be/src/gutil/strings/split.h 7 files changed, 37 insertions(+), 185 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/7414/5 -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Kim Jin Chul has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 4: (2 comments) Sorry for the late. Let me answer Jim's comment soon. http://gerrit.cloudera.org:8080/#/c/7414/3/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: PS3, Line 96: using std::hash; : > What code requires this remain? I answered in the PS#4 Line 189: > What code requires that this section remain? I answered in the PS#4 -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Tim Armstrong has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 4: Did you have a chance to look at Jim's comments? We are very close to getting this merged. -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Jim Apple has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 4: (5 comments) I may not have expressed my previous comments in a simple way. Let me try again. http://gerrit.cloudera.org:8080/#/c/7414/4/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: Line 96: using std::hash; Can these two lines be deleted? Line 187: #include Can this line be deleted? Line 208: bool operator()(const uint128& a, const uint128& b) const { Can this MSCV stuff be deleted? Line 215: // MSVC's STL requires an ever-so slightly different decl Can this MSVC stuff be deleted? Line 244: bool operator()(const pair& a, Can this MSVC stuff be deleted? -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Tim Armstrong has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 4: Code-Review+1 The change looks good to me. Thanks for contributing. It looks like you addressed Jim's remaining comments, but I'll let him confirm. -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Kim Jin Chul has uploaded a new patch set (#4). Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. IMPALA-5116: Remove deprecated hash_* types in gutil The following class templates are substituted from C++11 standard. __gnu_cxx::hash_map => std::unordered_map __gnu_cxx::hash_set => std::unordered_set __gnu_cxx::hash => std::hash Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 --- M be/src/gutil/hash/hash.cc M be/src/gutil/hash/hash.h M be/src/gutil/strings/join.h M be/src/gutil/strings/serialize.cc M be/src/gutil/strings/serialize.h M be/src/gutil/strings/split.cc M be/src/gutil/strings/split.h 7 files changed, 37 insertions(+), 154 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/7414/4 -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Jim Apple has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/7414/3/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: PS3, Line 96: #include : using std::hash; What code requires this remain? Line 189: namespace std { What code requires that this section remain? -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Kim Jin Chul has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7414/1/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: Line 278 > I looked at your comment. I did not respond yet because it is premature for I found the specialized code from 218 to 328 is unused. My conclusion is the code can be removed because of the reason below. I believe there is no logical issue on my change. 1. hash functions are only used in hash_map/hash_set (Checking with the command: git grep __gnu_cxx src) 2. hash_map/hash_set => unordered_map/unordered_set The key type is only 'string'. std::hash is specialized in C++11. -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Jim Apple has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7414/1/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: Line 278 > I understand what you worry about. Let me look into the code more deeply. I looked at your comment. I did not respond yet because it is premature for us to optimize a hash function before we understand where it is used. -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Kim Jin Chul has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/7414/1/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: Line 222 > I think you might be right. Where was it used before your patch? In the has Yes, it seems the specializations for hash are only used in the hash_* container. e.g. 00082 template, 00083class _EqualKey = equal_to<_Key>, class _Alloc = allocator<_Tp> > 00084 class hash_map By the way, I have not found any use case to create an object for hash_* container, so the code is now unused. Do you think we should leave it to use it later? Or it is better to remove the code to improve code coverage? Line 278 > My suggestion is to dig into this library and understand the details of wha I understand what you worry about. Let me look into the code more deeply. By the way, did you check my comment in the previous patch set? It would be nice if you take a look at the comment: https://gerrit.cloudera.org/#/c/7414/3/be/src/gutil/hash/hash.h@a251 -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Jim Apple has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/7414/1/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: Line 222 > I guess the reason is that __gnu_cxx::hash<> was not used anywhere. I didn I think you might be right. Where was it used before your patch? In the hash_set/hash_map definitions? Line 278 > I got it. My idea is that some specializations in __gnu_cxx should be moved My suggestion is to dig into this library and understand the details of what hash functions are used where. As-is, it seems a little bit like you are trying things just to see what seems to work, which is a dangerous way to live in C++. -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Kim Jin Chul has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/7414/3/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: PS3, Line 218: As I told in the previous comment, _gnu_ctx is replaced into std. https://gerrit.cloudera.org/#/c/7414/1/be/src/gutil/hash/hash.h@a278 Line 251 You may know std already defined pointer type as below. /// Partial specializations for pointer types. template struct hash<_Tp*> : public __hash_base { size_t operator()(_Tp* __p) const noexcept { return reinterpret_cast(__p); } }; I guess your hash function is more faster than std's implementation, but there are pros and cons. I found the interesting article: https://stackoverflow.com/questions/20953390/what-is-the-fastest-hash-function-for-pointers I have the following issue on this. Please answer it. How to implement a customized hash specialization for pointer type. I removed your code because it cannot be redefined. I think we need to introduce a new template class which extends std::hash if we want to keep own hash. -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Kim Jin Chul has uploaded a new patch set (#3). Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. IMPALA-5116: Remove deprecated hash_* types in gutil The following class templates are substituted from C++11 standard. __gnu_cxx::hash_map => std::unordered_map __gnu_cxx::hash_set => std::unordered_set __gnu_cxx::hash => std::hash Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 --- M be/src/gutil/hash/hash.cc M be/src/gutil/hash/hash.h M be/src/gutil/strings/join.h M be/src/gutil/strings/serialize.cc M be/src/gutil/strings/serialize.h M be/src/gutil/strings/split.cc M be/src/gutil/strings/split.h 7 files changed, 37 insertions(+), 121 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/7414/3 -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Kim Jin Chul has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/7414/1/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: Line 222 > OK, so it looks like you've added it back in. Before we can decide if it sh I guess the reason is that __gnu_cxx::hash<> was not used anywhere. I didn't find any use case for this. Would you please double check? Line 278 > So, do you need it to still exist, and if so, how do you make sure std::has I got it. My idea is that some specializations in __gnu_cxx should be moved to std namespace. I can be sure this will not create a potential problem. Do you agree on this? Or any suggestion? -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Jim Apple has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/7414/1/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: Line 222 > You're right. std::hash template class is specialized between 1 byte and 8 OK, so it looks like you've added it back in. Before we can decide if it should stay or not, let'd understand why it didn't break the build before. Line 278 > You're right. It should make wrong result or corruption. So, do you need it to still exist, and if so, how do you make sure std::hash, which may not be __gnu_cxx::hash, is specialized? -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Kim Jin Chul has posted comments on this change. Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. Patch Set 2: (7 comments) Thanks for your kind review. Would you please review again? http://gerrit.cloudera.org:8080/#/c/7414/1//COMMIT_MSG Commit Message: PS1, Line 7: Remove deprecated hash_* types in gutil > "Remove deprecated hash_* types in gutil" Done PS1, Line 9: tut > from Done PS1, Line 9: class tem > class templates Done http://gerrit.cloudera.org:8080/#/c/7414/1/be/src/gutil/hash/hash.h File be/src/gutil/hash/hash.h: Line 222 > I don't think gcc ships with a std::hash overload for uint128. Why doesn't You're right. std::hash template class is specialized between 1 byte and 8 bytes. I agree this is necessary for unsigned 16 bytes. Line 278 > http://en.cppreference.com/w/cpp/utility/hash : You're right. It should make wrong result or corruption. http://gerrit.cloudera.org:8080/#/c/7414/1/be/src/gutil/strings/split.h File be/src/gutil/strings/split.h: Line 135 > Has this fact about container-ism changed? You're right. It's my mistake. pair cannot be a container. Line 132: // multimap, unordered_set and unordered_map, and even std::pair which is not > Fix early line break Done -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5116: Remove deprecated hash * types in gutil
Kim Jin Chul has uploaded a new patch set (#2). Change subject: IMPALA-5116: Remove deprecated hash_* types in gutil .. IMPALA-5116: Remove deprecated hash_* types in gutil The following class templates are substituted from C++11 standard. __gnu_cxx::hash_map => std::unordered_map __gnu_cxx::hash_set => std::unordered_set __gnu_cxx::hash => std::hash Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 --- M be/src/gutil/hash/hash.cc M be/src/gutil/hash/hash.h M be/src/gutil/strings/join.h M be/src/gutil/strings/serialize.cc M be/src/gutil/strings/serialize.h M be/src/gutil/strings/split.cc M be/src/gutil/strings/split.h 7 files changed, 35 insertions(+), 106 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/7414/2 -- To view, visit http://gerrit.cloudera.org:8080/7414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I06af30fd15acd43a60e3c30af54056c96520b6e9 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Jim Apple