Todd Lipcon has posted comments on this change.

Change subject: WIP: KUDU-1992. striped64: don't heap-allocate the threadlocal 
hashcode
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6791/1//COMMIT_MSG
Commit Message:

PS1, Line 24: This fix seemed simpler than trying to ensure some specific 
ordering of
            : threadlocal destructors (eg by assigning destructor priorities or 
adding
            : some kind of dependency graph).
> This is reasonable.
thanks for investigating. I was taking a more empirical approach: we haven't 
seen any ASAN crashes related to this until this bug came along, so I'm 
assuming this is the only one :)


http://gerrit.cloudera.org:8080/#/c/6791/1/src/kudu/util/striped64.cc
File src/kudu/util/striped64.cc:

Line 20: #include "kudu/util/striped64.h"
> warning: #includes are not sorted properly [llvm-include-order]
Done


PS1, Line 57: // Avoid zero to allow xorShift rehash
> Now we're also avoiding zero because it's a magic value that indicates that
Done


http://gerrit.cloudera.org:8080/#/c/6791/1/src/kudu/util/striped64.h
File src/kudu/util/striped64.h:

Line 115:   void RetryUpdate(int64_t x, Rehash to_rehash);
> warning: function 'kudu::Striped64::RetryUpdate' has a definition with diff
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/6791
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2d4e5264134c70ced434c3ae8da5866a151464d5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to