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

shared singleton thread #16

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

shared singleton thread #16

wants to merge 4 commits into from

Conversation

daltoniam
Copy link
Collaborator

This for the enhancement for #15. Feedback appreciated.

static JFRThread *manager = nil;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
manager = [[[self class] alloc] init];
Copy link
Contributor

Choose a reason for hiding this comment

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

In a class method, self is already a Class, so this is redundant:
manager = [[[self class] alloc] init];
Better as:
manager = [[self alloc] init];

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nice catch.

@daltoniam daltoniam mentioned this pull request Apr 14, 2015
@adamkaplan
Copy link
Contributor

Note: should not be merged if #25 is accepted

@jtreanor
Copy link
Contributor

Any update on whether this or #25 will be accepted?

@acmacalister
Copy link
Owner

@jtreanor we ran into some snags with some reference problems using the queue based API in #25 (Since that seems to be the better way to go) and haven't gotten a chance since to look at it again. It is definitely something we would like to finish, as if it works well it would be a win for Starscream as well.

@jtreanor
Copy link
Contributor

Cool, thanks @acmacalister

We are considering moving back to jetfire from SocketRocket (we switched after encountering #9 and #13). It looks like Jetfire has gotten even better since the 😄

@acmacalister
Copy link
Owner

@jtreanor cool. If you end up switching back, let us know if you run into anything. 😄

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.

4 participants