Use a shared_ptr and thread_local to store credentials for use #202
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Turns out gcc 4.9.3 doesn't support put_time
Increased the number of reader threads in mutable_static_creds_provider_test to test performance under higher contention.
The performance of shared_ptr atomic loads/stores on gcc appears really bad when compared to clang. Not sure if this is Linux/macOS or just stdlib implementation.
sayantacC
approved these changes
Apr 13, 2018
| #else | ||
| #define update_step(...) | ||
| #endif | ||
| thread_local aws::auth::VersionedCredentials current_creds; |
There was a problem hiding this comment.
I think a short comment about the role of the thread local current creds and how its version is compared against the global version would make it easier to follow.
| update_step(debug_stats.success_); | ||
| return true; | ||
| std::shared_ptr<VersionedCredentials> new_credentials = std::make_shared<VersionedCredentials>(next_version, akid, sk, token); | ||
| std::atomic_store(&creds_, new_credentials); |
There was a problem hiding this comment.
would it be good to add a comment that this is updating the global credentials ?
Added some comments to explain how thread local credentials are updated.
Indicate that version is packaged with its credentials, and using unsigned integers ensure that this will work if the value wraps around.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Switched to use a shared_ptr for credential transfer, and thread_local for normal usage.
This gets rid of a bug that could occur with the previous approach around destruction of the string objects, while preserving performance in most cases.