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

Thread Safety #75

Closed
wants to merge 2 commits into from
Closed

Thread Safety #75

wants to merge 2 commits into from

Conversation

guyguy333
Copy link

This pull request is related to #74 adding a request / release lock in the cci param interface and so the implementation.
The lock is "unique" for all set/get by simplicity.

Some benchmark results :

Test has been done 5 times and the result is an average:

Test :

for(int i = 0; i < 100000000; i++) {
"param" = i;
}

Results on Linux :

unsigned int : 0.198s
cci_param : 12.98s
cci_param thread safe GCC atomics : 13,58 (+ 4,7%)
cci_param thread safe x86 : 14,66 (+ 12,96%)
cci_param thread safe pthread spin lock : 13,68 (+ 13,11%)

On OSX, x86 is faster than GCC atomics and the overhead is +1.4% (pthread spin lock not available)

Untested on Windows

Guillaume Delbergue added 2 commits May 20, 2016 17:14
Specify GS_SPINLOCK_X86, GS_SPINLOCK_GCC_ATOMICS or
GS_SPINLOCK_PTHREAD_SPINLOCK during build to choose
the spinlock implementation

Signed-off-by: Guillaume Delbergue <guillaume.delbergue@greensocs.com>
Add request / release exclusive methods
These methods use spinlock implementation

Signed-off-by: Guillaume Delbergue <guillaume.delbergue@greensocs.com>
/**
* Request lock on parameter
*/
virtual void request_exclusive() const = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest using acquire_exclusive (vs. request_exclusive) for consistency with the tlm_generic_payload's acquire/release.

@guyguy333
Copy link
Author

@pah @twieman Thanks for your comments, I will rework the patch.
For SpinLock implementation choice, it can be done automatically using CMake scripts compiler / architecture detection for example. I will add that to my CMake POC version of CCI.

@twieman twieman mentioned this pull request Jun 13, 2016
@twieman
Copy link
Contributor

twieman commented Jun 13, 2016

This is starting to become more involved as we dig into it. This is not deemed critical for our first release so we will shelf it for now and revisit it in the future. Closing the PR without merge (issue #74 will remain open).

@twieman twieman closed this Jun 13, 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.

3 participants