Skip to content

Remove GPR TLS API#26960

Closed
tamird wants to merge 1 commit intogrpc:masterfrom
tamird:remove-gpr-tls
Closed

Remove GPR TLS API#26960
tamird wants to merge 1 commit intogrpc:masterfrom
tamird:remove-gpr-tls

Conversation

@tamird
Copy link
Copy Markdown
Contributor

@tamird tamird commented Aug 10, 2021

Use c++11 thread_local everywhere. Use proper types everywhere.

An alternative to #26942.

@veblush

@tamird tamird mentioned this pull request Aug 10, 2021
Merged
@tamird
Remove GPR TLS API
b5e32a4 
Use c++11 thread_local everywhere. Use proper types everywhere.
@veblush
Copy link
Copy Markdown
Contributor

veblush commented Aug 10, 2021

I believe it's not ready to replace all TLS variants with c++11 thread_local yet. (ref: #24572)

@tamird
Copy link
Copy Markdown
Contributor Author

tamird commented Aug 10, 2021

Yep, see discussion in #26942 (comment).

Is there test coverage for that simulator version? It seems that #26912 introduced "naked" thread_local usage but still passed tests.

@veblush veblush added the release notes: no Indicates if PR should not be in release notes label Aug 10, 2021
@veblush
Copy link
Copy Markdown
Contributor

veblush commented Aug 10, 2021

I believe there is but not sure CI tests for the PR cover it. (There are more tests running on master branch only)

@tamird
Copy link
Copy Markdown
Contributor Author

tamird commented Aug 10, 2021

Where can I see the results of those tests?

@tamird
Copy link
Copy Markdown
Contributor Author

tamird commented Aug 10, 2021

Oh, I didn't realize #26912 hasn't landed yet.

@ctiller
Copy link
Copy Markdown
Member

ctiller commented Aug 10, 2021

#26912 isn't landed... would like to sort this out beforehand :)

@tamird
Copy link
Copy Markdown
Contributor Author

tamird commented Aug 10, 2021 •
edited
Loading

Indeed, no dice. https://source.cloud.google.com/results/invocations/3a2b47bb-10f0-4b1b-906b-a463006a21fd/log

Tue Aug 10 10:29:55 PDT 2021 - /Volumes/BuildData/tmpfs/src/github/grpc/workspace_objc_macos_opt_native/src/core/lib/iomgr/exec_ctx.h:249:10: error: thread-local storage is not supported for the current target
Tue Aug 10 10:29:55 PDT 2021 -   static thread_local ExecCtx* exec_ctx_;
Tue Aug 10 10:29:55 PDT 2021 -          ^
Tue Aug 10 10:29:55 PDT 2021 - /Volumes/BuildData/tmpfs/src/github/grpc/workspace_objc_macos_opt_native/src/core/lib/iomgr/exec_ctx.h:370:10: error: thread-local storage is not supported for the current target
Tue Aug 10 10:29:55 PDT 2021 -   static thread_local ApplicationCallbackExecCtx* callback_exec_ctx_;
Tue Aug 10 10:29:55 PDT 2021 -          ^

@tamird tamird closed this Aug 10, 2021
@tamird tamird deleted the remove-gpr-tls branch August 10, 2021 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

release notes: no Indicates if PR should not be in release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tamird @veblush @ctiller @grpc-kokoro