pat-lego opened a new pull request, #89:
URL: https://github.com/apache/sling-org-apache-sling-models-impl/pull/89
## Problem
`OSGiServiceInjector` calls `BundleContext.getServiceReferences()` and
`BundleContext.getService()` for **every** `@OSGiService` injection of
**every** model instance, and `ungetService()` again from the model's disposal
callback. Every one of these calls goes through the framework service registry.
On Felix, `getServiceReferences()` additionally gets, invokes and ungets every
registered `FindHook` on each call.
When a page instantiates many models concurrently, request threads end up
serialised on the service registry. A thread dump taken under load on a Felix 7
/ JDK 21 publish instance showed 79 of 180 request threads inside
`ServiceRegistry.getService()` called from `Felix.getServiceReferences()` (i.e.
while obtaining a `FindHook`). 73 of them were parked on a
`ServiceRegistry$ServiceHolder` `CountDownLatch`, and 64 came from
`OSGiServiceInjector`:
```
java.lang.Thread.State: WAITING (parking)
at jdk.internal.misc.Unsafe.park(Native Method)
- parking to wait for <0x00000007f3803288> (a
java.util.concurrent.CountDownLatch$Sync)
at java.util.concurrent.CountDownLatch.await(CountDownLatch.java:230)
at
org.apache.felix.framework.ServiceRegistry.getService(ServiceRegistry.java:380)
at org.apache.felix.framework.Felix.getService(Felix.java:3984)
at org.apache.felix.framework.Felix.getServiceReferences(Felix.java:3895)
<-- obtaining the FindHook
at
org.apache.felix.framework.Felix.getAllowedServiceReferences(Felix.java:3947)
at
org.apache.felix.framework.BundleContextImpl.getServiceReferences(BundleContextImpl.java:414)
at
org.apache.sling.models.impl.injectors.OSGiServiceInjector.getService(OSGiServiceInjector.java:124)
at
org.apache.sling.models.impl.injectors.OSGiServiceInjector.getValue(OSGiServiceInjector.java:198)
at
org.apache.sling.models.impl.injectors.OSGiServiceInjector.getValue(OSGiServiceInjector.java:111)
at
org.apache.sling.models.impl.ModelAdapterFactory.injectElementInternal(ModelAdapterFactory.java:600)
at
org.apache.sling.models.impl.ModelAdapterFactory.injectElement(ModelAdapterFactory.java:535)
at
org.apache.sling.models.impl.ModelAdapterFactory.createObject(ModelAdapterFactory.java:754)
at
org.apache.sling.models.impl.ModelAdapterFactory.internalCreateModel(ModelAdapterFactory.java:409)
at
org.apache.sling.models.impl.ModelAdapterFactory.getAdapter(ModelAdapterFactory.java:219)
at
org.apache.sling.adapter.internal.AdapterManagerImpl.getAdapter(AdapterManagerImpl.java:121)
at
org.apache.sling.api.adapter.SlingAdaptable.adaptTo(SlingAdaptable.java:104)
...
```
Other request threads in the same dump were creating a new `ServiceHolder`
(and latch) for the hook. Felix only does this when the previous usage of the
hook has been fully released, so the holder is torn down and rebuilt over and
over:
```
java.lang.Thread.State: RUNNABLE
at java.util.concurrent.CountDownLatch.<init>(CountDownLatch.java:199)
at
org.apache.felix.framework.ServiceRegistry$ServiceHolder.<init>(ServiceRegistry.java:786)
at
org.apache.felix.framework.ServiceRegistry.getService(ServiceRegistry.java:357)
at org.apache.felix.framework.Felix.getService(Felix.java:3984)
at org.apache.felix.framework.Felix.getServiceReferences(Felix.java:3895)
at
org.apache.felix.framework.Felix.getAllowedServiceReferences(Felix.java:3947)
at
org.apache.felix.framework.BundleContextImpl.getServiceReferences(BundleContextImpl.java:414)
at
org.apache.sling.models.impl.injectors.OSGiServiceInjector.getService(OSGiServiceInjector.java:124)
...
```
The set of services a model can inject only changes when a service is
registered, modified or unregistered, so doing a registry lookup per injection
is avoidable.
## Change
This PR adds a package-private `OSGiServiceCache`, used by
`OSGiServiceInjector`:
- **References** are cached per *(requesting bundle context, service class,
filter)* and sorted by descending service ranking once. Empty results are
cached too, so repeated lookups of absent `@Optional` services do not hit the
registry either. Any `ServiceEvent` (`REGISTERED`, `MODIFIED`, `UNREGISTERING`)
for a service evicts all cached lookups for each of its `objectClass` names.
The cache uses an `AllServiceListener`, so events are not filtered by
class-space compatibility. Lookups with an invalid filter are not cached.
- **Service objects** are obtained once per *(requesting bundle context,
reference)* and kept, similar to a static DS reference. They are released when
the service is unregistered, when the requesting (model) bundle stops
(`SynchronousBundleListener`, `STOPPING`), or when the injector is deactivated.
Cached services are no longer registered in the model's disposal callback.
Services obtained outside the cache still are.
- **Races:** the per-class map is resolved before the registry is queried,
so a result computed concurrently with an eviction ends up in the evicted map
and is never served. If a cached reference turns out to be unregistered
(`ServiceReference.getBundle() == null`), it is evicted and released on first
use.
- **Fallback:** while the cache is not active (before activation or after
deactivation), the injector behaves exactly as before: lookup, `getService` and
`ungetService` via the disposal callback.
No API or exported-package change (bnd baseline passes).
### Behaviour change to discuss
- Bundle-scoped `ServiceFactory` services now keep one usage per model
bundle for as long as the service is registered. Previously, the usage count
went to 0 after each model's disposal callback, which could trigger
`ServiceFactory.ungetService` (and, for delayed DS components, deactivation)
between requests. The injector keeps using `BundleContext.getService`, so
prototype-scope semantics are unchanged.
- If a `FindHook` changes its visibility decisions **without** a service
event for the affected service class, and the requesting bundle is not stopped,
the change only shows up after the next such event. Reviewers may know of
setups where this matters, such as dynamic region or subsystem filtering.
## Testing
- `mvn verify` on JDK 21: 301 unit tests and 28 integration tests pass;
spotless and bnd baseline pass.
- New `OSGiServiceCacheTest` (13 tests) covers lookup-once, ranking order,
negative caching, invalid filters, eviction by all object classes, per-context
service caching, unregistered-while-obtained, `UNREGISTERING` release, bundle
`STOPPING` and `clear()`.
- `OSGiInjectionTest` has 6 new tests: caching across model instances,
collection injection, eviction on `REGISTERED`, release on `UNREGISTERING`, a
stale cached reference, and deactivation fallback.
- I also ran a local microbenchmark: Felix 7.0.5 with Felix SCR, one no-op
delayed-DS `FindHook`, 3 candidate services, 64 threads, and 50 injections per
"request" before the disposal-callback `ungetService`. It measured about 19-21
µs per injection with the current lookup+get path and about 0.1-0.6 µs with the
cache (single thread: about 1.3-2.0 µs vs. about 0.01 µs). This synthetic setup
did not reproduce the latch parking seen in the production dump, which depends
on how expensive it is to obtain the real hooks. Treat these numbers as an
indication of per-injection registry overhead only.
## Notes
There is no SLING Jira issue for this yet. I'm opening this as a draft to
discuss the approach and am happy to create the issue and adjust naming or
conventions.
--
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]
