glimmerveen commented on code in PR #486:
URL: https://github.com/apache/felix-dev/pull/486#discussion_r2935037102
##########
scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java:
##########
@@ -718,42 +716,44 @@ public Dictionary<String, Object>
getServiceRegistrationProperties()
public void setRegistration(final
ServiceRegistration<ServiceComponentRuntime> reg)
{
- this.registration = reg;
+ long delay = m_configuration.serviceChangecountTimeout();
+ m_componentActor.scheduleWithFixedDelay(new
UpdateChangeCountProperty(reg), delay, delay, TimeUnit.MILLISECONDS);
Review Comment:
In my use-case I have a monitoring function that uses the update to the
changecount property as a trigger to inspect the state of the configurations
within SCR to see if any of them changed state. A low timeout helps to have
this trigger each my monitoring function in a timely manner. Having a large(r)
timeout still eventually reaches my monitoring function, but I have seen it
misses state changes in case a stream of changes continue to "invalidate" the
property update, and thus not reaching my monitoring function.
The previous implementation, which relied on creating and cancelling Timers,
was quite expensive with a low timeout value, as it would create a lot of
short-lived Threads during startup. I offered a change to move this as task to
the already existing ExecutorService (#419) which avoids the creation of
threads during startup. What still happens as you rightfully pointed out is
that each changecount update results in a task, and that task being scheduled
for execution *and* executing, and the evaluation if the task was invalidated
happens within the task's execution.
I think your suggestion of cancelling the task when a new one is about to be
scheduled can reduce the unnecessary execution of unneeded tasks. Looking at
the original approach with Timers, it appears to me that also in that case each
changecount update would be scheduled on the timer, similar to what is
happening right now. Though not as efficient as it perhaps can be, it looks to
me similar when a Timer was still being used, but as you report an additional
5% CPU time there is clearly something going on.
I remember you in the review voicing a
[concern](https://github.com/apache/felix-dev/pull/419#discussion_r2089032790)
on adding these changecount update on the already existing component actor
thread. I am not very familiar with what other types of tasks are executed
there, but perhaps you are observing different types of tasks scheduled on the
same thread interfering with each other?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
