cushon commented on code in PR #16604:
URL: https://github.com/apache/kafka/pull/16604#discussion_r1697156935
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/ReflectionScanner.java:
##########
@@ -77,53 +79,59 @@ private static <T> String versionFor(Class<? extends T>
pluginKlass) throws Refl
@Override
protected PluginScanResult scanPlugins(PluginSource source) {
- ConfigurationBuilder builder = new ConfigurationBuilder();
- builder.setClassLoaders(new ClassLoader[]{source.loader()});
- builder.addUrls(source.urls());
- builder.setScanners(Scanners.SubTypes);
- builder.setParallel(true);
- Reflections reflections = new Reflections(builder);
-
- return new PluginScanResult(
- getPluginDesc(reflections, PluginType.SINK, source),
- getPluginDesc(reflections, PluginType.SOURCE, source),
- getPluginDesc(reflections, PluginType.CONVERTER, source),
- getPluginDesc(reflections, PluginType.HEADER_CONVERTER,
source),
- getTransformationPluginDesc(source, reflections),
- getPredicatePluginDesc(source, reflections),
- getServiceLoaderPluginDesc(PluginType.CONFIGPROVIDER, source),
- getServiceLoaderPluginDesc(PluginType.REST_EXTENSION, source),
-
getServiceLoaderPluginDesc(PluginType.CONNECTOR_CLIENT_CONFIG_OVERRIDE_POLICY,
source)
- );
+ Set<URL> urls = new HashSet<>();
+ Collections.addAll(urls, source.urls());
+ ClassGraph classGraphBuilder = new ClassGraph()
+ .addClassLoader(source.loader())
+ .filterClasspathElementsByURL(urls::contains)
Review Comment:
> Is ClassGraph scanning all of the classes in the parent/classpath loader
too, requiring this filter?
I think it is including `java.class.path` in the scan. It needs to have
scanned classes in the type hierarchy to be able to find plugins that
transitively implement plugin interfaces, like the
`SubclassOfClasspathConverter` example in the tests. There is some related
discussion in https://github.com/classgraph/classgraph/issues/875.
So I'm not sure `filterClasspathElementsByURL` is optimizing anything, it
might be possible to omit that part.
> Is there a way to avoid that, because that means classpath scanning would
happen N times for each plugin location.
I think that a lot of the work processing each plugin source is being
duplicated. I had wondered if it might be possible to do one scan, instead of
one for each plugin source. That would improve performance when using
ClassGraph and avoid some of the duplicate work, but it would also require some
refactoring of how plugin sources are managed, and I'm not sure how widely used
that API is or how invasive a change that would be.
--
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]
