Skip to content

introduce usage of thread local storage structure #44

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

Closed
wants to merge 3 commits into from

Conversation

frankeh
Copy link
Contributor

@frankeh frankeh commented Feb 4, 2016

Reworked the gettid patch as discussed #42.

} while (0)

static
void
Copy link
Contributor

Choose a reason for hiding this comment

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

should be on the same line

@MadCoder
Copy link
Contributor

MadCoder commented Feb 4, 2016

Additional comments:
I think you're missing patches, at least in init.c. Look for DISPATCH_USE_DIRECT_TSD in the source, it's likely that you will want to also have DISPATCH_USE_THREAD_LOCAL_STORAGE clauses around them too. e.g. in init.c you have (may be slightly different

#if !DISPATCH_USE_DIRECT_TSD
pthread_key_t dispatch_queue_key;
pthread_key_t dispatch_sema4_key;
...

that should definitely become:

-#if !DISPATCH_USE_DIRECT_TSD
+#if !DISPATCH_USE_DIRECT_TSD && ! DISPATCH_USE_THREAD_LOCAL_STORAGE
pthread_key_t dispatch_queue_key;
pthread_key_t dispatch_sema4_key;
...

Or this:

#if DISPATCH_USE_DIRECT_TSD
const struct dispatch_tsd_indexes_s dispatch_tsd_indexes = {
    .dti_version = 2,
    .dti_queue_index = dispatch_queue_key,
    .dti_voucher_index = dispatch_voucher_key,
    .dti_qos_class_index = dispatch_priority_key,
};
#else // DISPATCH_USE_DIRECT_TSD
#ifndef __LINUX_PORT_HDD__
#error Not implemented on this platform
#endif
#endif // DISPATCH_USE_DIRECT_TSD

looks wrong, I think it's fine for a platform to not have some bits of the introspection that make no sense for the platform. Here I would just remove the #error for everyone (though @das may disagree?).

Overall this is going in the right direction, and I think it will be a tremendous performance improvement on linux.

Also, @das will need to review that one too, he'll probably have comments on top of mine

@frankeh
Copy link
Contributor Author

frankeh commented Feb 9, 2016

@das Is there anything else you would like addressed or you have concerns with
or can we merge

@das
Copy link
Contributor

das commented Feb 9, 2016 via email

@MadCoder
Copy link
Contributor

replaced with #82

@MadCoder MadCoder closed this Jun 16, 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