Stop flooding the scheduled executor with no-op tasks for change count updates#486
Open
tjwatson wants to merge 3 commits intoapache:masterfrom
Open
Stop flooding the scheduled executor with no-op tasks for change count updates#486tjwatson wants to merge 3 commits intoapache:masterfrom
tjwatson wants to merge 3 commits intoapache:masterfrom
Conversation
On startup SCR will blast change count updates for each component it satisfies and activates etc. This resulted in a very large number of task being submitted to the scheduled executor. Prior to using an executor a Timer was used. In both cases the tasks would wait a default of 5 seconds before updating the change count service property. Every task submitted would be a no-op except the very last one which had the "final" change count value. This behavior is to avoid flooding the system with service modified events. The issue is that now we are flooding the scheduled executor with a significant number of task that most all do nothing. Since moving to an executor we noticed a non-trivial bump in our CPU usage when the default 5 seconds passes to run all the queued tasks. It turns out that on the JVM we are using the Timer is actually more efficient than the scheduled executor for popping off all the tasks and running them when the delay timeout is hit. The overall design here is sub-optimal regardless. Flooding a queue with all but one task that do nothing is not efficient. This change moves to using a simple repeating task that just updates the change count service property, if needed, every delay period (defaults to every 5 seconds).
3 tasks
stbischof
reviewed
Mar 12, 2026
scr/src/main/java/org/apache/felix/scr/impl/inject/methods/BaseMethod.java
Show resolved
Hide resolved
stbischof
reviewed
Mar 12, 2026
scr/src/main/java/org/apache/felix/scr/impl/inject/methods/BaseMethod.java
Show resolved
Hide resolved
glimmerveen
suggested changes
Mar 12, 2026
scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java
Outdated
Show resolved
Hide resolved
stbischof
approved these changes
Mar 24, 2026
After at least 10 seconds of inactivity this will cancel the ScheduledFuture for updating the change count service property. If later the change count is updated then a new ScheduledFuture is submitted.
tjwatson
force-pushed
the
felixLogParams
branch
from
d1c738a to
51de0a8
Compare
glimmerveen
reviewed
Mar 24, 2026
| return; | ||
| } | ||
| try { | ||
| Long registeredChangeCount = (Long) currentReg.getReference().getProperty(PROP_CHANGECOUNT); |
Contributor
There was a problem hiding this comment.
I think getReference, just like setProperties, can throw an IllegalStateException. Should that also not be ignored, currently it will end up in the catch block where it is logged as a warning.
glimmerveen
reviewed
Mar 24, 2026
| public ComponentRegistry(final ScrConfiguration scrConfiguration, final ScrLogger logger, final ScheduledExecutorService componentActor ) | ||
| { | ||
| m_configuration = scrConfiguration; | ||
| m_updateChangeCountPropertyTask = new UpdateChangeCountProperty(m_configuration.serviceChangecountTimeout()); |
Contributor
There was a problem hiding this comment.
Should a safe guard be implemented here to deal with a timeout of 0 being configured? With the updated approach using a 'delay' of 0 does not when calculating the maxNumberOfNoChanges nor using it as interval when scheduling a task.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR has two commits