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

Change number of netty IO threads for feedback client #9

Closed
impactmobile-dev opened this issue Oct 24, 2013 · 2 comments
Closed

Change number of netty IO threads for feedback client #9

impactmobile-dev opened this issue Oct 24, 2013 · 2 comments
Milestone

Comments

@impactmobile-dev
Copy link

As of now, Netty will create 2*availableProcessers threads for the FeedbackServiceClient's bootstrap group. This is somewhat of a waste since pushy will only use 1 connection for the feedback service (it's obviously pointless to use more than one).

We can't set the io.netty.eventLoopThreads as we'll definitely want more than one for the regular apns client.

In FeedbackServiceClient constructor:

this.bootstrap.group(new NioEventLoopGroup(1));
@andrewscode
Copy link

Another thought on this, but for the regular ApnsClientThread. Since pushy already creates a thread per connection, it might make sense to set the netty group to only 1 thread. For example, right now on my machine (8 core), a pushManager with 1 thread will end up creating 16 threads for netty. The other 15 threads for netty will be wasted.

Is there any benefit to having netty use more than 1 thread when pushy already has a thread per connection?

Or should the ApnsClientThread share the same bootstrap group for all ApnsClientThreads? (I know this one is definitely not simple)

@jchambers
Copy link
Owner

This is somewhat of a waste since pushy will only use 1 connection for the feedback service (it's obviously pointless to use more than one).

Definitely true.

Is there any benefit to having netty use more than 1 thread when pushy already has a thread per connection?

Nope. This has actually been a TODO in my head for a while. Didn't realize quite how many threads we were using, though, so thanks for calling attention to it. #10 should do the easy stuff. I'd love to condense everything into a single bootstrap group, but will save that for a future enhancement.

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

No branches or pull requests

3 participants