From bbc0b775e137e6865c9213086c61b84e7415a9ec Mon Sep 17 00:00:00 2001
From: David Garcia Quintas <dgq@google.com>
Date: Wed, 29 Apr 2015 14:10:05 -0700
Subject: [PATCH] Merge comments. See below.
- Added ptag ignore annotation threshold
- prefixed everythig with grpc_
- removed spurious annotations in channel_create.c and client_channel.c
- removed stap_probes.h (it's generated from stap_probes.d by make)
- Updated Makefile and its template with the right way to generate stap
probe headers from its .d definition.
---
Makefile | 3 +-
src/core/channel/client_channel.c | 4 ---
src/core/iomgr/pollset_posix.c | 2 +-
src/core/iomgr/tcp_posix.c | 20 ++++++------
src/core/profiling/basic_timers.c | 4 ++-
src/core/profiling/stap_probes.h | 43 -------------------------
src/core/profiling/timers.h | 53 ++++++++++++++++++-------------
src/core/surface/channel_create.c | 8 ++---
src/cpp/client/channel.cc | 6 ++--
src/cpp/common/call.cc | 8 ++---
src/cpp/server/server.cc | 8 ++---
templates/Makefile.template | 3 +-
12 files changed, 62 insertions(+), 100 deletions(-)
delete mode 100644 src/core/profiling/stap_probes.h
diff --git a/Makefile b/Makefile
index d0a1284eb0..1b6c44f737 100644
--- a/Makefile
+++ b/Makefile
@@ -2814,7 +2814,8 @@ $(GENDIR)/test/proto/test.grpc.pb.cc: test/proto/test.proto $(PROTOBUF_DEP) $(PR
endif
-ifeq ($(CONFIG), stapprof)
+ifeq ($(CONFIG),stapprof)
+src/core/profiling/stap_timers.c: $(GENDIR)/src/core/profiling/stap_probes.h
ifeq ($(HAS_SYSTEMTAP),true)
$(GENDIR)/src/core/profiling/stap_probes.h: src/core/profiling/stap_probes.d
$(E) "[DTRACE] Compiling $<"
diff --git a/src/core/channel/client_channel.c b/src/core/channel/client_channel.c
index 7be89e5349..bc481e59ca 100644
--- a/src/core/channel/client_channel.c
+++ b/src/core/channel/client_channel.c
@@ -39,9 +39,7 @@
#include "src/core/channel/child_channel.h"
#include "src/core/channel/connected_channel.h"
#include "src/core/iomgr/iomgr.h"
-#include "src/core/profiling/timers.h"
#include "src/core/support/string.h"
-
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/sync.h>
@@ -129,7 +127,6 @@ static void complete_activate(grpc_call_element *elem, grpc_call_op *op) {
static void start_rpc(grpc_call_element *elem, grpc_call_op *op) {
call_data *calld = elem->call_data;
channel_data *chand = elem->channel_data;
- GRPC_TIMER_BEGIN(PTAG_OTHER_BASE + 1, NULL);
gpr_mu_lock(&chand->mu);
if (calld->state == CALL_CANCELLED) {
gpr_mu_unlock(&chand->mu);
@@ -175,7 +172,6 @@ static void start_rpc(grpc_call_element *elem, grpc_call_op *op) {
grpc_transport_setup_initiate(chand->transport_setup);
}
}
- GRPC_TIMER_BEGIN(PTAG_OTHER_BASE + 1, NULL);
}
static void remove_waiting_child(channel_data *chand, call_data *calld) {
diff --git a/src/core/iomgr/pollset_posix.c b/src/core/iomgr/pollset_posix.c
index 4985dcf739..4d1bcad9e2 100644
--- a/src/core/iomgr/pollset_posix.c
+++ b/src/core/iomgr/pollset_posix.c
@@ -411,7 +411,7 @@ static int unary_poll_pollset_maybe_work(grpc_pollset *pollset,
pfd[1].events = grpc_fd_begin_poll(fd, pollset, POLLIN, POLLOUT, &fd_watcher);
r = poll(pfd, GPR_ARRAY_SIZE(pfd), timeout);
- GRPC_TIMER_MARK(PTAG_POLL_FINISHED, r);
+ GRPC_TIMER_MARK(GRPC_PTAG_POLL_FINISHED, r);
grpc_fd_end_poll(&fd_watcher);
diff --git a/src/core/iomgr/tcp_posix.c b/src/core/iomgr/tcp_posix.c
index 7626a51184..40897fb8f8 100644
--- a/src/core/iomgr/tcp_posix.c
+++ b/src/core/iomgr/tcp_posix.c
@@ -327,7 +327,7 @@ static void grpc_tcp_handle_read(void *arg /* grpc_tcp */, int success) {
gpr_slice *final_slices;
size_t final_nslices;
- GRPC_TIMER_BEGIN(PTAG_HANDLE_READ, 0);
+ GRPC_TIMER_BEGIN(GRPC_PTAG_HANDLE_READ, 0);
slice_state_init(&read_state, static_read_slices, INLINE_SLICE_BUFFER_SIZE,
0);
@@ -350,11 +350,11 @@ static void grpc_tcp_handle_read(void *arg /* grpc_tcp */, int success) {
msg.msg_controllen = 0;
msg.msg_flags = 0;
- GRPC_TIMER_BEGIN(PTAG_RECVMSG, 0);
+ GRPC_TIMER_BEGIN(GRPC_PTAG_RECVMSG, 0);
do {
read_bytes = recvmsg(tcp->fd, &msg, 0);
} while (read_bytes < 0 && errno == EINTR);
- GRPC_TIMER_END(PTAG_RECVMSG, 0);
+ GRPC_TIMER_END(GRPC_PTAG_RECVMSG, 0);
if (read_bytes < allocated_bytes) {
/* TODO(klempner): Consider a second read first, in hopes of getting a
@@ -406,7 +406,7 @@ static void grpc_tcp_handle_read(void *arg /* grpc_tcp */, int success) {
++iov_size;
}
}
- GRPC_TIMER_END(PTAG_HANDLE_READ, 0);
+ GRPC_TIMER_END(GRPC_PTAG_HANDLE_READ, 0);
}
static void grpc_tcp_notify_on_read(grpc_endpoint *ep, grpc_endpoint_read_cb cb,
@@ -438,12 +438,12 @@ static grpc_endpoint_write_status grpc_tcp_flush(grpc_tcp *tcp) {
msg.msg_controllen = 0;
msg.msg_flags = 0;
- GRPC_TIMER_BEGIN(PTAG_SENDMSG, 0);
+ GRPC_TIMER_BEGIN(GRPC_PTAG_SENDMSG, 0);
do {
/* TODO(klempner): Cork if this is a partial write */
sent_length = sendmsg(tcp->fd, &msg, 0);
} while (sent_length < 0 && errno == EINTR);
- GRPC_TIMER_END(PTAG_SENDMSG, 0);
+ GRPC_TIMER_END(GRPC_PTAG_SENDMSG, 0);
if (sent_length < 0) {
if (errno == EAGAIN) {
@@ -479,7 +479,7 @@ static void grpc_tcp_handle_write(void *arg /* grpc_tcp */, int success) {
return;
}
- GRPC_TIMER_BEGIN(PTAG_TCP_CB_WRITE, 0);
+ GRPC_TIMER_BEGIN(GRPC_PTAG_TCP_CB_WRITE, 0);
write_status = grpc_tcp_flush(tcp);
if (write_status == GRPC_ENDPOINT_WRITE_PENDING) {
grpc_fd_notify_on_write(tcp->em_fd, &tcp->write_closure);
@@ -495,7 +495,7 @@ static void grpc_tcp_handle_write(void *arg /* grpc_tcp */, int success) {
cb(tcp->write_user_data, cb_status);
grpc_tcp_unref(tcp);
}
- GRPC_TIMER_END(PTAG_TCP_CB_WRITE, 0);
+ GRPC_TIMER_END(GRPC_PTAG_TCP_CB_WRITE, 0);
}
static grpc_endpoint_write_status grpc_tcp_write(grpc_endpoint *ep,
@@ -518,7 +518,7 @@ static grpc_endpoint_write_status grpc_tcp_write(grpc_endpoint *ep,
}
}
- GRPC_TIMER_BEGIN(PTAG_TCP_WRITE, 0);
+ GRPC_TIMER_BEGIN(GRPC_PTAG_TCP_WRITE, 0);
GPR_ASSERT(tcp->write_cb == NULL);
slice_state_init(&tcp->write_state, slices, nslices, nslices);
@@ -532,7 +532,7 @@ static grpc_endpoint_write_status grpc_tcp_write(grpc_endpoint *ep,
grpc_fd_notify_on_write(tcp->em_fd, &tcp->write_closure);
}
- GRPC_TIMER_END(PTAG_TCP_WRITE, 0);
+ GRPC_TIMER_END(GRPC_PTAG_TCP_WRITE, 0);
return status;
}
diff --git a/src/core/profiling/basic_timers.c b/src/core/profiling/basic_timers.c
index b33afa7e8c..f290875f78 100644
--- a/src/core/profiling/basic_timers.c
+++ b/src/core/profiling/basic_timers.c
@@ -129,7 +129,9 @@ static void grpc_timers_log_add(grpc_timers_log* log, int tag, void* id,
/* Latency profiler API implementation. */
void grpc_timer_add_mark(int tag, void* id, const char* file, int line) {
- grpc_timers_log_add(grpc_timers_log_global, tag, id, file, line);
+ if (tag <= GRPC_PTAG_IGNORE_THRESHOLD) {
+ grpc_timers_log_add(grpc_timers_log_global, tag, id, file, line);
+ }
}
void grpc_timer_begin(int tag, void* id, const char *file, int line) {}
diff --git a/src/core/profiling/stap_probes.h b/src/core/profiling/stap_probes.h
deleted file mode 100644
index ba8020c5a3..0000000000
--- a/src/core/profiling/stap_probes.h
+++ /dev/null
@@ -1,43 +0,0 @@
-/* Generated by the Systemtap dtrace wrapper */
-
-#define _SDT_HAS_SEMAPHORES 1
-
-
-#define STAP_HAS_SEMAPHORES 1 /* deprecated */
-
-
-#include <sys/sdt.h>
-
-/* _STAP_ADD_MARK ( int tag) */
-#if defined STAP_SDT_V1
-#define _STAP_ADD_MARK_ENABLED() __builtin_expect (add_mark_semaphore, 0)
-#define _stap_add_mark_semaphore add_mark_semaphore
-#else
-#define _STAP_ADD_MARK_ENABLED() __builtin_expect (_stap_add_mark_semaphore, 0)
-#endif
-__extension__ extern unsigned short _stap_add_mark_semaphore __attribute__ ((unused)) __attribute__ ((section (".probes")));
-#define _STAP_ADD_MARK(arg1) \
-DTRACE_PROBE1(_stap,add_mark,arg1)
-
-/* _STAP_TIMING_NS_BEGIN ( int tag) */
-#if defined STAP_SDT_V1
-#define _STAP_TIMING_NS_BEGIN_ENABLED() __builtin_expect (timing_ns_begin_semaphore, 0)
-#define _stap_timing_ns_begin_semaphore timing_ns_begin_semaphore
-#else
-#define _STAP_TIMING_NS_BEGIN_ENABLED() __builtin_expect (_stap_timing_ns_begin_semaphore, 0)
-#endif
-__extension__ extern unsigned short _stap_timing_ns_begin_semaphore __attribute__ ((unused)) __attribute__ ((section (".probes")));
-#define _STAP_TIMING_NS_BEGIN(arg1) \
-DTRACE_PROBE1(_stap,timing_ns_begin,arg1)
-
-/* _STAP_TIMING_NS_END ( int tag) */
-#if defined STAP_SDT_V1
-#define _STAP_TIMING_NS_END_ENABLED() __builtin_expect (timing_ns_end_semaphore, 0)
-#define _stap_timing_ns_end_semaphore timing_ns_end_semaphore
-#else
-#define _STAP_TIMING_NS_END_ENABLED() __builtin_expect (_stap_timing_ns_end_semaphore, 0)
-#endif
-__extension__ extern unsigned short _stap_timing_ns_end_semaphore __attribute__ ((unused)) __attribute__ ((section (".probes")));
-#define _STAP_TIMING_NS_END(arg1) \
-DTRACE_PROBE1(_stap,timing_ns_end,arg1)
-
diff --git a/src/core/profiling/timers.h b/src/core/profiling/timers.h
index a78bbea4ec..d0b8286c03 100644
--- a/src/core/profiling/timers.h
+++ b/src/core/profiling/timers.h
@@ -45,27 +45,30 @@ void grpc_timer_add_mark(int tag, void* id, const char *file, int line);
void grpc_timer_begin(int tag, void* id, const char *file, int line);
void grpc_timer_end(int tag, void* id, const char *file, int line);
-enum profiling_tags {
+enum grpc_profiling_tags {
+ /* Any GRPC_PTAG_* >= than the threshold won't generate any profiling mark. */
+ GRPC_PTAG_IGNORE_THRESHOLD = 1000000,
+
/* Re. Protos. */
- PTAG_PROTO_SERIALIZE = 100,
- PTAG_PROTO_DESERIALIZE = 101,
+ GRPC_PTAG_PROTO_SERIALIZE = 100 + GRPC_PTAG_IGNORE_THRESHOLD,
+ GRPC_PTAG_PROTO_DESERIALIZE = 101 + GRPC_PTAG_IGNORE_THRESHOLD,
/* Re. sockets. */
- PTAG_HANDLE_READ = 200,
- PTAG_SENDMSG = 201,
- PTAG_RECVMSG = 202,
- PTAG_POLL_FINISHED = 203,
- PTAG_TCP_CB_WRITE = 204,
- PTAG_TCP_WRITE = 205,
+ GRPC_PTAG_HANDLE_READ = 200 + GRPC_PTAG_IGNORE_THRESHOLD,
+ GRPC_PTAG_SENDMSG = 201 + GRPC_PTAG_IGNORE_THRESHOLD,
+ GRPC_PTAG_RECVMSG = 202 + GRPC_PTAG_IGNORE_THRESHOLD,
+ GRPC_PTAG_POLL_FINISHED = 203 + GRPC_PTAG_IGNORE_THRESHOLD,
+ GRPC_PTAG_TCP_CB_WRITE = 204 + GRPC_PTAG_IGNORE_THRESHOLD,
+ GRPC_PTAG_TCP_WRITE = 205 + GRPC_PTAG_IGNORE_THRESHOLD,
/* C++ */
- PTAG_CPP_CALL_CREATED = 300,
- PTAG_CPP_PERFORM_OPS = 301,
+ GRPC_PTAG_CPP_CALL_CREATED = 300 + GRPC_PTAG_IGNORE_THRESHOLD,
+ GRPC_PTAG_CPP_PERFORM_OPS = 301 + GRPC_PTAG_IGNORE_THRESHOLD,
/* > 1024 Unassigned reserved. For any miscellaneous use.
- * Use addition to generate tags from this base or take advantage of the 10
- * zero'd bits for OR-ing. */
- PTAG_OTHER_BASE = 1024
+ * Use addition to generate tags from this base or take advantage of the 10
+ * zero'd bits for OR-ing. */
+ GRPC_PTAG_OTHER_BASE = 1024
};
#if !(defined(GRPC_STAP_PROFILER) + defined(GRPC_BASIC_PROFILER))
@@ -86,14 +89,20 @@ enum profiling_tags {
#endif
/* Generic profiling interface. */
-#define GRPC_TIMER_MARK(tag, id) \
- grpc_timer_add_mark(tag, ((void *)(gpr_intptr)(id)), __FILE__, __LINE__)
-
-#define GRPC_TIMER_BEGIN(tag, id) \
- grpc_timer_begin(tag, ((void *)(gpr_intptr)(id)), __FILE__, __LINE__)
-
-#define GRPC_TIMER_END(tag, id) \
- grpc_timer_end(tag, ((void *)(gpr_intptr)(id)), __FILE__, __LINE__)
+#define GRPC_TIMER_MARK(tag, id) \
+ if (tag < GRPC_PTAG_IGNORE_THRESHOLD) { \
+ grpc_timer_add_mark(tag, ((void *)(gpr_intptr)(id)), __FILE__, __LINE__); \
+ }
+
+#define GRPC_TIMER_BEGIN(tag, id) \
+ if (tag < GRPC_PTAG_IGNORE_THRESHOLD) { \
+ grpc_timer_begin(tag, ((void *)(gpr_intptr)(id)), __FILE__, __LINE__); \
+ }
+
+#define GRPC_TIMER_END(tag, id) \
+ if (tag < GRPC_PTAG_IGNORE_THRESHOLD) { \
+ grpc_timer_end(tag, ((void *)(gpr_intptr)(id)), __FILE__, __LINE__); \
+ }
#ifdef GRPC_STAP_PROFILER
/* Empty placeholder for now. */
diff --git a/src/core/surface/channel_create.c b/src/core/surface/channel_create.c
index c44dcce7de..3104b1d00d 100644
--- a/src/core/surface/channel_create.c
+++ b/src/core/surface/channel_create.c
@@ -31,7 +31,6 @@
*
*/
-
#include "src/core/iomgr/sockaddr.h"
#include <grpc/grpc.h>
@@ -49,12 +48,10 @@
#include "src/core/iomgr/endpoint.h"
#include "src/core/iomgr/resolve_address.h"
#include "src/core/iomgr/tcp_client.h"
-#include "src/core/profiling/timers.h"
-#include "src/core/support/string.h"
#include "src/core/surface/channel.h"
#include "src/core/surface/client.h"
+#include "src/core/support/string.h"
#include "src/core/transport/chttp2_transport.h"
-
#include <grpc/support/alloc.h>
#include <grpc/support/log.h>
#include <grpc/support/sync.h>
@@ -198,7 +195,6 @@ grpc_channel *grpc_channel_create(const char *target,
#define MAX_FILTERS 3
const grpc_channel_filter *filters[MAX_FILTERS];
int n = 0;
- GRPC_TIMER_BEGIN(PTAG_OTHER_BASE + 2, NULL);
filters[n++] = &grpc_client_surface_filter;
if (grpc_channel_args_is_census_enabled(args)) {
filters[n++] = &grpc_client_census_filter;
@@ -214,6 +210,6 @@ grpc_channel *grpc_channel_create(const char *target,
grpc_client_setup_create_and_attach(grpc_channel_get_channel_stack(channel),
args, mdctx, initiate_setup, done_setup,
s);
- GRPC_TIMER_END(PTAG_OTHER_BASE + 2, NULL);
+
return channel;
}
diff --git a/src/cpp/client/channel.cc b/src/cpp/client/channel.cc
index 3d9d1307a4..475a20d883 100644
--- a/src/cpp/client/channel.cc
+++ b/src/cpp/client/channel.cc
@@ -70,7 +70,7 @@ Call Channel::CreateCall(const RpcMethod& method, ClientContext* context,
? target_.c_str()
: context->authority().c_str(),
context->raw_deadline());
- GRPC_TIMER_MARK(PTAG_CPP_CALL_CREATED, c_call);
+ GRPC_TIMER_MARK(GRPC_PTAG_CPP_CALL_CREATED, c_call);
context->set_call(c_call, shared_from_this());
return Call(c_call, this, cq);
}
@@ -79,11 +79,11 @@ void Channel::PerformOpsOnCall(CallOpBuffer* buf, Call* call) {
static const size_t MAX_OPS = 8;
size_t nops = MAX_OPS;
grpc_op ops[MAX_OPS];
- GRPC_TIMER_BEGIN(PTAG_CPP_PERFORM_OPS, call->call());
+ GRPC_TIMER_BEGIN(GRPC_PTAG_CPP_PERFORM_OPS, call->call());
buf->FillOps(ops, &nops);
GPR_ASSERT(GRPC_CALL_OK ==
grpc_call_start_batch(call->call(), ops, nops, buf));
- GRPC_TIMER_END(PTAG_CPP_PERFORM_OPS, call->call());
+ GRPC_TIMER_END(GRPC_PTAG_CPP_PERFORM_OPS, call->call());
}
void* Channel::RegisterMethod(const char* method) {
diff --git a/src/cpp/common/call.cc b/src/cpp/common/call.cc
index 41d64276e2..a60282316a 100644
--- a/src/cpp/common/call.cc
+++ b/src/cpp/common/call.cc
@@ -232,13 +232,13 @@ void CallOpBuffer::FillOps(grpc_op* ops, size_t* nops) {
}
if (send_message_ || send_message_buffer_) {
if (send_message_) {
- GRPC_TIMER_BEGIN(PTAG_PROTO_SERIALIZE, 0);
+ GRPC_TIMER_BEGIN(GRPC_PTAG_PROTO_SERIALIZE, 0);
bool success = SerializeProto(*send_message_, &send_buf_);
if (!success) {
abort();
// TODO handle parse failure
}
- GRPC_TIMER_END(PTAG_PROTO_SERIALIZE, 0);
+ GRPC_TIMER_END(GRPC_PTAG_PROTO_SERIALIZE, 0);
} else {
send_buf_ = send_message_buffer_->buffer();
}
@@ -310,10 +310,10 @@ bool CallOpBuffer::FinalizeResult(void** tag, bool* status) {
if (recv_buf_) {
got_message = *status;
if (recv_message_) {
- GRPC_TIMER_BEGIN(PTAG_PROTO_DESERIALIZE, 0);
+ GRPC_TIMER_BEGIN(GRPC_PTAG_PROTO_DESERIALIZE, 0);
*status = *status && DeserializeProto(recv_buf_, recv_message_);
grpc_byte_buffer_destroy(recv_buf_);
- GRPC_TIMER_END(PTAG_PROTO_DESERIALIZE, 0);
+ GRPC_TIMER_END(GRPC_PTAG_PROTO_DESERIALIZE, 0);
} else {
recv_message_buffer_->set_buffer(recv_buf_);
}
diff --git a/src/cpp/server/server.cc b/src/cpp/server/server.cc
index de536ef2cf..34aac4c584 100644
--- a/src/cpp/server/server.cc
+++ b/src/cpp/server/server.cc
@@ -124,12 +124,12 @@ class Server::SyncRequest GRPC_FINAL : public CompletionQueueTag {
std::unique_ptr<grpc::protobuf::Message> req;
std::unique_ptr<grpc::protobuf::Message> res;
if (has_request_payload_) {
- GRPC_TIMER_BEGIN(PTAG_PROTO_DESERIALIZE, call_.call());
+ GRPC_TIMER_BEGIN(GRPC_PTAG_PROTO_DESERIALIZE, call_.call());
req.reset(method_->AllocateRequestProto());
if (!DeserializeProto(request_payload_, req.get())) {
abort(); // for now
}
- GRPC_TIMER_END(PTAG_PROTO_DESERIALIZE, call_.call());
+ GRPC_TIMER_END(GRPC_PTAG_PROTO_DESERIALIZE, call_.call());
}
if (has_response_payload_) {
res.reset(method_->AllocateResponseProto());
@@ -346,9 +346,9 @@ class Server::AsyncRequest GRPC_FINAL : public CompletionQueueTag {
bool orig_status = *status;
if (*status && request_) {
if (payload_) {
- GRPC_TIMER_BEGIN(PTAG_PROTO_DESERIALIZE, call_);
+ GRPC_TIMER_BEGIN(GRPC_PTAG_PROTO_DESERIALIZE, call_);
*status = DeserializeProto(payload_, request_);
- GRPC_TIMER_END(PTAG_PROTO_DESERIALIZE, call_);
+ GRPC_TIMER_END(GRPC_PTAG_PROTO_DESERIALIZE, call_);
} else {
*status = false;
}
diff --git a/templates/Makefile.template b/templates/Makefile.template
index fa1ff2b9bb..4c63fb2ed9 100644
--- a/templates/Makefile.template
+++ b/templates/Makefile.template
@@ -898,7 +898,8 @@ endif
% endfor
-ifeq ($(CONFIG), stapprof)
+ifeq ($(CONFIG),stapprof)
+src/core/profiling/stap_timers.c: $(GENDIR)/src/core/profiling/stap_probes.h
ifeq ($(HAS_SYSTEMTAP),true)
$(GENDIR)/src/core/profiling/stap_probes.h: src/core/profiling/stap_probes.d
$(E) "[DTRACE] Compiling $<"
--
GitLab