]
Mario Fusco commented on DROOLS-483:
------------------------------------
Hi and sorry for the huge delay in dealing with this.
The improvement you're suggesting is ok in principle, but as you anticipated, it
doesn't work well in an OSGi environment and this is why we didn't merge your pull
request yet.
I believe this further issue could be fixable by making our OSGi Activators to recognize
this situation and deal with it, but this needs a bit of time to be done correctly.
That is why we chose the safest (and slowest) path for now. Anyway I think that what
you're proposing makes completely sense and I'll try to extend what you did to
make it work also on OSGi asap. You don't need to rebase your PR, I'll do it when
I'll have time to work on it.
Thanks for your help and patience,
Mario
Repeated class loading of absent services
-----------------------------------------
Key: DROOLS-483
URL:
https://issues.jboss.org/browse/DROOLS-483
Project: Drools
Issue Type: Enhancement
Security Level: Public(Everyone can see)
Affects Versions: 5.5.0.Final, 5.6.0.Final, 6.0.1.Final, 6.1.0.Final
Reporter: Frank Pavageau
Assignee: Mark Proctor
h3. Context
The {{ServiceRegistry}} is used to get the providers for a number of services, several of
which don't have an implementation in a Drools artifact but in jBPM, for example.
Those providers include:
- {{ProcessBuilderFactoryService}} (implemented by
{{org.jbpm.process.builder.ProcessBuilderFactoryServiceImpl}}) called in the constructor
of {{KnowledgeBuilderImpl}} through {{ProcessBuilderFactory}}
- {{ProcessRuntimeFactoryService}} (implemented by
{{org.jbpm.process.instance.ProcessRuntimeFactoryServiceImpl}}) called in the constructor
of {{StatefulKnowledgeSessionImpl}} through {{ProcessRuntimeFactory}}, so this one
especially can be called frequently
Both {{ProcessBuilderFactory}} and {{ProcessRuntimeFactory}} use the fact that the static
field containing the service is not null to know they've been initialized, which means
they'll continuously try to load the class if it's not actually in the classpath.
h3. Enhancement
If this behavior is not needed for the OSGi support, I'd like to see the
initialization test use a boolean field to only try loading the class once. And if it is
needed for OSGi, maybe the behavior could be configured.
h3. Rationale
Attempting to load a class is a costly operation, especially in the context of a web
application running on Tomcat. I've pushed a
[
JMH|http://openjdk.java.net/projects/code-tools/jmh/] benchmark on
[
Github|https://github.com/fpavageau/bench-for-name] to illustrate the point (see the
[
README.md|https://github.com/fpavageau/bench-for-name#results]): a failed call to
Class.forName() takes ~22 microseconds using the context classloader of the benchmark
(i.e. probably the system classloader), ~75 microseconds using a Tomcat classloader
configured ({{WebappClassLoader}}) with 100 empty jars (that need to be scanned for the
missing class). Combined with the fact that {{Class.forName()}} calls
{{ClassLoader.loadClass()}} which in the case of {{WebappClassLoader}} is a synchronized
method, it becomes a nice bottleneck in a high-traffic application creating
{{StatefulKnowledgeSessions}} extensively, because of the lock contention.
Another side-effect of this flow is that {{ServiceRegistryImpl}} continuously gets a
{{ClassNotFoundException}}, then throws an {{IllegalArgumentException}}, which means 2
stacktraces to fill for each call, and that is also an [expensive
operation|http://shipilev.net/blog/2014/exceptional-performance/] when throwing exceptions
is not exceptional and becomes the regular control flow.
I've been able to grab stacktraces from Drools to {{WebappClassLoader.loadClass()}}
when doing thread dumps on an application with sufficient load, so it's frequent
enough that it can be observed by sampling and not just a rhetorical question.
I could create a pull request, but I'd like to get some feedback first.
Note: most calls to {{ServiceRegistry.get()}} use the same pattern, but the 2 I have
picked out are the ones which happen the most frequently for my use case.