You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@struts.apache.org by "Falko Modler (Created) (JIRA)" <ji...@apache.org> on 2011/10/10 14:39:29 UTC
[jira] [Created] (WW-3691) BackgroundProcess should use a
java.util.concurrent.Executor alternatively to spawning a new thread
BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
---------------------------------------------------------------------------------------------------
Key: WW-3691
URL: https://issues.apache.org/jira/browse/WW-3691
Project: Struts 2
Issue Type: Improvement
Components: Core Interceptors
Affects Versions: 2.2.3.1
Reporter: Falko Modler
Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (WW-3691) BackgroundProcess should use a
java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Falko Modler (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Falko Modler updated WW-3691:
-----------------------------
Attachment: WW-3691_core.txt
Patch for ExecuteAndWaitInterceptor and BackgroundProcess. No test yet.
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch, WW-3691_core.txt, WW-3691_showcase.txt
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (WW-3691) BackgroundProcess should use a
java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Falko Modler (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13158307#comment-13158307 ]
Falko Modler commented on WW-3691:
----------------------------------
The implementation of my idea (see previous comment) was successful and I can provide a patch file for that by wednesday at the latest.
Test was ok in the demo app. I will provide a separate patch file for this.
I have yet to figure out, how to test it in the respective junit test. Are there any examples that test injection functionality?
I also want to test it in conjunction with spring (inject spring managed thread pool).
Sorry for the delay!
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (WW-3691) BackgroundProcess should use a
java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Falko Modler (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13160478#comment-13160478 ]
Falko Modler commented on WW-3691:
----------------------------------
Ok, I uploaded the patches.
Test is still missing and I still couldn't test it in conjunction with Spring.
Btw, getNewBackgroundProcess in ExecuteAndWaitInterceptor should be renamed to startNewBackgroundProcess. But that might break backwards compatibility.
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch, WW-3691_core.txt, WW-3691_showcase.txt
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (WW-3691) BackgroundProcess should use a
java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Lukasz Lenart (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lukasz Lenart updated WW-3691:
------------------------------
Fix Version/s: 3.x
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Assignee: Lukasz Lenart
> Fix For: 3.x
>
> Attachments: WW-3691.patch, WW-3691_core.txt, WW-3691_showcase.txt
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (WW-3691) BackgroundProcess should use a
java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Falko Modler (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Falko Modler updated WW-3691:
-----------------------------
Attachment: WW-3691_showcase.txt
Patch for the showcase-app.
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch, WW-3691_core.txt, WW-3691_showcase.txt
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Issue Comment Edited] (WW-3691) BackgroundProcess should
use a java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Falko Modler (Issue Comment Edited) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13126216#comment-13126216 ]
Falko Modler edited comment on WW-3691 at 10/12/11 10:44 PM:
-------------------------------------------------------------
Thanks for the quick reply!
I don't think this is a sufficient solution because in your patch you just delegate the execution of the Runnable/Thread to the interceptor which now implements Executor. But because there is no Thread.start() anymore, there is no more "background processing thread" anymore.
My idea is more like:
Inject an Executor into the interceptor (@Inject(required=false)) which is then passed to the BackgroundProcess constructor. BackgroundProcess then just creates a Runnable instance and passes it to the executer.
Additionally (mainly to preserve backward compatibility), the current constructor which spawns a new thread should be left almost untouched.
I will try it out myself this weekend and I will provide a patch if everything works as expected.
was (Author: famod):
Thanks for the quick reply!
I don't think this a sufficient solution because in your patch you just delegate the execution of the Runnable/Thread to the interceptor which now implements Executor. But because there is no Thread.start() anymore, there is no more "background processing thread" anymore.
My idea is more like:
Inject an Executor into the interceptor (@Inject(required=false)) which is then passed to the BackgroundProcess constructor.
I will try it out myself this weekend and I will provide a patch if everything works as expected.
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Assigned] (WW-3691) BackgroundProcess should use a
java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Lukasz Lenart (Assigned) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lukasz Lenart reassigned WW-3691:
---------------------------------
Assignee: Lukasz Lenart
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Assignee: Lukasz Lenart
> Attachments: WW-3691.patch, WW-3691_core.txt, WW-3691_showcase.txt
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (WW-3691) BackgroundProcess should use a
java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Lukasz Lenart (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lukasz Lenart updated WW-3691:
------------------------------
Attachment: WW-3691.patch
Is it a sufficient solution ?
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (WW-3691) BackgroundProcess should use a
java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Falko Modler (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13126216#comment-13126216 ]
Falko Modler commented on WW-3691:
----------------------------------
Thanks for the quick reply!
I don't think this a sufficient solution because in your patch you just delegate the execution of the Runnable/Thread to the interceptor which now implements Executor. But because there is no Thread.start() anymore, there is no more "background processing thread" anymore.
My idea is more like:
Inject an Executor into the interceptor (@Inject(required=false)) which is then passed to the BackgroundProcess constructor.
I will try it out myself this weekend and I will provide a patch if everything works as expected.
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (WW-3691) BackgroundProcess should use a
java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Lukasz Lenart (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13158316#comment-13158316 ]
Lukasz Lenart commented on WW-3691:
-----------------------------------
Thanks a lot!
Take a look on XWorkTestCase and container field.
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Issue Comment Edited] (WW-3691) BackgroundProcess should
use a java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Lukasz Lenart (Issue Comment Edited) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13124358#comment-13124358 ]
Lukasz Lenart edited comment on WW-3691 at 10/10/11 6:21 PM:
-------------------------------------------------------------
Is it a sufficient solution ? See attached patch
was (Author: lukaszlenart):
Is it a sufficient solution ?
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Issue Comment Edited] (WW-3691) BackgroundProcess should
use a java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Falko Modler (Issue Comment Edited) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13160478#comment-13160478 ]
Falko Modler edited comment on WW-3691 at 12/1/11 12:07 AM:
------------------------------------------------------------
Ok, I uploaded the patches.
Test is still missing and I still couldn't test it in conjunction with Spring.
I added some javadoc comments regarding the executor but I forgot to mention that threadPriority is ignored when the executor is used.
Btw, getNewBackgroundProcess in ExecuteAndWaitInterceptor should be renamed to startNewBackgroundProcess. But that might break backwards compatibility.
was (Author: famod):
Ok, I uploaded the patches.
Test is still missing and I still couldn't test it in conjunction with Spring.
Btw, getNewBackgroundProcess in ExecuteAndWaitInterceptor should be renamed to startNewBackgroundProcess. But that might break backwards compatibility.
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch, WW-3691_core.txt, WW-3691_showcase.txt
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Issue Comment Edited] (WW-3691) BackgroundProcess should
use a java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Falko Modler (Issue Comment Edited) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13126216#comment-13126216 ]
Falko Modler edited comment on WW-3691 at 10/12/11 10:45 PM:
-------------------------------------------------------------
Thanks for the quick reply!
I don't think this is a sufficient solution because in your patch you just delegate the execution of the Runnable/Thread to the interceptor which now implements Executor. But because Thread.start() has been removed, there is no more "background processing thread" anymore.
My idea is more like:
Inject an Executor into the interceptor (@Inject(required=false)) which is then passed to the BackgroundProcess constructor, if available. BackgroundProcess then just creates a Runnable instance and passes it to the executer.
Additionally (mainly to preserve backward compatibility), the current constructor which spawns a new thread should be left almost untouched.
I will try it out myself this weekend and I will provide a patch if everything works as expected.
was (Author: famod):
Thanks for the quick reply!
I don't think this is a sufficient solution because in your patch you just delegate the execution of the Runnable/Thread to the interceptor which now implements Executor. But because Thread.start() has been removed, there is no more "background processing thread" anymore.
My idea is more like:
Inject an Executor into the interceptor (@Inject(required=false)) which is then passed to the BackgroundProcess constructor. BackgroundProcess then just creates a Runnable instance and passes it to the executer.
Additionally (mainly to preserve backward compatibility), the current constructor which spawns a new thread should be left almost untouched.
I will try it out myself this weekend and I will provide a patch if everything works as expected.
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (WW-3691) BackgroundProcess should use a
java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Lukasz Lenart (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13158277#comment-13158277 ]
Lukasz Lenart commented on WW-3691:
-----------------------------------
Any progress ?
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Issue Comment Edited] (WW-3691) BackgroundProcess should
use a java.util.concurrent.Executor alternatively to spawning a new thread
Posted by "Falko Modler (Issue Comment Edited) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/WW-3691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13126216#comment-13126216 ]
Falko Modler edited comment on WW-3691 at 10/12/11 10:45 PM:
-------------------------------------------------------------
Thanks for the quick reply!
I don't think this is a sufficient solution because in your patch you just delegate the execution of the Runnable/Thread to the interceptor which now implements Executor. But because Thread.start() has been removed, there is no more "background processing thread" anymore.
My idea is more like:
Inject an Executor into the interceptor (@Inject(required=false)) which is then passed to the BackgroundProcess constructor. BackgroundProcess then just creates a Runnable instance and passes it to the executer.
Additionally (mainly to preserve backward compatibility), the current constructor which spawns a new thread should be left almost untouched.
I will try it out myself this weekend and I will provide a patch if everything works as expected.
was (Author: famod):
Thanks for the quick reply!
I don't think this is a sufficient solution because in your patch you just delegate the execution of the Runnable/Thread to the interceptor which now implements Executor. But because there is no Thread.start() anymore, there is no more "background processing thread" anymore.
My idea is more like:
Inject an Executor into the interceptor (@Inject(required=false)) which is then passed to the BackgroundProcess constructor. BackgroundProcess then just creates a Runnable instance and passes it to the executer.
Additionally (mainly to preserve backward compatibility), the current constructor which spawns a new thread should be left almost untouched.
I will try it out myself this weekend and I will provide a patch if everything works as expected.
> BackgroundProcess should use a java.util.concurrent.Executor alternatively to spawning a new thread
> ---------------------------------------------------------------------------------------------------
>
> Key: WW-3691
> URL: https://issues.apache.org/jira/browse/WW-3691
> Project: Struts 2
> Issue Type: Improvement
> Components: Core Interceptors
> Affects Versions: 2.2.3.1
> Reporter: Falko Modler
> Attachments: WW-3691.patch
>
>
> Every new instance of org.apache.struts2.interceptor.BackgroundProcess spawns a new Thread (see constructor), no thread pooling is used.
> Besides problems in environments where some container might need to manage the creation of threads, this issue also prevents certain performance/efficiency optimizations via ThreadLocal from taking full effect. E.g.: We use ThreadLocals for Random and SimpleDateFormat and those thread local instances "are lost" when a new Thread is created.
> Therefore BackgroundProcess should be given a new constructor that takes a java.util.concurrent.Executor instance that is used to execute the Runnable.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira