[jira] [Commented] (THRIFT-4190) improve C# TThreadPoolServer defaults

2017-05-13 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4190?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16009212#comment-16009212
 ] 

ASF GitHub Bot commented on THRIFT-4190:


Github user Jens-G commented on the issue:

https://github.com/apache/thrift/pull/1268
  
The NPM timed out, no other errors. If there are no objections against it, 
I'll commit that shortly. 
So please speak now (or be quiet forever, as they say) ... :-)


> improve C# TThreadPoolServer defaults
> -
>
> Key: THRIFT-4190
> URL: https://issues.apache.org/jira/browse/THRIFT-4190
> Project: Thrift
>  Issue Type: Improvement
>  Components: C# - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>
> The TThreadPoolServer uses hardcoded values to specify min/max number of 
> threads, if the caller does not specify them. This is rather unexpected in my 
> opinion, since the global C# ThreadPool (which is used internally) comes with 
> its own defaults for all 4 values - yes, 4, not 2: there are different 
> settings for the number of threads on one hand and the number of asyn IO 
> completion ports on the other, and they are not necessary identical numbers. 
> For example, on my machine I get these numbers by default:
> - min 4 threads and 4 I/O completion ports
> - max 37267 threads and 1000 I/O completion ports
> There are several *problems* with this approach:
> # There is really no way to bypass the defaults of min 10/10 and max 100/100 
> that are hard-coded into TThreadPoolServer and use the defaults provided by 
> the NET framework instead, since we can only pass number which is then used 
> for threads AND io ports. In my example, no matter what value I pass, 37267 
> or 1000, it will be something other than the defaults.
> # It is rather unexpected to have Thrift override the default settings of the 
> global thread pool object if I don't even provide values by calling one of 
> the simpler TThreadPoolServer  CTORs.
> # I'm not sure where the defaults are come from. Both numbers look like wild 
> guesswork to me. The defaults provided by the runtime make much more sense, 
> as they automatically adapt to the machine's capabilities.
> My *proposal* to solve it comes in two parts:
> # Change  the CTOR in a way that interprets 0 or negative values as intention 
> to stick with the NET default settings. I think that is the best way to 
> handle it, as the current implementation would just throw in a very defined 
> way, so we don't get any compatibility conflicts here that pass undetectedly.
> # Additionally make the default values {{DEFAULT_MAX_THREADS}} and 
> {{DEFAULT_MIN_THREADS}} both 0 (or negative) to enforce the system's 
> defaults. Since this will be a breaking change, as it changes the current 
> default behaviour, I'd like to know the opinions of the community before I 
> commit that part of the changes.
> Further reference
> - [SetMinThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setminthreads(v=vs.110).aspx]
> - [SetMaxThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setmaxthreads(v=vs.110).aspx]



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (THRIFT-4190) improve C# TThreadPoolServer defaults

2017-05-12 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4190?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16008724#comment-16008724
 ] 

ASF GitHub Bot commented on THRIFT-4190:


GitHub user Jens-G opened a pull request:

https://github.com/apache/thrift/pull/1268

THRIFT-4190 Improve C# TThreadPoolServer defaults (part 2 of 2)

Client: C#
Patch: Jens Geyer

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Jens-G/thrift THRIFT-4190

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/thrift/pull/1268.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1268


commit 6f1a3c23744d573488dc64c196ba4e8932c33d3b
Author: Jens Geyer 
Date:   2017-05-12T20:49:57Z

THRIFT-4190 Improve C# TThreadPoolServer defaults (part 2 of 2)
Client: C#
Patch: Jens Geyer




> improve C# TThreadPoolServer defaults
> -
>
> Key: THRIFT-4190
> URL: https://issues.apache.org/jira/browse/THRIFT-4190
> Project: Thrift
>  Issue Type: Improvement
>  Components: C# - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>
> The TThreadPoolServer uses hardcoded values to specify min/max number of 
> threads, if the caller does not specify them. This is rather unexpected in my 
> opinion, since the global C# ThreadPool (which is used internally) comes with 
> its own defaults for all 4 values - yes, 4, not 2: there are different 
> settings for the number of threads on one hand and the number of asyn IO 
> completion ports on the other, and they are not necessary identical numbers. 
> For example, on my machine I get these numbers by default:
> - min 4 threads and 4 I/O completion ports
> - max 37267 threads and 1000 I/O completion ports
> There are several *problems* with this approach:
> # There is really no way to bypass the defaults of min 10/10 and max 100/100 
> that are hard-coded into TThreadPoolServer and use the defaults provided by 
> the NET framework instead, since we can only pass number which is then used 
> for threads AND io ports. In my example, no matter what value I pass, 37267 
> or 1000, it will be something other than the defaults.
> # It is rather unexpected to have Thrift override the default settings of the 
> global thread pool object if I don't even provide values by calling one of 
> the simpler TThreadPoolServer  CTORs.
> # I'm not sure where the defaults are come from. Both numbers look like wild 
> guesswork to me. The defaults provided by the runtime make much more sense, 
> as they automatically adapt to the machine's capabilities.
> My *proposal* to solve it comes in two parts:
> # Change  the CTOR in a way that interprets 0 or negative values as intention 
> to stick with the NET default settings. I think that is the best way to 
> handle it, as the current implementation would just throw in a very defined 
> way, so we don't get any compatibility conflicts here that pass undetectedly.
> # Additionally make the default values {{DEFAULT_MAX_THREADS}} and 
> {{DEFAULT_MIN_THREADS}} both 0 (or negative) to enforce the system's 
> defaults. Since this will be a breaking change, as it changes the current 
> default behaviour, I'd like to know the opinions of the community before I 
> commit that part of the changes.
> Further reference
> - [SetMinThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setminthreads(v=vs.110).aspx]
> - [SetMaxThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setmaxthreads(v=vs.110).aspx]



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (THRIFT-4190) improve C# TThreadPoolServer defaults

2017-05-07 Thread Jens Geyer (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4190?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15999764#comment-15999764
 ] 

Jens Geyer commented on THRIFT-4190:


Curent status is this:
- part 1 is committed. One may now pass 0 or negative values w/o earning an 
exception

- part 2 open: Additionally make the default values DEFAULT_MAX_THREADS and 
DEFAULT_MIN_THREADS both 0 (or negative) to enforce the system's defaults. 
Since this will be a breaking change, as it changes the current default 
behaviour, I'd like to know the opinions of the community before I commit that 
part of the changes.

- part 3 open: add a CTOR that expects a struct with settings, or b) simply 
ignore that case for now and rely on the developer to call the global thread 
pool methods after instantiating the Thrift server object.

If there are no objections, I will continue working on #2 and #3a).


> improve C# TThreadPoolServer defaults
> -
>
> Key: THRIFT-4190
> URL: https://issues.apache.org/jira/browse/THRIFT-4190
> Project: Thrift
>  Issue Type: Improvement
>  Components: C# - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>
> The TThreadPoolServer uses hardcoded values to specify min/max number of 
> threads, if the caller does not specify them. This is rather unexpected in my 
> opinion, since the global C# ThreadPool (which is used internally) comes with 
> its own defaults for all 4 values - yes, 4, not 2: there are different 
> settings for the number of threads on one hand and the number of asyn IO 
> completion ports on the other, and they are not necessary identical numbers. 
> For example, on my machine I get these numbers by default:
> - min 4 threads and 4 I/O completion ports
> - max 37267 threads and 1000 I/O completion ports
> There are several *problems* with this approach:
> # There is really no way to bypass the defaults of min 10/10 and max 100/100 
> that are hard-coded into TThreadPoolServer and use the defaults provided by 
> the NET framework instead, since we can only pass number which is then used 
> for threads AND io ports. In my example, no matter what value I pass, 37267 
> or 1000, it will be something other than the defaults.
> # It is rather unexpected to have Thrift override the default settings of the 
> global thread pool object if I don't even provide values by calling one of 
> the simpler TThreadPoolServer  CTORs.
> # I'm not sure where the defaults are come from. Both numbers look like wild 
> guesswork to me. The defaults provided by the runtime make much more sense, 
> as they automatically adapt to the machine's capabilities.
> My *proposal* to solve it comes in two parts:
> # Change  the CTOR in a way that interprets 0 or negative values as intention 
> to stick with the NET default settings. I think that is the best way to 
> handle it, as the current implementation would just throw in a very defined 
> way, so we don't get any compatibility conflicts here that pass undetectedly.
> # Additionally make the default values {{DEFAULT_MAX_THREADS}} and 
> {{DEFAULT_MIN_THREADS}} both 0 (or negative) to enforce the system's 
> defaults. Since this will be a breaking change, as it changes the current 
> default behaviour, I'd like to know the opinions of the community before I 
> commit that part of the changes.
> Further reference
> - [SetMinThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setminthreads(v=vs.110).aspx]
> - [SetMaxThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setmaxthreads(v=vs.110).aspx]



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (THRIFT-4190) improve C# TThreadPoolServer defaults

2017-05-07 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4190?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15999762#comment-15999762
 ] 

ASF GitHub Bot commented on THRIFT-4190:


Github user Jens-G closed the pull request at:

https://github.com/apache/thrift/pull/1262


> improve C# TThreadPoolServer defaults
> -
>
> Key: THRIFT-4190
> URL: https://issues.apache.org/jira/browse/THRIFT-4190
> Project: Thrift
>  Issue Type: Improvement
>  Components: C# - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>
> The TThreadPoolServer uses hardcoded values to specify min/max number of 
> threads, if the caller does not specify them. This is rather unexpected in my 
> opinion, since the global C# ThreadPool (which is used internally) comes with 
> its own defaults for all 4 values - yes, 4, not 2: there are different 
> settings for the number of threads on one hand and the number of asyn IO 
> completion ports on the other, and they are not necessary identical numbers. 
> For example, on my machine I get these numbers by default:
> - min 4 threads and 4 I/O completion ports
> - max 37267 threads and 1000 I/O completion ports
> There are several *problems* with this approach:
> # There is really no way to bypass the defaults of min 10/10 and max 100/100 
> that are hard-coded into TThreadPoolServer and use the defaults provided by 
> the NET framework instead, since we can only pass number which is then used 
> for threads AND io ports. In my example, no matter what value I pass, 37267 
> or 1000, it will be something other than the defaults.
> # It is rather unexpected to have Thrift override the default settings of the 
> global thread pool object if I don't even provide values by calling one of 
> the simpler TThreadPoolServer  CTORs.
> # I'm not sure where the defaults are come from. Both numbers look like wild 
> guesswork to me. The defaults provided by the runtime make much more sense, 
> as they automatically adapt to the machine's capabilities.
> My *proposal* to solve it comes in two parts:
> # Change  the CTOR in a way that interprets 0 or negative values as intention 
> to stick with the NET default settings. I think that is the best way to 
> handle it, as the current implementation would just throw in a very defined 
> way, so we don't get any compatibility conflicts here that pass undetectedly.
> # Additionally make the default values {{DEFAULT_MAX_THREADS}} and 
> {{DEFAULT_MIN_THREADS}} both 0 (or negative) to enforce the system's 
> defaults. Since this will be a breaking change, as it changes the current 
> default behaviour, I'd like to know the opinions of the community before I 
> commit that part of the changes.
> Further reference
> - [SetMinThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setminthreads(v=vs.110).aspx]
> - [SetMaxThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setmaxthreads(v=vs.110).aspx]



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (THRIFT-4190) improve C# TThreadPoolServer defaults

2017-05-06 Thread Jens Geyer (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4190?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15999580#comment-15999580
 ] 

Jens Geyer commented on THRIFT-4190:


That still does not solve the problem, that one may (for whatever reason) may 
want to change the numbers of threads and I/O completion ports to different 
values. Instead of adding two more arguments we could a) add a CTOR that 
expects a struct with settings, or b) ignore that case and rely on the 
developer to call the global thread pool methods after instantiating the Thrift 
server object. My favourite would be a).

> improve C# TThreadPoolServer defaults
> -
>
> Key: THRIFT-4190
> URL: https://issues.apache.org/jira/browse/THRIFT-4190
> Project: Thrift
>  Issue Type: Improvement
>  Components: C# - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>
> The TThreadPoolServer uses hardcoded values to specify min/max number of 
> threads, if the caller does not specify them. This is rather unexpected in my 
> opinion, since the global C# ThreadPool (which is used internally) comes with 
> its own defaults for all 4 values - yes, 4, not 2: there are different 
> settings for the number of threads on one hand and the number of asyn IO 
> completion ports on the other, and they are not necessary identical numbers. 
> For example, on my machine I get these numbers by default:
> - min 4 threads and 4 I/O completion ports
> - max 37267 threads and 1000 I/O completion ports
> There are several *problems* with this approach:
> # There is really no way to bypass the defaults of min 10/10 and max 100/100 
> that are hard-coded into TThreadPoolServer and use the defaults provided by 
> the NET framework instead, since we can only pass number which is then used 
> for threads AND io ports. In my example, no matter what value I pass, 37267 
> or 1000, it will be something other than the defaults.
> # It is rather unexpected to have Thrift override the default settings of the 
> global thread pool object if I don't even provide values by calling one of 
> the simpler TThreadPoolServer  CTORs.
> # I'm not sure where the defaults are come from. Both numbers look like wild 
> guesswork to me. The defaults provided by the runtime make much more sense, 
> as they automatically adapt to the machine's capabilities.
> My *proposal* to solve it comes in two parts:
> # Change  the CTOR in a way that interprets 0 or negative values as intention 
> to stick with the NET default settings. I think that is the best way to 
> handle it, as the current implementation would just throw in a very defined 
> way, so we don't get any compatibility conflicts here that pass undetectedly.
> # Additionally make the default values {{DEFAULT_MAX_THREADS}} and 
> {{DEFAULT_MIN_THREADS}} both 0 (or negative) to enforce the system's 
> defaults. Since this will be a breaking change, as it changes the current 
> default behaviour, I'd like to know the opinions of the community before I 
> commit that part of the changes.
> Further reference
> - [SetMinThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setminthreads(v=vs.110).aspx]
> - [SetMaxThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setmaxthreads(v=vs.110).aspx]



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Commented] (THRIFT-4190) improve C# TThreadPoolServer defaults

2017-05-06 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/THRIFT-4190?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15999578#comment-15999578
 ] 

ASF GitHub Bot commented on THRIFT-4190:


GitHub user Jens-G opened a pull request:

https://github.com/apache/thrift/pull/1262

THRIFT-4190 Improve C# TThreadPoolServer defaults

Client: C#
Patch: Jens Geyer

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Jens-G/thrift THRIFT-4190

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/thrift/pull/1262.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1262






> improve C# TThreadPoolServer defaults
> -
>
> Key: THRIFT-4190
> URL: https://issues.apache.org/jira/browse/THRIFT-4190
> Project: Thrift
>  Issue Type: Improvement
>  Components: C# - Library
>Reporter: Jens Geyer
>Assignee: Jens Geyer
>
> The TThreadPoolServer uses hardcoded values to specify min/max number of 
> threads, if the caller does not specify them. This is rather unexpected in my 
> opinion, since the global C# ThreadPool (which is used internally) comes with 
> its own defaults for all 4 values - yes, 4, not 2: there are different 
> settings for the number of threads on one hand and the number of asyn IO 
> completion ports on the other, and they are not necessary identical numbers. 
> For example, on my machine I get these numbers by default:
> - min 4 threads and 4 I/O completion ports
> - max 37267 threads and 1000 I/O completion ports
> There are several *problems* with this approach:
> # There is really no way to bypass the defaults of min 10/10 and max 100/100 
> that are hard-coded into TThreadPoolServer and use the defaults provided by 
> the NET framework instead, since we can only pass number which is then used 
> for threads AND io ports. In my example, no matter what value I pass, 37267 
> or 1000, it will be something other than the defaults.
> # It is rather unexpected to have Thrift override the default settings of the 
> global thread pool object if I don't even provide values by calling one of 
> the simpler TThreadPoolServer  CTORs.
> # I'm not sure where the defaults are come from. Both numbers look like wild 
> guesswork to me. The defaults provided by the runtime make much more sense, 
> as they automatically adapt to the machine's capabilities.
> My *proposal* to solve it comes in two parts:
> # Change  the CTOR in a way that interprets 0 or negative values as intention 
> to stick with the NET default settings. I think that is the best way to 
> handle it, as the current implementation would just throw in a very defined 
> way, so we don't get any compatibility conflicts here that pass undetectedly.
> # Additionally make the default values {{DEFAULT_MAX_THREADS}} and 
> {{DEFAULT_MIN_THREADS}} both 0 (or negative) to enforce the system's 
> defaults. Since this will be a breaking change, as it changes the current 
> default behaviour, I'd like to know the opinions of the community before I 
> commit that part of the changes.
> Further reference
> - [SetMinThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setminthreads(v=vs.110).aspx]
> - [SetMaxThreads 
> method|https://msdn.microsoft.com/en-us/library/system.threading.threadpool.setmaxthreads(v=vs.110).aspx]



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)