jbonofre commented on code in PR #741:
URL: https://github.com/apache/camel-karaf/pull/741#discussion_r3907732586
##########
core/camel-core-osgi/src/main/java/org/apache/camel/karaf/core/OsgiFactoryFinder.java:
##########
@@ -79,23 +87,51 @@ public Optional<Class<?>> findClass(String key) {
// NOTE, the first found factory will be return
public BundleEntry getResource(String name) {
BundleEntry entry = null;
- Bundle[] bundles;
+ // only allocated when more than one bundle provides the same
descriptor, which is not the normal case
+ List<Bundle> alsoProviding = null;
- bundles = bundleContext.getBundles();
+ Bundle[] bundles = bundleContext.getBundles();
- URL url;
for (Bundle bundle : bundles) {
- url = bundle.getEntry(getResourcePath() + name);
+ URL url = bundle.getEntry(getResourcePath() + name);
Review Comment:
Addressed, and this is now the point of the PR. The scan skips
`Bundle.UNINSTALLED` and catches `IllegalStateException` from `getEntry`
(needed as well as the state check, for the bundle uninstalled between the two).
Confirmed against the 4.18.1 bytecode: the mapping function in
`addToClassMap` catches `Exception`, does `classesNotFoundExceptions.put(key,
e)` and rethrows, and on entry `addToClassMap` rethrows the cached exception
for that key. So yes — one race and the key is dead until `clear()`.
Two tests cover it: an `UNINSTALLED` bundle is never asked for an entry, and
a bundle that throws mid-scan does not fail the lookup for the bundles after it.
##########
core/camel-core-osgi/src/main/java/org/apache/camel/karaf/core/OsgiFactoryFinder.java:
##########
@@ -79,23 +87,51 @@ public Optional<Class<?>> findClass(String key) {
// NOTE, the first found factory will be return
public BundleEntry getResource(String name) {
BundleEntry entry = null;
- Bundle[] bundles;
+ // only allocated when more than one bundle provides the same
descriptor, which is not the normal case
+ List<Bundle> alsoProviding = null;
- bundles = bundleContext.getBundles();
+ Bundle[] bundles = bundleContext.getBundles();
- URL url;
for (Bundle bundle : bundles) {
Review Comment:
Agreed, the `break` is back. Selection and scan cost are unchanged from
`main`.
Worth recording why it matters more than it looks: misses cache too
(`classesNotFound`), so without the `break` every key — hit or miss — costs a
full sweep of all installed bundles, ~300 in a typical Camel Karaf install.
--
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]
