apache / celix

Apache Celix is a framework for C and C++14 to develop dynamic modular software applications using component and in-process service-oriented programming.
https://celix.apache.org/
Apache License 2.0
166 stars 87 forks source link

Feature/scheduled event on event thread #583

Closed pnoltes closed 1 year ago

pnoltes commented 1 year ago

This PR adds a scheduled event abstraction to the Celix framework, enabling the execution of scheduled events on the Celix event thread.

While scheduled events are processed on the Apache Celix event thread, they are not part of the Apache Celix event queue, meaning that a waitForEmptyQueue call will not wait for scheduled events.

The introduction of scheduled events allows small, low-frequency tasks to be performed on the existing Apache Celix event thread, thus reducing the number of threads that Apache Celix requires.

There are two types of scheduled events:

A scheduled event can be added, removed, or woken up from the bundle context using both C and C++ APIs.

In addition to these changes, this PR:

Please note that the use of scheduled events in remote services, PubSub, and/or remote services shared memory watchers can be reverted if we decide that this step is unnecessary or too risky.

codecov-commenter commented 1 year ago

Codecov Report

Merging #583 (4739e45) into master (f9e2875) will increase coverage by 0.29%. The diff coverage is 96.55%.

:exclamation: Current head 4739e45 differs from pull request most recent head b3fcf16. Consider uploading reports for the commit b3fcf16 to get more accurate results

@@            Coverage Diff             @@
##           master     #583      +/-   ##
==========================================
+ Coverage   78.24%   78.54%   +0.29%     
==========================================
  Files         230      234       +4     
  Lines       35010    35386     +376     
==========================================
+ Hits        27395    27793     +398     
+ Misses       7615     7593      -22     
Impacted Files Coverage Δ
...sub_topology_manager/src/pubsub_topology_manager.c 52.38% <ø> (ø)
libs/framework/include/celix/Framework.h 100.00% <ø> (ø)
libs/framework/include/celix/dm/Component.h 94.44% <ø> (ø)
...ibs/framework/include/celix/dm/DependencyManager.h 100.00% <ø> (ø)
libs/utils/src/celix_log_utils.c 68.88% <60.00%> (-16.83%) :arrow_down:
...te_services/topology_manager/tms_tst/tms_tests.cpp 98.11% <80.00%> (-0.78%) :arrow_down:
libs/utils/src/celix_threads.c 83.33% <85.71%> (+6.26%) :arrow_up:
libs/framework/src/framework.c 82.98% <94.68%> (+1.53%) :arrow_up:
libs/framework/src/service_tracker.c 77.63% <95.23%> (-0.49%) :arrow_down:
libs/framework/include/celix/BundleContext.h 100.00% <100.00%> (ø)
... and 6 more

... and 6 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

PengZheng commented 1 year ago

Elegant design and clean implementation :+1: I really enjoy reading the newly added codes.

Some quick fixes are deliberately sent in small pieces, so that they can be reverted very easily if there is anything wrong or any disagreement.

Have not finished my review yet.

pnoltes commented 1 year ago

My review is done. I found the event loop is trickier than I thought:

* Processing one kind of events should not starve other event processing. Currently, either `fw_handleEvents` or `celix_framework_processScheduledEvents` could run arbitrarily long (see above remarks for details).

* We should strive for more accurate timer management, i.e. taking the event processing time into account.

* Network IO or other operations that could block for indefinite duration should be prohibited.

I will update the document so that it is documented that Network IO should not be done in scheduled event callbacks.

PengZheng commented 1 year ago

Before merging it, we'd better pay attention to ScheduledEventTestSuite.ScheduledEventTimeoutLogTest, which frequently failed.

pnoltes commented 1 year ago

Before merging it, we'd better pay attention to ScheduledEventTestSuite.ScheduledEventTimeoutLogTest, which frequently failed.

I will look into this. Note that getting the timings and especially the allowed error margin working on our CI pipeline has been quite a strugle. On my local machine I can set the error margins much lower, but on the CI pipeline (virtual machines) this needs to increased.