Skip to content

Conversation

@frankeh
Copy link
Contributor

@frankeh frankeh commented Feb 4, 2016

Reworked the gettid patch as discussed #42.

src/queue.c Outdated
} 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