Skip to content

Conversation

@dapengzhang0
Copy link
Contributor

@dapengzhang0 dapengzhang0 commented Feb 26, 2018

The is one of a sequence of PRs to allow application to provide all threads.
This PR added InProcess{Channel, Server}Builder.scheduledExecutorService() API

return new InProcessServer(name, GrpcUtil.TIMER_SERVICE, streamTracerFactories);
ObjectPool<ScheduledExecutorService> schedulerPool =
scheduledExecutorService == null
? SharedResourcePool.forResource(GrpcUtil.TIMER_SERVICE)
Copy link
Member

Choose a reason for hiding this comment

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

Use a ObjectPool field instead of ScheduledExecutorService and make this the default value. Then in the scheduledExecutorService() method overwrite it with the FixedObjectPool.

You could do this pattern in InProcessChannelBuilder as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. The pattern does not provide much gain for InProcessChannelBuilder, and currently it's in the same pattern as NettyChannelBuilder and OkHttpChannelBuilder.

/**
* Only used to make sure the scheduler has at least one reference. Since child transports can
* outlive this server, they must get their own reference.
*/
Copy link
Member

Choose a reason for hiding this comment

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

You need to update schedulerPool's documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@dapengzhang0 dapengzhang0 merged commit 39decad into grpc:master Feb 27, 2018
@dapengzhang0 dapengzhang0 deleted the provideallthreads1 branch May 2, 2018 17:18
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants