[
https://issues.apache.org/jira/browse/SOLR-972?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12667643#action_12667643
]
Shalin Shekhar Mangar commented on SOLR-972:
--------------------------------------------
bq. The rules about modification apply well enough for interfaces and abstract
classes. Having an abstract class limits the object hierarchy of the handler
unnecessary when it could be more flexible through the interfaces.
Not really, since you don't necessarily need to keep the new methods abstract,
they can be a no-op. Thus you maintain binary compatibility with old code. One
doesn't have this luxury with interfaces.
bq. Imagine a requesthandler in the Servlet having no state and being
instantiated every time for every request. By recreating EventListener-s , we
have a similar analogy. That would make (it already does and I am working on
patched version of Solr) development harder to share any state between
successive calls.
I don't think that is an apples to apples comparison. Servlets are supposed to
be called a lot many times per second. Also, containers may create many
instances of servlets.
bq. Use of the reflection API ( Class.forName() , Class.newInstance() ) in any
piece of code that is going to be executed frequently ( delta import in this
case ) is hardly the best way to scale. I can come up with a sample unit test
case for that - but I think it is moot.
I try to stay away from micro-optimizations (which is what I felt this issue
was about) as they don't add any value to the system. One wouldn't even be able
to see any measurable performance difference unless one calls delta-imports
hundreds of times per second. Even if someone did, the bottleneck would
probably be the database rather than creation of event listeners.
If this issue is more about maintaining state rather than the optimization, can
you see if SOLR-988 can solve your problem? Most objects in DIH are created per
import request and I'd like to keep it that way. I agree with Noble that
EventListener cannot be seen as an exception. There are enough inconsistencies
already and I'd like to cut them down rather than add one more exception.
> EventListener-s creation changed from a per request ( full / delta-imports)
> scenario to once through the lifetime of the DIH plugin.
> ------------------------------------------------------------------------------------------------------------------------------------
>
> Key: SOLR-972
> URL: https://issues.apache.org/jira/browse/SOLR-972
> Project: Solr
> Issue Type: Improvement
> Components: contrib - DataImportHandler
> Environment: Java 6, Tomcat 6
> Reporter: Kay Kay
> Fix For: 1.4
>
> Attachments: SOLR-972.patch
>
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> The EventListener plugin for notification of start / end import events
> (SOLR-938) creates an instance of EventListener before every notification.
> This has 2 drawbacks.
> * No state is stored between successive invocations of events as it is a new
> object
> * When writing plugins for delta imports - it is very inefficient to do a
> class loader lookup by reflection / instantiate an instance and call a method
> on the same.
> Attached patch has one EventListener through the lifetime of the DIH plugin .
> Also EventListener is changed to an interface rather than an abstract class
> for better decoupling (especially painful when the start/end eventlistener
> has an independent hierarchy by itself ).
> By default, a no-op listener is registered to avoid boiler plate code to
> check if there is a start / end listener specified. Efficient JRE impls
> should be able to optimize the no-op for minimum overhead compared to
> checking the reference for null and branching out.
> Specifying an onImportStart / onImportEnd overrides the default handler
> though.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
