Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace clock_ticks with std::time #12

Merged
merged 7 commits into from
Jul 25, 2016
Merged

Replace clock_ticks with std::time #12

merged 7 commits into from
Jul 25, 2016

Conversation

serprex
Copy link
Contributor

@serprex serprex commented Jul 25, 2016

Instant is currently grabbed in thread_local init of Library. If it's desirable to share Instant then a write-once read-many global could be used for the epoch

Developing through github, feel free to squash

@llogiq
Copy link
Owner

llogiq commented Jul 25, 2016

Before we merge it, I'd like to see an benchmark where multiple threads stress the time handling – I have a hunch that the thread-local implementation will be faster in that case because it avoids synchronization (which can become a bottleneck).

Note that we are usually interested in the performance of the highly-contended hot areas of the code, so anything that synchronizes them in any way should be avoided.

@serprex
Copy link
Contributor Author

serprex commented Jul 25, 2016

This PR doesn't implement a shared epoch currently. Should I take your desire for a contended epoch benchmark to imply that this PR should implement a shared epoch?

@TyOverby
Copy link
Collaborator

@tomaka's clock ticks library already uses std::time under the hood, so I'll merge this now and the conversation about synchronization can continue.

@llogiq Where are you seeing synchronization?

@TyOverby TyOverby merged commit 34c2ef6 into llogiq:master Jul 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants