Skip to content

Move TLS out of header file to avoid bazel incompatibility on Mac#13916

Closed
vjpai wants to merge 1 commit intogrpc:masterfrom
vjpai:tls-bazel
Closed

Move TLS out of header file to avoid bazel incompatibility on Mac#13916
vjpai wants to merge 1 commit intogrpc:masterfrom
vjpai:tls-bazel

Conversation

@vjpai
Copy link
Copy Markdown
Contributor

@vjpai vjpai commented Jan 4, 2018

Fixes #13856

@vjpai vjpai requested a review from yashykt January 4, 2018 18:30
@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Jan 4, 2018 

****************************************************************

libgrpc.so

     VM SIZE                                                                                            FILE SIZE
 ++++++++++++++ GROWING                                                                              ++++++++++++++
  +7.3%     +64 src/core/lib/iomgr/exec_ctx.cc                                                           +64  +7.3%
      [NEW]     +28 grpc_core::ExecCtx::Get                                                                  +28  [NEW]
       +54%     +25 [Unmapped]                                                                               +25   +54%
      [NEW]     +25 grpc_core::ExecCtx::Set                                                                  +25  [NEW]
      [NEW]      +2 grpc_core::ExecCtx::GlobalShutdown                                                        +2  [NEW]

 -------------- SHRINKING                                                                            --------------
  -0.9%   -1017 [Other]                                                                                -1017  -0.9%
  -1.6%    -512 src/core/ext/transport/chttp2/transport/chttp2_transport.cc                             -512  -1.6%
     -21.1%    -308 grpc_chttp2_initiate_write                                                              -308 -21.1%
      -2.7%     -96 perform_stream_op_locked                                                                 -96  -2.7%
      -8.9%     -64 write_action_begin_locked                                                                -64  -8.9%
      -0.5%     -16 grpc_create_chttp2_transport                                                             -16  -0.5%
     -17.5%     -11 start_keepalive_ping_locked                                                              -11 -17.5%
      -9.4%     -11 finish_keepalive_ping_locked                                                             -11  -9.4%
      -3.0%      -8 init_keepalive_ping_locked                                                                -8  -3.0%
  -9.1%    -435 src/core/lib/debug/stats_data.cc                                                        -435  -9.1%
     -88.2%     -60 [Unmapped]                                                                               -60 -88.2%
     -11.9%     -43 grpc_stats_inc_poll_events_returned                                                      -43 -11.9%
      -9.6%     -35 grpc_stats_inc_server_cqs_checked                                                        -35  -9.6%
      -7.4%     -27 grpc_stats_inc_tcp_read_size                                                             -27  -7.4%
      -7.4%     -27 grpc_stats_inc_tcp_read_offer                                                            -27  -7.4%
      -7.4%     -27 grpc_stats_inc_tcp_write_size                                                            -27  -7.4%
      -7.9%     -27 grpc_stats_inc_call_initial_size                                                         -27  -7.9%
      -7.4%     -27 grpc_stats_inc_tcp_write_iov_size                                                        -27  -7.4%
      -7.4%     -27 grpc_stats_inc_http2_send_message_size                                                   -27  -7.4%
      -7.4%     -27 grpc_stats_inc_tcp_read_offer_iov_size                                                   -27  -7.4%
      -7.4%     -27 grpc_stats_inc_http2_send_flowctl_per_write                                              -27  -7.4%
      -7.4%     -27 grpc_stats_inc_http2_send_message_per_write                                              -27  -7.4%
      -7.4%     -27 grpc_stats_inc_http2_send_initial_metadata_per_write                                     -27  -7.4%
      -7.4%     -27 grpc_stats_inc_http2_send_trailing_metadata_per_write                                    -27  -7.4%
  -3.0%    -288 src/core/lib/iomgr/ev_epoll1_linux.cc                                                   -288  -3.0%
      [DEL]    -597 check_neighborhood_for_available_poller(pollset_neighborhood*) [clone .isra.7]          -597  [DEL]
      -7.0%    -166 pollset_kick                                                                            -166  -7.0%
      -2.2%     -74 pollset_work                                                                             -74  -2.2%
      -5.8%     -16 [Unmapped]                                                                               -16  -5.8%
      -3.1%     -16 pollset_shutdown                                                                         -16  -3.1%
  -2.7%    -227 src/core/lib/surface/completion_queue.cc                                                -227  -2.7%
      -2.8%     -32 cq_next                                                                                  -32  -2.8%
     -50.0%     -28 ExecCtxNext::~ExecCtxNext                                                                -28 -50.0%
     -50.0%     -28 ExecCtxPluck::~ExecCtxPluck                                                              -28 -50.0%
      -1.8%     -25 cq_pluck                                                                                 -25  -1.8%
      -6.2%     -17 grpc_completion_queue_shutdown                                                           -17  -6.2%
      -4.1%     -16 non_polling_poller_work                                                                  -16  -4.1%
      -3.7%     -16 grpc_completion_queue_thread_local_cache_flush                                           -16  -3.7%
      -2.9%     -13 cq_event_queue_pop                                                                       -13  -2.9%
     -22.6%     -12 ExecCtxNext::~ExecCtxNext                                                                -12 -22.6%
     -22.6%     -12 ExecCtxPluck::~ExecCtxPluck                                                              -12 -22.6%
      -6.9%     -11 ExecCtxNext::CheckReadyToFinish                                                          -11  -6.9%
      -2.7%      -8 ExecCtxPluck::CheckReadyToFinish                                                          -8  -2.7%
      -3.0%      -8 grpc_completion_queue_destroy                                                             -8  -3.0%
      -1.6%      -5 [Unmapped]                                                                                -5  -1.6%
  -1.6%    -176 src/core/lib/iomgr/ev_epollex_linux.cc                                                  -176  -1.6%
      [DEL]    -193 pollset_kick_all(grpc_pollset*) [clone .isra.10]                                        -193  [DEL]
      -2.4%     -47 pollset_work                                                                             -47  -2.4%
     -10.4%     -45 pollset_kick                                                                             -45 -10.4%
      -7.9%     -45 kick_one_worker                                                                          -45  -7.9%
      -7.2%     -23 [Unmapped]                                                                               -23  -7.2%
      -9.3%      -8 poll_deadline_to_millis_timeout                                                           -8  -9.3%
  -5.5%    -160 src/core/lib/iomgr/executor.cc                                                          -160  -5.5%
      [DEL]    -166 run_closures(grpc_closure_list) [clone .isra.4]                                         -166  [DEL]
      -6.1%     -80 executor_push                                                                            -80  -6.1%
      -9.5%     -64 executor_thread                                                                          -64  -9.5%
      -6.1%      -5 [Unmapped]                                                                                -5  -6.1%
  -1.3%    -144 src/core/ext/transport/chttp2/transport/hpack_parser.cc                                 -144  -1.3%
      -7.3%     -38 begin_parse_string                                                                       -38  -7.3%
      -8.2%     -32 [Unmapped]                                                                               -32  -8.2%
      -1.6%     -16 append_string                                                                            -16  -1.6%
      -3.2%     -14 finish_lithdr_incidx                                                                     -14  -3.2%
      -3.2%     -14 finish_lithdr_notidx                                                                     -14  -3.2%
      -3.2%     -14 finish_lithdr_nvridx                                                                     -14  -3.2%
      -1.7%      -6 finish_lithdr_incidx_v                                                                    -6  -1.7%
      -1.8%      -6 finish_lithdr_notidx_v                                                                    -6  -1.8%
      -1.8%      -6 finish_lithdr_nvridx_v                                                                    -6  -1.8%
  -2.6%    -144 src/core/lib/surface/channel.cc                                                         -144  -2.6%
      -2.1%     -32 grpc_channel_create_with_builder                                                         -32  -2.1%
      -4.8%     -32 grpc_channel_register_call                                                               -32  -4.8%
      -4.0%     -24 grpc_channel_create_call                                                                 -24  -4.0%
      -3.5%     -17 grpc_channel_create_registered_call                                                      -17  -3.5%
      -4.5%     -17 grpc_channel_destroy                                                                     -17  -4.5%
     -11.1%     -14 [Unmapped]                                                                               -14 -11.1%
      -3.5%      -8 grpc_channel_get_info                                                                     -8  -3.5%
  -1.4%    -112 src/core/ext/transport/chttp2/transport/hpack_encoder.cc                                -112  -1.4%
     -17.1%     -59 get_wire_value                                                                           -59 -17.1%
      -2.7%     -16 grpc_chttp2_encode_header                                                                -16  -2.7%
      -3.4%     -16 emit_lithdr_incidx                                                                       -16  -3.4%
      -6.7%     -11 emit_indexed(grpc_chttp2_hpack_compressor*, unsigned int, framer_state*) [clone .isr     -11  -6.7%
      -7.0%     -10 [Unmapped]                                                                               -10  -7.0%
  -0.7%    -112 src/core/lib/surface/call.cc                                                            -112  -0.7%
      -7.0%     -40 grpc_call_unref                                                                          -40  -7.0%
      -7.4%     -30 grpc_call_start_batch                                                                    -30  -7.4%
      -0.8%     -16 grpc_call_create                                                                         -16  -0.8%
      -3.9%     -15 grpc_call_cancel_with_status                                                             -15  -3.9%
      -2.4%      -8 grpc_call_cancel                                                                          -8  -2.4%
      -0.8%      -3 [Unmapped]                                                                                -3  -0.8%
  -0.8%    -112 src/core/lib/surface/server.cc                                                          -112  -0.8%
      -4.5%     -24 grpc_server_destroy                                                                      -24  -4.5%
      -3.1%     -22 grpc_server_request_registered_call                                                      -22  -3.1%
      -4.3%     -17 grpc_server_cancel_all_calls                                                             -17  -4.3%
      -2.8%     -16 publish_new_rpc                                                                          -16  -2.8%
      -2.6%     -16 grpc_server_request_call                                                                 -16  -2.6%
      -0.9%      -8 grpc_server_shutdown_and_notify                                                           -8  -0.9%
      -1.5%      -8 grpc_server_start                                                                         -8  -1.5%
      -0.3%      -1 [Unmapped]                                                                                -1  -0.3%
  -3.1%     -80 src/core/lib/iomgr/combiner.cc                                                           -80  -3.1%
      -7.9%     -80 grpc_combiner_continue_exec_ctx                                                          -80  -7.9%
     -97.0%     -64 offload                                                                                  -64 -97.0%
     -12.8%     -50 combiner_exec                                                                            -50 -12.8%
      -8.8%     -32 combiner_finally_exec                                                                    -32  -8.8%
     -12.4%     -21 queue_offload                                                                            -21 -12.4%
  -5.4%     -70 src/core/lib/surface/init.cc                                                             -70  -5.4%
     -75.0%     -42 grpc_core::ExecCtx::~ExecCtx                                                             -42 -75.0%
     -22.6%     -12 grpc_core::ExecCtx::~ExecCtx                                                             -12 -22.6%
     -17.6%      -9 [Unmapped]                                                                                -9 -17.6%
      -1.7%      -7 grpc_shutdown                                                                             -7  -1.7%
  -4.3%     -64 src/core/ext/filters/client_channel/subchannel_index.cc                                  -64  -4.3%
      -7.6%     -33 grpc_subchannel_index_register                                                           -33  -7.6%
     -18.0%     -20 [Unmapped]                                                                               -20 -18.0%
     -12.7%     -18 grpc_subchannel_index_find                                                               -18 -12.7%
     -12.8%     -11 grpc_subchannel_index_unref                                                              -11 -12.8%
  -1.1%     -64 src/core/ext/transport/chttp2/transport/writing.cc                                       -64  -1.1%
      -1.3%     -68 grpc_chttp2_begin_write                                                                  -68  -1.3%
  -0.5%     -64 src/core/lib/iomgr/ev_poll_posix.cc                                                      -64  -0.5%
      [DEL]    -106 pollset_kick_locked(grpc_fd_watcher*) [clone .isra.12]                                  -106  [DEL]
      -1.7%     -48 pollset_work                                                                             -48  -1.7%
      -2.3%     -15 pollset_kick_ext                                                                         -15  -2.3%
      -0.3%      -1 [Unmapped]                                                                                -1  -0.3%
  -2.6%     -64 src/core/lib/iomgr/timer_manager.cc                                                      -64  -2.6%
      -4.1%     -48 timer_thread                                                                             -48  -4.1%
     -10.7%      -8 [Unmapped]                                                                                -8 -10.7%
      -3.6%      -8 grpc_timer_manager_tick                                                                   -8  -3.6%
  -1.9%     -64 src/core/lib/security/credentials/google_default/google_default_credentials.cc           -64  -1.9%
      -2.5%     -40 grpc_google_default_credentials_create                                                   -40  -2.5%
      -5.9%     -17 grpc_flush_cached_google_default_credentials                                             -17  -5.9%
     -10.4%      -7 [Unmapped]                                                                                -7 -10.4%
  -0.3%     -48 src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc                           -48  -0.3%
      -4.0%     -16 maybe_restart_lb_call                                                                    -16  -4.0%
      -6.0%     -11 start_picking_locked                                                                     -11  -6.0%
     -10.2%     -11 schedule_next_client_load_report                                                         -11 -10.2%
      -0.6%      -8 query_for_backends_locked(glb_lb_policy*) [clone .part.11]                                -8  -0.6%
      -0.5%      -2 [Unmapped]                                                                                -2  -0.5%
 -12.9%     -48 src/core/lib/backoff/backoff.cc                                                          -48 -12.9%
      -8.4%     -19 grpc_core::BackOff::Step                                                                 -19  -8.4%
     -36.7%     -18 [Unmapped]                                                                               -18 -36.7%
     -29.7%     -11 grpc_core::BackOff::Begin                                                                -11 -29.7%

 -+-+-+-+-+-+-+ MIXED                                                                                +-+-+-+-+-+-+-
  +0.1%    +445 [None]                                                                                -194Ki  -3.3%
      [NEW]     +60 CSWTCH.2                                                                                 +60  [NEW]
      [DEL]     -60 CSWTCH.3                                                                                 -60  [DEL]

  -0.3% -3.41Ki TOTAL                                                                                 -198Ki  -3.1%


****************************************************************

libgrpc++.so

     VM SIZE                                                                                            FILE SIZE
 ++++++++++++++ GROWING                                                                              ++++++++++++++

 -------------- SHRINKING                                                                            --------------
  -0.0%     -90 [None]                                                                               -4.24Ki  -0.1%
      -0.0%     -74 [Unmapped]                                                                           -4.24Ki  -0.1%
     -13.1%     -16 [None]                                                                                     0  [ = ]
  -1.8%    -102 src/cpp/common/channel_arguments.cc                                                     -102  -1.8%
      [DEL]    -194 void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >    -194  [DEL]
     -75.0%     -42 grpc_core::ExecCtx::~ExecCtx                                                             -42 -75.0%
      -6.8%     -24 grpc::ChannelArguments::~ChannelArguments                                                -24  -6.8%
      -2.0%     -16 grpc::ChannelArguments::SetSocketMutator                                                 -16  -2.0%
     -22.6%     -12 grpc_core::ExecCtx::~ExecCtx                                                             -12 -22.6%
      -5.9%      -8 [Unmapped]                                                                                -8  -5.9%

  -0.1%    -192 TOTAL                                                                                -4.34Ki  -0.1%

@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Jan 4, 2018 

[trickle] No significant performance differences

vjpai
vjpai commented Jan 4, 2018
View reviewed changes
src/core/lib/iomgr/exec_ctx.cc
namespace grpc_core {
GPR_TLS_CLASS_DEF(ExecCtx::exec_ctx_);
namespace {
GPR_TLS_DECL(g_exec_ctx);
Copy link
Copy Markdown
Contributor Author

@vjpai vjpai Jan 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know that at the current time, this is doubling-down since GPR_TLS_DECL already gives it static linkage but I thought that I should wrap it in an anonymous namespace to make the intention more clear.

@vjpai vjpai mentioned this pull request Jan 4, 2018
Closed
@grpc-testing
Copy link
Copy Markdown

grpc-testing commented Jan 4, 2018 

[microbenchmarks] Performance differences noted:
Benchmark                       cpu_time    real_time
------------------------------  ----------  -----------
BM_ClosureReschedOnCombiner     +6%         +6%
BM_ClosureSched2OnCombiner      +6%         +6%
BM_ClosureSched2OnTwoCombiners  +4%         +4%
BM_ClosureSched3OnCombiner      +8%         +8%
BM_ClosureSched4OnTwoCombiners  +7%         +7%
BM_ClosureSchedOnExecCtx        +7%         +7%
BM_NoOpExecCtx                  +4%         +4%
BM_WellFlushed                  +13%        +13%

yashykt
yashykt approved these changes Jan 4, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@yashykt yashykt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
We might want to revert this once the bazel issue is fixed though.

@vjpai
Copy link
Copy Markdown
Contributor Author

vjpai commented Jan 5, 2018

@yashykt : I agree that we might want to revert this. Given that this has hurt some microbenchmark numbers, I am tempted to say that we shouldn't merge it either. I can craft another one that only affects Mac.

@vjpai vjpai mentioned this pull request Jan 5, 2018
Merged
@vjpai
Copy link
Copy Markdown
Contributor Author

vjpai commented Jan 5, 2018

Closing this since #13929 merged instead

@vjpai vjpai closed this Jan 5, 2018
@yashykt yashykt mentioned this pull request Mar 14, 2018
Merged
@vjpai vjpai deleted the tls-bazel branch August 13, 2018 18:03
@lock lock bot locked as resolved and limited conversation to collaborators Nov 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@yashykt yashykt yashykt approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vjpai @grpc-testing @yashykt