You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@felix.apache.org by "glimmerveen (via GitHub)" <gi...@apache.org> on 2025/05/26 20:30:29 UTC
[PR] FELIX-6784 Reduce number of thread (re)creations of the Configurator Worker Thread [felix-dev]
glimmerveen opened a new pull request, #427:
URL: https://github.com/apache/felix-dev/pull/427
Changed the WorkerQueue to be based on the Java's standard ScheduledThreadPoolExecutor. This already implements keeping track of tasks, and allows for the configuring a keep alive time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6784 Reduce number of thread (re)creations of the Configurator Worker Thread [felix-dev]
Posted by "glimmerveen (via GitHub)" <gi...@apache.org>.
glimmerveen commented on code in PR #427:
URL: https://github.com/apache/felix-dev/pull/427#discussion_r2107831833
##########
configurator/src/main/java/org/apache/felix/configurator/impl/WorkerQueue.java:
##########
@@ -18,67 +18,32 @@
*/
package org.apache.felix.configurator.impl;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
import org.apache.felix.configurator.impl.logger.SystemLogger;
-public class WorkerQueue implements Runnable {
-
- private final ThreadFactory threadFactory;
-
- private final List<Runnable> tasks = new ArrayList<>();
-
- private volatile Thread backgroundThread;
-
- private volatile boolean stopped = false;
+public class WorkerQueue extends ScheduledThreadPoolExecutor {
public WorkerQueue() {
- this.threadFactory = Executors.defaultThreadFactory();
- }
-
- public void stop() {
- synchronized ( this.tasks ) {
- this.stopped = true;
- }
- }
-
- public void enqueue(final Runnable r) {
- synchronized ( this.tasks ) {
- if ( !this.stopped ) {
- this.tasks.add(r);
- if ( this.backgroundThread == null ) {
- this.backgroundThread = this.threadFactory.newThread(this);
- this.backgroundThread.setDaemon(true);
- this.backgroundThread.setName("Apache Felix Configurator Worker Thread");
- this.backgroundThread.start();
- }
+ super(1, new ThreadFactory() {
+ @Override
+ public Thread newThread(Runnable runnable) {
+ Thread thread = new Thread(runnable);
+ thread.setDaemon(true);
+ thread.setName("Apache Felix Configurator Worker Thread");
+ return thread;
}
- }
+ });
+ setKeepAliveTime(5, TimeUnit.SECONDS);
Review Comment:
This could be made configurable, but my initial proposal is to have a static, relatively short, keep alive time fixed in the code. Its purpose is mainly to let the underlying thread terminate at some point if no tasks are queued.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6784 Reduce number of thread (re)creations of the Configurator Worker Thread [felix-dev]
Posted by "stbischof (via GitHub)" <gi...@apache.org>.
stbischof commented on PR #427:
URL: https://github.com/apache/felix-dev/pull/427#issuecomment-3569236497
i see, so we have to add tck for configurator first.
do you know how to?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6784 Reduce number of thread (re)creations of the Configurator Worker Thread [felix-dev]
Posted by "stbischof (via GitHub)" <gi...@apache.org>.
stbischof commented on PR #427:
URL: https://github.com/apache/felix-dev/pull/427#issuecomment-3567872561
Hi, did you run this against the tck?
Can you rebase your pr and push again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6784 Reduce number of thread (re)creations of the Configurator Worker Thread [felix-dev]
Posted by "glimmerveen (via GitHub)" <gi...@apache.org>.
glimmerveen commented on PR #427:
URL: https://github.com/apache/felix-dev/pull/427#issuecomment-3569117830
> Hi, did you run this against the tck?
>
> Can you rebase your pr and push again? Then some actions should start the test.
I did not run the TCK myself, as I don't know how to do this; is there instructions on how to do this?
I did rebase the changes and push the change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6784 Reduce number of thread (re)creations of the Configurator Worker Thread [felix-dev]
Posted by "glimmerveen (via GitHub)" <gi...@apache.org>.
glimmerveen commented on PR #427:
URL: https://github.com/apache/felix-dev/pull/427#issuecomment-3574010829
> i see, so we have to add tck for configurator first.
>
> do you know how to?
I do see a way to add configurator's regular maven build to the ci workflows, but I don't really grasp how I could include the TCK for configurator specifically. I see there is a framework.tck module, but it does not build on my machine so I am not really sure how I could use that to setup an equivalent setup for configurator.
I am willing to set this up, but I need a bit of help on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FELIX-6784 Reduce number of thread (re)creations of the Configurator Worker Thread [felix-dev]
Posted by "glimmerveen (via GitHub)" <gi...@apache.org>.
glimmerveen commented on PR #427:
URL: https://github.com/apache/felix-dev/pull/427#issuecomment-3567864968
Hi @stbischof & @tjwatson , could you have a look at this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@felix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org