You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2018/02/15 00:24:31 UTC
[kudu-CR] gutil: properly hook up ANNOTATE HAPPENS BEFORE/AFTER
Hello Will Berkeley, Mike Percy,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/9325
to review the following change.
Change subject: gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER
......................................................................
gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER
This updates dynamic_annotations to hook up ANNOTATE_HAPPENS_BEFORE and
ANNOTATE_HAPPENS_AFTER to the appropriate annotations in the TSAN
runtime library. According to [1] (committed in 2011) the functions have
been implemented in TSAN for quite some time, and it was an error that
we weren't using them.
The previous implementation of the function called some
condition-variable annotations in the TSAN runtime library instead.
Those functions actually turn out to be implemented as no-ops. So, I
looked for existing call sites to ANNOTATE_HAPPENS_BEFORE and AFTER and
removed them. This cleaned up atomic_refcount pretty substantially.
Again I found a matching change[2] in Chromium, from which this code is
derived.
[1] https://codereview.chromium.org/6982022
[2] https://codereview.chromium.org/580813002
Change-Id: Ida27aff6b9771c0009fba5e31ec7a0c7c53caa59
---
M src/kudu/gutil/atomic_refcount.h
M src/kudu/gutil/dynamic_annotations.c
M src/kudu/gutil/dynamic_annotations.h
M src/kudu/gutil/once.cc
M src/kudu/gutil/once.h
5 files changed, 20 insertions(+), 57 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/9325/1
--
To view, visit http://gerrit.cloudera.org:8080/9325
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ida27aff6b9771c0009fba5e31ec7a0c7c53caa59
Gerrit-Change-Number: 9325
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] gutil: properly hook up ANNOTATE HAPPENS BEFORE/AFTER
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9325 )
Change subject: gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9325
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ida27aff6b9771c0009fba5e31ec7a0c7c53caa59
Gerrit-Change-Number: 9325
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 21 Feb 2018 01:22:02 +0000
Gerrit-HasComments: No
[kudu-CR] gutil: properly hook up ANNOTATE HAPPENS BEFORE/AFTER
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9325 )
Change subject: gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER
......................................................................
gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER
This updates dynamic_annotations to hook up ANNOTATE_HAPPENS_BEFORE and
ANNOTATE_HAPPENS_AFTER to the appropriate annotations in the TSAN
runtime library. According to [1] (committed in 2011) the functions have
been implemented in TSAN for quite some time, and it was an error that
we weren't using them.
The previous implementation of the function called some
condition-variable annotations in the TSAN runtime library instead.
Those functions actually turn out to be implemented as no-ops. So, I
looked for existing call sites to ANNOTATE_HAPPENS_BEFORE and AFTER and
removed them. This cleaned up atomic_refcount pretty substantially.
Again I found a matching change[2] in Chromium, from which this code is
derived.
[1] https://codereview.chromium.org/6982022
[2] https://codereview.chromium.org/580813002
Change-Id: Ida27aff6b9771c0009fba5e31ec7a0c7c53caa59
Reviewed-on: http://gerrit.cloudera.org:8080/9325
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M src/kudu/gutil/atomic_refcount.h
M src/kudu/gutil/dynamic_annotations.c
M src/kudu/gutil/dynamic_annotations.h
M src/kudu/gutil/once.cc
M src/kudu/gutil/once.h
5 files changed, 22 insertions(+), 59 deletions(-)
Approvals:
Kudu Jenkins: Verified
Mike Percy: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/9325
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ida27aff6b9771c0009fba5e31ec7a0c7c53caa59
Gerrit-Change-Number: 9325
Gerrit-PatchSet: 4
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] gutil: properly hook up ANNOTATE HAPPENS BEFORE/AFTER
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9325
to look at the new patch set (#2).
Change subject: gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER
......................................................................
gutil: properly hook up ANNOTATE_HAPPENS_BEFORE/AFTER
This updates dynamic_annotations to hook up ANNOTATE_HAPPENS_BEFORE and
ANNOTATE_HAPPENS_AFTER to the appropriate annotations in the TSAN
runtime library. According to [1] (committed in 2011) the functions have
been implemented in TSAN for quite some time, and it was an error that
we weren't using them.
The previous implementation of the function called some
condition-variable annotations in the TSAN runtime library instead.
Those functions actually turn out to be implemented as no-ops. So, I
looked for existing call sites to ANNOTATE_HAPPENS_BEFORE and AFTER and
removed them. This cleaned up atomic_refcount pretty substantially.
Again I found a matching change[2] in Chromium, from which this code is
derived.
[1] https://codereview.chromium.org/6982022
[2] https://codereview.chromium.org/580813002
Change-Id: Ida27aff6b9771c0009fba5e31ec7a0c7c53caa59
---
M src/kudu/gutil/atomic_refcount.h
M src/kudu/gutil/dynamic_annotations.c
M src/kudu/gutil/dynamic_annotations.h
M src/kudu/gutil/once.cc
M src/kudu/gutil/once.h
5 files changed, 22 insertions(+), 59 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/9325/2
--
To view, visit http://gerrit.cloudera.org:8080/9325
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ida27aff6b9771c0009fba5e31ec7a0c7c53caa59
Gerrit-Change-Number: 9325
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>