[jira] Created: (LUCENE-1693) AttributeSource/TokenStream API improvements

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733095#action_12733095 ]

Uwe Schindler commented on LUCENE-1693:
---------------------------------------

Yes, good idea. I added it locally, works good. This is also a speed improvement (the synchronized access to the cache is also removed by this).

I think a new patch is not needed, just add it to your checkout, too!

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead by default incrementToken() throws a subclass of
>   UnsupportedOperationException. The indexer tries to call
>   incrementToken() initially once to see if the exception is thrown;
>   if so, it falls back to the old API.
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> Also, the TokenStream API does not change, except for the removal
> of the set/getUseNewAPI methods. So the patches in LUCENE-1460
> should still work.
> All core tests pass, however, I need to update all the documentation
> and also add some unit tests for the new AttributeSource
> functionality. So this patch is not ready to commit yet, but I wanted
> to post it already for some feedback.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733096#action_12733096 ]

Michael Busch commented on LUCENE-1693:
---------------------------------------

yep, did already and all tests pass!

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead by default incrementToken() throws a subclass of
>   UnsupportedOperationException. The indexer tries to call
>   incrementToken() initially once to see if the exception is thrown;
>   if so, it falls back to the old API.
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> Also, the TokenStream API does not change, except for the removal
> of the set/getUseNewAPI methods. So the patches in LUCENE-1460
> should still work.
> All core tests pass, however, I need to update all the documentation
> and also add some unit tests for the new AttributeSource
> functionality. So this patch is not ready to commit yet, but I wanted
> to post it already for some feedback.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733580#action_12733580 ]

Michael McCandless commented on LUCENE-1693:
--------------------------------------------

OK I looked at the latest patch!  It looks good -- only these questions:

  * Are we going to keep Token.java or not?  (Current patch still has
    it deprecated).

  * Typo (occure) in CHANGES.txt

  * It looks like we are making all core tokenizers/filters final (to
    foreclose future back-compat problems if we need to change the
    API)?  If so, we should also make CachingTokenFilter final?

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead by default incrementToken() throws a subclass of
>   UnsupportedOperationException. The indexer tries to call
>   incrementToken() initially once to see if the exception is thrown;
>   if so, it falls back to the old API.
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> Also, the TokenStream API does not change, except for the removal
> of the set/getUseNewAPI methods. So the patches in LUCENE-1460
> should still work.
> All core tests pass, however, I need to update all the documentation
> and also add some unit tests for the new AttributeSource
> functionality. So this patch is not ready to commit yet, but I wanted
> to post it already for some feedback.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733604#action_12733604 ]

Grant Ingersoll commented on LUCENE-1693:
-----------------------------------------

One of the things that works really well in Solr is that any time some significant JIRA issue is undertaken, a Wiki page is also generated that effectively documents the ideas in the patch, as well as how to use it and thus results in the final page effectively becoming the documentation.  I know Mike and Uwe have done a ton of work on this, but would it be too much trouble to ask for a Wiki page that describes the current state of the patch?  It is really hard to follow, in JIRA, all the different threads and which ones are still valid and which are not.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead by default incrementToken() throws a subclass of
>   UnsupportedOperationException. The indexer tries to call
>   incrementToken() initially once to see if the exception is thrown;
>   if so, it falls back to the old API.
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> Also, the TokenStream API does not change, except for the removal
> of the set/getUseNewAPI methods. So the patches in LUCENE-1460
> should still work.
> All core tests pass, however, I need to update all the documentation
> and also add some unit tests for the new AttributeSource
> functionality. So this patch is not ready to commit yet, but I wanted
> to post it already for some feedback.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

[markrmiller]

bq. * Are we going to keep Token.java or not?  (Current patch still has it deprecated).

I need to know this as well - I have to make a new Token class for the Highlighter package if this one is deprecated. It would seem a convenience to keep it around.

On Tue, Jul 21, 2009 at 8:51 AM, Grant Ingersoll (JIRA) <jira@...> wrote:

   [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733604#action_12733604 ]

Grant Ingersoll commented on LUCENE-1693:
-----------------------------------------

One of the things that works really well in Solr is that any time some significant JIRA issue is undertaken, a Wiki page is also generated that effectively documents the ideas in the patch, as well as how to use it and thus results in the final page effectively becoming the documentation.  I know Mike and Uwe have done a ton of work on this, but would it be too much trouble to ask for a Wiki page that describes the current state of the patch?  It is really hard to follow, in JIRA, all the different threads and which ones are still valid and which are not.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead by default incrementToken() throws a subclass of
>   UnsupportedOperationException. The indexer tries to call
>   incrementToken() initially once to see if the exception is thrown;
>   if so, it falls back to the old API.
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> Also, the TokenStream API does not change, except for the removal
> of the set/getUseNewAPI methods. So the patches in LUCENE-1460
> should still work.
> All core tests pass, however, I need to update all the documentation
> and also add some unit tests for the new AttributeSource
> functionality. So this patch is not ready to commit yet, but I wanted
> to post it already for some feedback.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

--
--
- Mark

http://www.lucidimagination.com

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733761#action_12733761 ]

Uwe Schindler commented on LUCENE-1693:
---------------------------------------

Mark Miller on java-dev:

bq. * Are we going to keep Token.java or not?  (Current patch still has it deprecated).
I need to know this as well - I have to make a new Token class for the Highlighter package if this one is deprecated. It would seem a convenience to keep it around.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead by default incrementToken() throws a subclass of
>   UnsupportedOperationException. The indexer tries to call
>   incrementToken() initially once to see if the exception is thrown;
>   if so, it falls back to the old API.
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> Also, the TokenStream API does not change, except for the removal
> of the set/getUseNewAPI methods. So the patches in LUCENE-1460
> should still work.
> All core tests pass, however, I need to update all the documentation
> and also add some unit tests for the new AttributeSource
> functionality. So this patch is not ready to commit yet, but I wanted
> to post it already for some feedback.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733765#action_12733765 ]

Uwe Schindler commented on LUCENE-1693:
---------------------------------------

{quote}
bq. * Are we going to keep Token.java or not?  (Current patch still has it deprecated).
I need to know this as well - I have to make a new Token class for the Highlighter package if this one is deprecated. It would seem a convenience to keep it around.
{quote}

I would also keep it non-deprecated. It is useful also as a convenience class and can be used as one attribute instance for all 6 base attributes (the backwards compatibility layer of the current patch already does this).

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead by default incrementToken() throws a subclass of
>   UnsupportedOperationException. The indexer tries to call
>   incrementToken() initially once to see if the exception is thrown;
>   if so, it falls back to the old API.
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> Also, the TokenStream API does not change, except for the removal
> of the set/getUseNewAPI methods. So the patches in LUCENE-1460
> should still work.
> All core tests pass, however, I need to update all the documentation
> and also add some unit tests for the new AttributeSource
> functionality. So this patch is not ready to commit yet, but I wanted
> to post it already for some feedback.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Uwe Schindler updated LUCENE-1693:
----------------------------------

    Attachment: LUCENE-1693.patch

Updated patch that has the following changes:
- Updated QueryParser.jj instead of .java and rebuilt the parser
- added final to incrementToken() in CachingTokenFilter
- added Michael's AttributeSource improvement

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead by default incrementToken() throws a subclass of
>   UnsupportedOperationException. The indexer tries to call
>   incrementToken() initially once to see if the exception is thrown;
>   if so, it falls back to the old API.
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> Also, the TokenStream API does not change, except for the removal
> of the set/getUseNewAPI methods. So the patches in LUCENE-1460
> should still work.
> All core tests pass, however, I need to update all the documentation
> and also add some unit tests for the new AttributeSource
> functionality. So this patch is not ready to commit yet, but I wanted
> to post it already for some feedback.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733857#action_12733857 ]

Michael Busch commented on LUCENE-1693:
---------------------------------------

I agree  - we should keep Token and not deprecate it.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead by default incrementToken() throws a subclass of
>   UnsupportedOperationException. The indexer tries to call
>   incrementToken() initially once to see if the exception is thrown;
>   if so, it falls back to the old API.
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> Also, the TokenStream API does not change, except for the removal
> of the set/getUseNewAPI methods. So the patches in LUCENE-1460
> should still work.
> All core tests pass, however, I need to update all the documentation
> and also add some unit tests for the new AttributeSource
> functionality. So this patch is not ready to commit yet, but I wanted
> to post it already for some feedback.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael Busch updated LUCENE-1693:
----------------------------------

    Attachment: lucene-1693.patch

Uwe's latest patch with these changes:

- Token is not deprecated anymore.
- Updated the examples in the 'New TokenStream API" section in package.html (analysis package) to reflect the changes this patch introduces (Attribute vs. AttributeImpl.)
- All new files have svn:eol-style=native set now.

test-core, test-contrib and test-tag all pass. I think this is ready to commit now.

We might want to improve some javadocs a bit, especially the "expert" APIs, such as AttributeFactory. Also  another section about how to improve caching performance would be good. But I think we should open a separate issue for the doc improvements to get the other related issues going. The main APIs are pretty well documented I think.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead by default incrementToken() throws a subclass of
>   UnsupportedOperationException. The indexer tries to call
>   incrementToken() initially once to see if the exception is thrown;
>   if so, it falls back to the old API.
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> Also, the TokenStream API does not change, except for the removal
> of the set/getUseNewAPI methods. So the patches in LUCENE-1460
> should still work.
> All core tests pass, however, I need to update all the documentation
> and also add some unit tests for the new AttributeSource
> functionality. So this patch is not ready to commit yet, but I wanted
> to post it already for some feedback.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12733931#action_12733931 ]

Michael Busch commented on LUCENE-1693:
---------------------------------------

I think the points listed in the description of this issue are still mostly valid. Uwe, do you want to add the changes you made in addition?

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead by default incrementToken() throws a subclass of
>   UnsupportedOperationException. The indexer tries to call
>   incrementToken() initially once to see if the exception is thrown;
>   if so, it falls back to the old API.
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> Also, the TokenStream API does not change, except for the removal
> of the set/getUseNewAPI methods. So the patches in LUCENE-1460
> should still work.
> All core tests pass, however, I need to update all the documentation
> and also add some unit tests for the new AttributeSource
> functionality. So this patch is not ready to commit yet, but I wanted
> to post it already for some feedback.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Uwe Schindler updated LUCENE-1693:
----------------------------------

    Description:
This patch makes the following improvements to AttributeSource and
TokenStream/Filter:

- introduces interfaces for all Attributes. The corresponding
  implementations have the postfix 'Impl', e.g. TermAttribute and
  TermAttributeImpl. AttributeSource now has a factory for creating
  the Attribute instances; the default implementation looks for
  implementing classes with the postfix 'Impl'. Token now implements
  all 6 TokenAttribute interfaces.

- new method added to AttributeSource:
  addAttributeImpl(AttributeImpl). Using reflection it walks up in the
  class hierarchy of the passed in object and finds all interfaces
  that the class or superclasses implement and that extend the
  Attribute interface. It then adds the interface->instance mappings
  to the attribute map for each of the found interfaces.

- removes the set/getUseNewAPI() methods (including the standard
  ones). Instead it is now enough to only implement the new API, if one old TokenStream implements still the old API (next()/next(Token)), it is wrapped automatically. The delegation path is determined via reflection (the patch determines, which of the three methods was overridden).

- Token is no longer deprecated, instead it implements all 6 standard token interfaces (see above). The wrapper for next() and next(Token) uses this, to automatically map all attribute interfaces to one TokenWrapper instance (implementing all 6 interfaces), that contains a Token instance. next() and next(Token) exchange the inner Token instance as needed. For the new incrementToken(), only one TokenWrapper instance is visible, delegating to the currect reusable Token. This API also preserves custom Token subclasses, that maybe created by very special token streams (see example in Backwards-Test).

- AttributeImpl now has a default implementation of toString that uses
  reflection to print out the values of the attributes in a default
  formatting. This makes it a bit easier to implement AttributeImpl,
  because toString() was declared abstract before.

- Cloning is now done much more efficiently in
  captureState. The method figures out which unique AttributeImpl
  instances are contained as values in the attributes map, because
  those are the ones that need to be cloned. It creates a single
  linked list that supports deep cloning (in the inner class
  AttributeSource.State). AttributeSource keeps track of when this
  state changes, i.e. whenever new attributes are added to the
  AttributeSource. Only in that case will captureState recompute the
  state, otherwise it will simply clone the precomputed state and
  return the clone. restoreState(AttributeSource.State) walks the
  linked list and uses the copyTo() method of AttributeImpl to copy
  all values over into the attribute that the source stream
  (e.g. SinkTokenizer) uses.

The cloning performance can be greatly improved if not multiple
AttributeImpl instances are used in one TokenStream. A user can
e.g. simply add a Token instance to the stream instead of the individual
attributes. Or the user could implement a subclass of AttributeImpl that
implements exactly the Attribute interfaces needed. I think this
should be considered an expert API (addAttributeImpl), as this manual
optimization is only needed if cloning performance is crucial. I ran
some quick performance tests using Tee/Sink tokenizers (which do
cloning) and the performance was roughly 20% faster with the new
API. I'll run some more performance tests and post more numbers then.

Note also that when we add serialization to the Attributes, e.g. for
supporting storing serialized TokenStreams in the index, then the
serialization should benefit even significantly more from the new API
than cloning.

This issue contains one backwards-compatibility break:
TokenStreams/Filters/Tokenizers should normally be final (see LUCENE-1753 for the explaination). Some of these core classes are not final and so one could override the next() or next(Token) methods. In this case, the backwards-wrapper would automatically use incrementToken(), because it is implemented, so the overridden method is never called. To prevent users from errors not visible during compilation or testing (the streams just behave wrong), this patch makes all implementation methods final (next(), next(Token), incrementToken()), whenever the class itsself is not final. This is a BW break, but users will clearly see, that they have done something unsupoorted and should better create a custom TokenFilter with their additional implementation (instead of extending a core implementation).

For further changing contrib token streams the following procedere should be used:

    *  rewrite and replace next(Token)/next() implementations by new API
    * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
    * if the class is non-final add the following methods to the class:
{code:java}
      /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
       * not be overridden. Delegates to the backwards compatibility layer. */
      public final Token next(final Token reusableToken) throws java.io.IOException {
        return super.next(reusableToken);
      }

      /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
       * not be overridden. Delegates to the backwards compatibility layer. */
      public final Token next() throws java.io.IOException {
        return super.next();
      }
{code}
 Also the incrementToken() method must be final in this case (and the new method end() of LUCENE-1448)


  was:
This patch makes the following improvements to AttributeSource and
TokenStream/Filter:

- removes the set/getUseNewAPI() methods (including the standard
  ones). Instead by default incrementToken() throws a subclass of
  UnsupportedOperationException. The indexer tries to call
  incrementToken() initially once to see if the exception is thrown;
  if so, it falls back to the old API.

- introduces interfaces for all Attributes. The corresponding
  implementations have the postfix 'Impl', e.g. TermAttribute and
  TermAttributeImpl. AttributeSource now has a factory for creating
  the Attribute instances; the default implementation looks for
  implementing classes with the postfix 'Impl'. Token now implements
  all 6 TokenAttribute interfaces.

- new method added to AttributeSource:
  addAttributeImpl(AttributeImpl). Using reflection it walks up in the
  class hierarchy of the passed in object and finds all interfaces
  that the class or superclasses implement and that extend the
  Attribute interface. It then adds the interface->instance mappings
  to the attribute map for each of the found interfaces.

- AttributeImpl now has a default implementation of toString that uses
  reflection to print out the values of the attributes in a default
  formatting. This makes it a bit easier to implement AttributeImpl,
  because toString() was declared abstract before.

- Cloning is now done much more efficiently in
  captureState. The method figures out which unique AttributeImpl
  instances are contained as values in the attributes map, because
  those are the ones that need to be cloned. It creates a single
  linked list that supports deep cloning (in the inner class
  AttributeSource.State). AttributeSource keeps track of when this
  state changes, i.e. whenever new attributes are added to the
  AttributeSource. Only in that case will captureState recompute the
  state, otherwise it will simply clone the precomputed state and
  return the clone. restoreState(AttributeSource.State) walks the
  linked list and uses the copyTo() method of AttributeImpl to copy
  all values over into the attribute that the source stream
  (e.g. SinkTokenizer) uses.

The cloning performance can be greatly improved if not multiple
AttributeImpl instances are used in one TokenStream. A user can
e.g. simply add a Token instance to the stream instead of the individual
attributes. Or the user could implement a subclass of AttributeImpl that
implements exactly the Attribute interfaces needed. I think this
should be considered an expert API (addAttributeImpl), as this manual
optimization is only needed if cloning performance is crucial. I ran
some quick performance tests using Tee/Sink tokenizers (which do
cloning) and the performance was roughly 20% faster with the new
API. I'll run some more performance tests and post more numbers then.

Note also that when we add serialization to the Attributes, e.g. for
supporting storing serialized TokenStreams in the index, then the
serialization should benefit even significantly more from the new API
than cloning.

Also, the TokenStream API does not change, except for the removal
of the set/getUseNewAPI methods. So the patches in LUCENE-1460
should still work.

All core tests pass, however, I need to update all the documentation
and also add some unit tests for the new AttributeSource
functionality. So this patch is not ready to commit yet, but I wanted
to post it already for some feedback.


Updated issue description/summary to reflect last state.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead it is now enough to only implement the new API, if one old TokenStream implements still the old API (next()/next(Token)), it is wrapped automatically. The delegation path is determined via reflection (the patch determines, which of the three methods was overridden).
> - Token is no longer deprecated, instead it implements all 6 standard token interfaces (see above). The wrapper for next() and next(Token) uses this, to automatically map all attribute interfaces to one TokenWrapper instance (implementing all 6 interfaces), that contains a Token instance. next() and next(Token) exchange the inner Token instance as needed. For the new incrementToken(), only one TokenWrapper instance is visible, delegating to the currect reusable Token. This API also preserves custom Token subclasses, that maybe created by very special token streams (see example in Backwards-Test).
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> This issue contains one backwards-compatibility break:
> TokenStreams/Filters/Tokenizers should normally be final (see LUCENE-1753 for the explaination). Some of these core classes are not final and so one could override the next() or next(Token) methods. In this case, the backwards-wrapper would automatically use incrementToken(), because it is implemented, so the overridden method is never called. To prevent users from errors not visible during compilation or testing (the streams just behave wrong), this patch makes all implementation methods final (next(), next(Token), incrementToken()), whenever the class itsself is not final. This is a BW break, but users will clearly see, that they have done something unsupoorted and should better create a custom TokenFilter with their additional implementation (instead of extending a core implementation).
> For further changing contrib token streams the following procedere should be used:
>     *  rewrite and replace next(Token)/next() implementations by new API
>     * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
>     * if the class is non-final add the following methods to the class:
> {code:java}
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next(final Token reusableToken) throws java.io.IOException {
>         return super.next(reusableToken);
>       }
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next() throws java.io.IOException {
>         return super.next();
>       }
> {code}
>  Also the incrementToken() method must be final in this case (and the new method end() of LUCENE-1448)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

     [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Uwe Schindler updated LUCENE-1693:
----------------------------------

    Description:
This patch makes the following improvements to AttributeSource and
TokenStream/Filter:

- introduces interfaces for all Attributes. The corresponding
  implementations have the postfix 'Impl', e.g. TermAttribute and
  TermAttributeImpl. AttributeSource now has a factory for creating
  the Attribute instances; the default implementation looks for
  implementing classes with the postfix 'Impl'. Token now implements
  all 6 TokenAttribute interfaces.

- new method added to AttributeSource:
  addAttributeImpl(AttributeImpl). Using reflection it walks up in the
  class hierarchy of the passed in object and finds all interfaces
  that the class or superclasses implement and that extend the
  Attribute interface. It then adds the interface->instance mappings
  to the attribute map for each of the found interfaces.

- removes the set/getUseNewAPI() methods (including the standard
  ones). Instead it is now enough to only implement the new API,
  if one old TokenStream implements still the old API (next()/next(Token)),
  it is wrapped automatically. The delegation path is determined via
  reflection (the patch determines, which of the three methods was
  overridden).

- Token is no longer deprecated, instead it implements all 6 standard
  token interfaces (see above). The wrapper for next() and next(Token)
  uses this, to automatically map all attribute interfaces to one
  TokenWrapper instance (implementing all 6 interfaces), that contains
  a Token instance. next() and next(Token) exchange the inner Token
  instance as needed. For the new incrementToken(), only one
  TokenWrapper instance is visible, delegating to the currect reusable
  Token. This API also preserves custom Token subclasses, that maybe
  created by very special token streams (see example in Backwards-Test).

- AttributeImpl now has a default implementation of toString that uses
  reflection to print out the values of the attributes in a default
  formatting. This makes it a bit easier to implement AttributeImpl,
  because toString() was declared abstract before.

- Cloning is now done much more efficiently in
  captureState. The method figures out which unique AttributeImpl
  instances are contained as values in the attributes map, because
  those are the ones that need to be cloned. It creates a single
  linked list that supports deep cloning (in the inner class
  AttributeSource.State). AttributeSource keeps track of when this
  state changes, i.e. whenever new attributes are added to the
  AttributeSource. Only in that case will captureState recompute the
  state, otherwise it will simply clone the precomputed state and
  return the clone. restoreState(AttributeSource.State) walks the
  linked list and uses the copyTo() method of AttributeImpl to copy
  all values over into the attribute that the source stream
  (e.g. SinkTokenizer) uses.

- Tee- and SinkTokenizer were deprecated, because they use
Token instances for caching. This is not compatible to the new API
using AttributeSource.State objects. You can still use the old
deprecated ones, but new features provided by new Attribute types
may get lost in the chain. A replacement is a new TeeSinkTokenFilter,
which has a factory to create new Sink instances, that have compatible
attributes. Sink instances created by one Tee can also be added to
another Tee, as long as the attribute implementations are compatible
(it is not possible to add a sink from a tee using one Token instance
to a tee using the six separate attribute impls). In this case UOE is thrown.

The cloning performance can be greatly improved if not multiple
AttributeImpl instances are used in one TokenStream. A user can
e.g. simply add a Token instance to the stream instead of the individual
attributes. Or the user could implement a subclass of AttributeImpl that
implements exactly the Attribute interfaces needed. I think this
should be considered an expert API (addAttributeImpl), as this manual
optimization is only needed if cloning performance is crucial. I ran
some quick performance tests using Tee/Sink tokenizers (which do
cloning) and the performance was roughly 20% faster with the new
API. I'll run some more performance tests and post more numbers then.

Note also that when we add serialization to the Attributes, e.g. for
supporting storing serialized TokenStreams in the index, then the
serialization should benefit even significantly more from the new API
than cloning.

This issue contains one backwards-compatibility break:
TokenStreams/Filters/Tokenizers should normally be final
(see LUCENE-1753 for the explaination). Some of these core classes are
not final and so one could override the next() or next(Token) methods.
In this case, the backwards-wrapper would automatically use
incrementToken(), because it is implemented, so the overridden
method is never called. To prevent users from errors not visible
during compilation or testing (the streams just behave wrong),
this patch makes all implementation methods final
(next(), next(Token), incrementToken()), whenever the class
itsself is not final. This is a BW break, but users will clearly see,
that they have done something unsupoorted and should better
create a custom TokenFilter with their additional implementation
(instead of extending a core implementation).

For further changing contrib token streams the following procedere should be used:

    *  rewrite and replace next(Token)/next() implementations by new API
    * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
    * if the class is non-final add the following methods to the class:
{code:java}
      /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
       * not be overridden. Delegates to the backwards compatibility layer. */
      public final Token next(final Token reusableToken) throws java.io.IOException {
        return super.next(reusableToken);
      }

      /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
       * not be overridden. Delegates to the backwards compatibility layer. */
      public final Token next() throws java.io.IOException {
        return super.next();
      }
{code}
Also the incrementToken() method must be final in this case
(and the new method end() of LUCENE-1448)


  was:
This patch makes the following improvements to AttributeSource and
TokenStream/Filter:

- introduces interfaces for all Attributes. The corresponding
  implementations have the postfix 'Impl', e.g. TermAttribute and
  TermAttributeImpl. AttributeSource now has a factory for creating
  the Attribute instances; the default implementation looks for
  implementing classes with the postfix 'Impl'. Token now implements
  all 6 TokenAttribute interfaces.

- new method added to AttributeSource:
  addAttributeImpl(AttributeImpl). Using reflection it walks up in the
  class hierarchy of the passed in object and finds all interfaces
  that the class or superclasses implement and that extend the
  Attribute interface. It then adds the interface->instance mappings
  to the attribute map for each of the found interfaces.

- removes the set/getUseNewAPI() methods (including the standard
  ones). Instead it is now enough to only implement the new API, if one old TokenStream implements still the old API (next()/next(Token)), it is wrapped automatically. The delegation path is determined via reflection (the patch determines, which of the three methods was overridden).

- Token is no longer deprecated, instead it implements all 6 standard token interfaces (see above). The wrapper for next() and next(Token) uses this, to automatically map all attribute interfaces to one TokenWrapper instance (implementing all 6 interfaces), that contains a Token instance. next() and next(Token) exchange the inner Token instance as needed. For the new incrementToken(), only one TokenWrapper instance is visible, delegating to the currect reusable Token. This API also preserves custom Token subclasses, that maybe created by very special token streams (see example in Backwards-Test).

- AttributeImpl now has a default implementation of toString that uses
  reflection to print out the values of the attributes in a default
  formatting. This makes it a bit easier to implement AttributeImpl,
  because toString() was declared abstract before.

- Cloning is now done much more efficiently in
  captureState. The method figures out which unique AttributeImpl
  instances are contained as values in the attributes map, because
  those are the ones that need to be cloned. It creates a single
  linked list that supports deep cloning (in the inner class
  AttributeSource.State). AttributeSource keeps track of when this
  state changes, i.e. whenever new attributes are added to the
  AttributeSource. Only in that case will captureState recompute the
  state, otherwise it will simply clone the precomputed state and
  return the clone. restoreState(AttributeSource.State) walks the
  linked list and uses the copyTo() method of AttributeImpl to copy
  all values over into the attribute that the source stream
  (e.g. SinkTokenizer) uses.

The cloning performance can be greatly improved if not multiple
AttributeImpl instances are used in one TokenStream. A user can
e.g. simply add a Token instance to the stream instead of the individual
attributes. Or the user could implement a subclass of AttributeImpl that
implements exactly the Attribute interfaces needed. I think this
should be considered an expert API (addAttributeImpl), as this manual
optimization is only needed if cloning performance is crucial. I ran
some quick performance tests using Tee/Sink tokenizers (which do
cloning) and the performance was roughly 20% faster with the new
API. I'll run some more performance tests and post more numbers then.

Note also that when we add serialization to the Attributes, e.g. for
supporting storing serialized TokenStreams in the index, then the
serialization should benefit even significantly more from the new API
than cloning.

This issue contains one backwards-compatibility break:
TokenStreams/Filters/Tokenizers should normally be final (see LUCENE-1753 for the explaination). Some of these core classes are not final and so one could override the next() or next(Token) methods. In this case, the backwards-wrapper would automatically use incrementToken(), because it is implemented, so the overridden method is never called. To prevent users from errors not visible during compilation or testing (the streams just behave wrong), this patch makes all implementation methods final (next(), next(Token), incrementToken()), whenever the class itsself is not final. This is a BW break, but users will clearly see, that they have done something unsupoorted and should better create a custom TokenFilter with their additional implementation (instead of extending a core implementation).

For further changing contrib token streams the following procedere should be used:

    *  rewrite and replace next(Token)/next() implementations by new API
    * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
    * if the class is non-final add the following methods to the class:
{code:java}
      /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
       * not be overridden. Delegates to the backwards compatibility layer. */
      public final Token next(final Token reusableToken) throws java.io.IOException {
        return super.next(reusableToken);
      }

      /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
       * not be overridden. Delegates to the backwards compatibility layer. */
      public final Token next() throws java.io.IOException {
        return super.next();
      }
{code}
 Also the incrementToken() method must be final in this case (and the new method end() of LUCENE-1448)



Forgot the new TeeSinkTokenFilter and reformatted the text to use NLs.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead it is now enough to only implement the new API,
>   if one old TokenStream implements still the old API (next()/next(Token)),
>   it is wrapped automatically. The delegation path is determined via
>   reflection (the patch determines, which of the three methods was
>   overridden).
> - Token is no longer deprecated, instead it implements all 6 standard
>   token interfaces (see above). The wrapper for next() and next(Token)
>   uses this, to automatically map all attribute interfaces to one
>   TokenWrapper instance (implementing all 6 interfaces), that contains
>   a Token instance. next() and next(Token) exchange the inner Token
>   instance as needed. For the new incrementToken(), only one
>   TokenWrapper instance is visible, delegating to the currect reusable
>   Token. This API also preserves custom Token subclasses, that maybe
>   created by very special token streams (see example in Backwards-Test).
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> - Tee- and SinkTokenizer were deprecated, because they use
> Token instances for caching. This is not compatible to the new API
> using AttributeSource.State objects. You can still use the old
> deprecated ones, but new features provided by new Attribute types
> may get lost in the chain. A replacement is a new TeeSinkTokenFilter,
> which has a factory to create new Sink instances, that have compatible
> attributes. Sink instances created by one Tee can also be added to
> another Tee, as long as the attribute implementations are compatible
> (it is not possible to add a sink from a tee using one Token instance
> to a tee using the six separate attribute impls). In this case UOE is thrown.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> This issue contains one backwards-compatibility break:
> TokenStreams/Filters/Tokenizers should normally be final
> (see LUCENE-1753 for the explaination). Some of these core classes are
> not final and so one could override the next() or next(Token) methods.
> In this case, the backwards-wrapper would automatically use
> incrementToken(), because it is implemented, so the overridden
> method is never called. To prevent users from errors not visible
> during compilation or testing (the streams just behave wrong),
> this patch makes all implementation methods final
> (next(), next(Token), incrementToken()), whenever the class
> itsself is not final. This is a BW break, but users will clearly see,
> that they have done something unsupoorted and should better
> create a custom TokenFilter with their additional implementation
> (instead of extending a core implementation).
> For further changing contrib token streams the following procedere should be used:
>     *  rewrite and replace next(Token)/next() implementations by new API
>     * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
>     * if the class is non-final add the following methods to the class:
> {code:java}
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next(final Token reusableToken) throws java.io.IOException {
>         return super.next(reusableToken);
>       }
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next() throws java.io.IOException {
>         return super.next();
>       }
> {code}
> Also the incrementToken() method must be final in this case
> (and the new method end() of LUCENE-1448)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12734274#action_12734274 ]

Dave Been commented on LUCENE-1693:
-----------------------------------

My first post to the list, it appears i should comment here in the JIRA, not reply to email, apologize if i did this wrong:

I've been following this AttributeSource/TokenStream patch thread and reviewing the changes/backwards compatibility issues and the changes.  
extremely interesting problem/solution.

while looking at Uwe's PerfTest3 I noticed an unused allocation in the last run for "reused stream new API only"

     for (int i = 0; i < c; i++) {
        if (i==1000) t = System.currentTimeMillis();
        tz.reset(new StringReader(text));
        // Token reusableToken=new Token();   <<<<     This one
        int num=0;
        while (tok.incrementToken()) {
          num++;
        }
      }


just a small cost, but makes the new reusable api slightly faster

With extra alloc:

Time for 100000 runs with new instances (old API): 12.75s
Time for 100000 runs with reused stream (old API): 9.969s
Time for 100000 runs with new instances (new API only): 13.969s
Time for 100000 runs with reused stream (new API only): 11.735s  <<<<<<<<<<<<<<<<<<<<<<<<<<


Without extra alloc (changes only the last line's time):

Time for 100000 runs with new instances (old API): 12.593s
Time for 100000 runs with reused stream (old API): 9.578s
Time for 100000 runs with new instances (new API only): 13.75s
Time for 100000 runs with reused stream (new API only): 11.453s     <<<<<<<<<<<<<<<<<<<<<<<<<<



dave

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead it is now enough to only implement the new API,
>   if one old TokenStream implements still the old API (next()/next(Token)),
>   it is wrapped automatically. The delegation path is determined via
>   reflection (the patch determines, which of the three methods was
>   overridden).
> - Token is no longer deprecated, instead it implements all 6 standard
>   token interfaces (see above). The wrapper for next() and next(Token)
>   uses this, to automatically map all attribute interfaces to one
>   TokenWrapper instance (implementing all 6 interfaces), that contains
>   a Token instance. next() and next(Token) exchange the inner Token
>   instance as needed. For the new incrementToken(), only one
>   TokenWrapper instance is visible, delegating to the currect reusable
>   Token. This API also preserves custom Token subclasses, that maybe
>   created by very special token streams (see example in Backwards-Test).
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> - Tee- and SinkTokenizer were deprecated, because they use
> Token instances for caching. This is not compatible to the new API
> using AttributeSource.State objects. You can still use the old
> deprecated ones, but new features provided by new Attribute types
> may get lost in the chain. A replacement is a new TeeSinkTokenFilter,
> which has a factory to create new Sink instances, that have compatible
> attributes. Sink instances created by one Tee can also be added to
> another Tee, as long as the attribute implementations are compatible
> (it is not possible to add a sink from a tee using one Token instance
> to a tee using the six separate attribute impls). In this case UOE is thrown.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> This issue contains one backwards-compatibility break:
> TokenStreams/Filters/Tokenizers should normally be final
> (see LUCENE-1753 for the explaination). Some of these core classes are
> not final and so one could override the next() or next(Token) methods.
> In this case, the backwards-wrapper would automatically use
> incrementToken(), because it is implemented, so the overridden
> method is never called. To prevent users from errors not visible
> during compilation or testing (the streams just behave wrong),
> this patch makes all implementation methods final
> (next(), next(Token), incrementToken()), whenever the class
> itsself is not final. This is a BW break, but users will clearly see,
> that they have done something unsupoorted and should better
> create a custom TokenFilter with their additional implementation
> (instead of extending a core implementation).
> For further changing contrib token streams the following procedere should be used:
>     *  rewrite and replace next(Token)/next() implementations by new API
>     * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
>     * if the class is non-final add the following methods to the class:
> {code:java}
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next(final Token reusableToken) throws java.io.IOException {
>         return super.next(reusableToken);
>       }
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next() throws java.io.IOException {
>         return super.next();
>       }
> {code}
> Also the incrementToken() method must be final in this case
> (and the new method end() of LUCENE-1448)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12734289#action_12734289 ]

Michael Busch commented on LUCENE-1693:
---------------------------------------

Thanks, Dave... I'll remove that unused allocation before committing.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead it is now enough to only implement the new API,
>   if one old TokenStream implements still the old API (next()/next(Token)),
>   it is wrapped automatically. The delegation path is determined via
>   reflection (the patch determines, which of the three methods was
>   overridden).
> - Token is no longer deprecated, instead it implements all 6 standard
>   token interfaces (see above). The wrapper for next() and next(Token)
>   uses this, to automatically map all attribute interfaces to one
>   TokenWrapper instance (implementing all 6 interfaces), that contains
>   a Token instance. next() and next(Token) exchange the inner Token
>   instance as needed. For the new incrementToken(), only one
>   TokenWrapper instance is visible, delegating to the currect reusable
>   Token. This API also preserves custom Token subclasses, that maybe
>   created by very special token streams (see example in Backwards-Test).
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> - Tee- and SinkTokenizer were deprecated, because they use
> Token instances for caching. This is not compatible to the new API
> using AttributeSource.State objects. You can still use the old
> deprecated ones, but new features provided by new Attribute types
> may get lost in the chain. A replacement is a new TeeSinkTokenFilter,
> which has a factory to create new Sink instances, that have compatible
> attributes. Sink instances created by one Tee can also be added to
> another Tee, as long as the attribute implementations are compatible
> (it is not possible to add a sink from a tee using one Token instance
> to a tee using the six separate attribute impls). In this case UOE is thrown.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> This issue contains one backwards-compatibility break:
> TokenStreams/Filters/Tokenizers should normally be final
> (see LUCENE-1753 for the explaination). Some of these core classes are
> not final and so one could override the next() or next(Token) methods.
> In this case, the backwards-wrapper would automatically use
> incrementToken(), because it is implemented, so the overridden
> method is never called. To prevent users from errors not visible
> during compilation or testing (the streams just behave wrong),
> this patch makes all implementation methods final
> (next(), next(Token), incrementToken()), whenever the class
> itsself is not final. This is a BW break, but users will clearly see,
> that they have done something unsupoorted and should better
> create a custom TokenFilter with their additional implementation
> (instead of extending a core implementation).
> For further changing contrib token streams the following procedere should be used:
>     *  rewrite and replace next(Token)/next() implementations by new API
>     * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
>     * if the class is non-final add the following methods to the class:
> {code:java}
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next(final Token reusableToken) throws java.io.IOException {
>         return super.next(reusableToken);
>       }
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next() throws java.io.IOException {
>         return super.next();
>       }
> {code}
> Also the incrementToken() method must be final in this case
> (and the new method end() of LUCENE-1448)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12734291#action_12734291 ]

Michael Busch commented on LUCENE-1693:
---------------------------------------

OK, I think we're finally ready to commit here!

I'll wait until Friday - if nobody objects until then, I will commit the latest patch.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead it is now enough to only implement the new API,
>   if one old TokenStream implements still the old API (next()/next(Token)),
>   it is wrapped automatically. The delegation path is determined via
>   reflection (the patch determines, which of the three methods was
>   overridden).
> - Token is no longer deprecated, instead it implements all 6 standard
>   token interfaces (see above). The wrapper for next() and next(Token)
>   uses this, to automatically map all attribute interfaces to one
>   TokenWrapper instance (implementing all 6 interfaces), that contains
>   a Token instance. next() and next(Token) exchange the inner Token
>   instance as needed. For the new incrementToken(), only one
>   TokenWrapper instance is visible, delegating to the currect reusable
>   Token. This API also preserves custom Token subclasses, that maybe
>   created by very special token streams (see example in Backwards-Test).
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> - Tee- and SinkTokenizer were deprecated, because they use
> Token instances for caching. This is not compatible to the new API
> using AttributeSource.State objects. You can still use the old
> deprecated ones, but new features provided by new Attribute types
> may get lost in the chain. A replacement is a new TeeSinkTokenFilter,
> which has a factory to create new Sink instances, that have compatible
> attributes. Sink instances created by one Tee can also be added to
> another Tee, as long as the attribute implementations are compatible
> (it is not possible to add a sink from a tee using one Token instance
> to a tee using the six separate attribute impls). In this case UOE is thrown.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> This issue contains one backwards-compatibility break:
> TokenStreams/Filters/Tokenizers should normally be final
> (see LUCENE-1753 for the explaination). Some of these core classes are
> not final and so one could override the next() or next(Token) methods.
> In this case, the backwards-wrapper would automatically use
> incrementToken(), because it is implemented, so the overridden
> method is never called. To prevent users from errors not visible
> during compilation or testing (the streams just behave wrong),
> this patch makes all implementation methods final
> (next(), next(Token), incrementToken()), whenever the class
> itsself is not final. This is a BW break, but users will clearly see,
> that they have done something unsupoorted and should better
> create a custom TokenFilter with their additional implementation
> (instead of extending a core implementation).
> For further changing contrib token streams the following procedere should be used:
>     *  rewrite and replace next(Token)/next() implementations by new API
>     * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
>     * if the class is non-final add the following methods to the class:
> {code:java}
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next(final Token reusableToken) throws java.io.IOException {
>         return super.next(reusableToken);
>       }
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next() throws java.io.IOException {
>         return super.next();
>       }
> {code}
> Also the incrementToken() method must be final in this case
> (and the new method end() of LUCENE-1448)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12734867#action_12734867 ]

Grant Ingersoll commented on LUCENE-1693:
-----------------------------------------

Checking now.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead it is now enough to only implement the new API,
>   if one old TokenStream implements still the old API (next()/next(Token)),
>   it is wrapped automatically. The delegation path is determined via
>   reflection (the patch determines, which of the three methods was
>   overridden).
> - Token is no longer deprecated, instead it implements all 6 standard
>   token interfaces (see above). The wrapper for next() and next(Token)
>   uses this, to automatically map all attribute interfaces to one
>   TokenWrapper instance (implementing all 6 interfaces), that contains
>   a Token instance. next() and next(Token) exchange the inner Token
>   instance as needed. For the new incrementToken(), only one
>   TokenWrapper instance is visible, delegating to the currect reusable
>   Token. This API also preserves custom Token subclasses, that maybe
>   created by very special token streams (see example in Backwards-Test).
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> - Tee- and SinkTokenizer were deprecated, because they use
> Token instances for caching. This is not compatible to the new API
> using AttributeSource.State objects. You can still use the old
> deprecated ones, but new features provided by new Attribute types
> may get lost in the chain. A replacement is a new TeeSinkTokenFilter,
> which has a factory to create new Sink instances, that have compatible
> attributes. Sink instances created by one Tee can also be added to
> another Tee, as long as the attribute implementations are compatible
> (it is not possible to add a sink from a tee using one Token instance
> to a tee using the six separate attribute impls). In this case UOE is thrown.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> This issue contains one backwards-compatibility break:
> TokenStreams/Filters/Tokenizers should normally be final
> (see LUCENE-1753 for the explaination). Some of these core classes are
> not final and so one could override the next() or next(Token) methods.
> In this case, the backwards-wrapper would automatically use
> incrementToken(), because it is implemented, so the overridden
> method is never called. To prevent users from errors not visible
> during compilation or testing (the streams just behave wrong),
> this patch makes all implementation methods final
> (next(), next(Token), incrementToken()), whenever the class
> itsself is not final. This is a BW break, but users will clearly see,
> that they have done something unsupoorted and should better
> create a custom TokenFilter with their additional implementation
> (instead of extending a core implementation).
> For further changing contrib token streams the following procedere should be used:
>     *  rewrite and replace next(Token)/next() implementations by new API
>     * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
>     * if the class is non-final add the following methods to the class:
> {code:java}
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next(final Token reusableToken) throws java.io.IOException {
>         return super.next(reusableToken);
>       }
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next() throws java.io.IOException {
>         return super.next();
>       }
> {code}
> Also the incrementToken() method must be final in this case
> (and the new method end() of LUCENE-1448)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12734888#action_12734888 ]

Grant Ingersoll commented on LUCENE-1693:
-----------------------------------------

{quote}Token is no longer deprecated, instead it implements all 6 standard
token interfaces (see above). The wrapper for next() and next(Token)
uses this, to automatically map all attribute interfaces to one
TokenWrapper instance (implementing all 6 interfaces), that contains
a Token instance. next() and next(Token) exchange the inner Token
instance as needed. For the new incrementToken(), only one
TokenWrapper instance is visible, delegating to the currect reusable
Token. This API also preserves custom Token subclasses, that maybe
created by very special token streams (see example in Backwards-Test)
{quote}

Token is no longer deprecated, but all the methods that return it are, right?  I think they are, but it is late, and I may have missed something.  So, what happens in 3.0?  What good does a Token do at that point?

Also, in looking at incrementToken(), nearly the entire implementation seems to be based on deprecated stuff, doesn't that mean it has to all be re-implemented in 3.0?  Something about that doesn't feel right.

More later...

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead it is now enough to only implement the new API,
>   if one old TokenStream implements still the old API (next()/next(Token)),
>   it is wrapped automatically. The delegation path is determined via
>   reflection (the patch determines, which of the three methods was
>   overridden).
> - Token is no longer deprecated, instead it implements all 6 standard
>   token interfaces (see above). The wrapper for next() and next(Token)
>   uses this, to automatically map all attribute interfaces to one
>   TokenWrapper instance (implementing all 6 interfaces), that contains
>   a Token instance. next() and next(Token) exchange the inner Token
>   instance as needed. For the new incrementToken(), only one
>   TokenWrapper instance is visible, delegating to the currect reusable
>   Token. This API also preserves custom Token subclasses, that maybe
>   created by very special token streams (see example in Backwards-Test).
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> - Tee- and SinkTokenizer were deprecated, because they use
> Token instances for caching. This is not compatible to the new API
> using AttributeSource.State objects. You can still use the old
> deprecated ones, but new features provided by new Attribute types
> may get lost in the chain. A replacement is a new TeeSinkTokenFilter,
> which has a factory to create new Sink instances, that have compatible
> attributes. Sink instances created by one Tee can also be added to
> another Tee, as long as the attribute implementations are compatible
> (it is not possible to add a sink from a tee using one Token instance
> to a tee using the six separate attribute impls). In this case UOE is thrown.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> This issue contains one backwards-compatibility break:
> TokenStreams/Filters/Tokenizers should normally be final
> (see LUCENE-1753 for the explaination). Some of these core classes are
> not final and so one could override the next() or next(Token) methods.
> In this case, the backwards-wrapper would automatically use
> incrementToken(), because it is implemented, so the overridden
> method is never called. To prevent users from errors not visible
> during compilation or testing (the streams just behave wrong),
> this patch makes all implementation methods final
> (next(), next(Token), incrementToken()), whenever the class
> itsself is not final. This is a BW break, but users will clearly see,
> that they have done something unsupoorted and should better
> create a custom TokenFilter with their additional implementation
> (instead of extending a core implementation).
> For further changing contrib token streams the following procedere should be used:
>     *  rewrite and replace next(Token)/next() implementations by new API
>     * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
>     * if the class is non-final add the following methods to the class:
> {code:java}
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next(final Token reusableToken) throws java.io.IOException {
>         return super.next(reusableToken);
>       }
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next() throws java.io.IOException {
>         return super.next();
>       }
> {code}
> Also the incrementToken() method must be final in this case
> (and the new method end() of LUCENE-1448)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12734955#action_12734955 ]

Michael Busch commented on LUCENE-1693:
---------------------------------------

{quote}
Token is no longer deprecated, but all the methods that return it are, right? I think they are, but it is late, and I may have missed something. So, what happens in 3.0? What good does a Token do at that point?
{quote}

It's what you often asked for: if you don't want to deal with multiple Attributes you can simply add a Token to the AttributeSource and cache the Token reference locally in your stream/filter, because Token now implements all core token attributes.

{quote}
Also, in looking at incrementToken(), nearly the entire implementation seems to be based on deprecated stuff, doesn't that mean it has to all be re-implemented in 3.0? Something about that doesn't feel right.
{quote}

Ideally there wouldn't be an incrementToken() implementation and it would be abstract. However, that's of course not backwards-compatible. Everything is deprecated in the implementation because this code was only written to support backwards-compatibility with the old API.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead it is now enough to only implement the new API,
>   if one old TokenStream implements still the old API (next()/next(Token)),
>   it is wrapped automatically. The delegation path is determined via
>   reflection (the patch determines, which of the three methods was
>   overridden).
> - Token is no longer deprecated, instead it implements all 6 standard
>   token interfaces (see above). The wrapper for next() and next(Token)
>   uses this, to automatically map all attribute interfaces to one
>   TokenWrapper instance (implementing all 6 interfaces), that contains
>   a Token instance. next() and next(Token) exchange the inner Token
>   instance as needed. For the new incrementToken(), only one
>   TokenWrapper instance is visible, delegating to the currect reusable
>   Token. This API also preserves custom Token subclasses, that maybe
>   created by very special token streams (see example in Backwards-Test).
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> - Tee- and SinkTokenizer were deprecated, because they use
> Token instances for caching. This is not compatible to the new API
> using AttributeSource.State objects. You can still use the old
> deprecated ones, but new features provided by new Attribute types
> may get lost in the chain. A replacement is a new TeeSinkTokenFilter,
> which has a factory to create new Sink instances, that have compatible
> attributes. Sink instances created by one Tee can also be added to
> another Tee, as long as the attribute implementations are compatible
> (it is not possible to add a sink from a tee using one Token instance
> to a tee using the six separate attribute impls). In this case UOE is thrown.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> This issue contains one backwards-compatibility break:
> TokenStreams/Filters/Tokenizers should normally be final
> (see LUCENE-1753 for the explaination). Some of these core classes are
> not final and so one could override the next() or next(Token) methods.
> In this case, the backwards-wrapper would automatically use
> incrementToken(), because it is implemented, so the overridden
> method is never called. To prevent users from errors not visible
> during compilation or testing (the streams just behave wrong),
> this patch makes all implementation methods final
> (next(), next(Token), incrementToken()), whenever the class
> itsself is not final. This is a BW break, but users will clearly see,
> that they have done something unsupoorted and should better
> create a custom TokenFilter with their additional implementation
> (instead of extending a core implementation).
> For further changing contrib token streams the following procedere should be used:
>     *  rewrite and replace next(Token)/next() implementations by new API
>     * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
>     * if the class is non-final add the following methods to the class:
> {code:java}
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next(final Token reusableToken) throws java.io.IOException {
>         return super.next(reusableToken);
>       }
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next() throws java.io.IOException {
>         return super.next();
>       }
> {code}
> Also the incrementToken() method must be final in this case
> (and the new method end() of LUCENE-1448)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...

    [ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12734959#action_12734959 ]

Uwe Schindler commented on LUCENE-1693:
---------------------------------------

And in 3.0 all my phantastic backwards-compatibility stuff is completely removed - and incrementToken() is abstract - What a pity :( The default impl in incrementToken and all other code parts in TokenStream.java will be removed (this is why also new code parts are marked deprectated, it makes it easier to be removed). The code will be about 1/3 of the current size when the backwards layer is removed :-)
The biggest problem of the migration to the new API is the backwards stuff, because we have now 3 different TokenStream APIs (next(), next(Token), incrementToken()). Also TokenWrapper is backwards stuff and will be removed.

> AttributeSource/TokenStream API improvements
> --------------------------------------------
>
>                 Key: LUCENE-1693
>                 URL: https://issues.apache.org/jira/browse/LUCENE-1693
>             Project: Lucene - Java
>          Issue Type: Improvement
>          Components: Analysis
>            Reporter: Michael Busch
>            Assignee: Michael Busch
>            Priority: Minor
>             Fix For: 2.9
>
>         Attachments: lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, lucene-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, LUCENE-1693.patch, lucene-1693.patch, PerfTest3.java, TestAPIBackwardsCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java, TestCompatibility.java
>
>
> This patch makes the following improvements to AttributeSource and
> TokenStream/Filter:
> - introduces interfaces for all Attributes. The corresponding
>   implementations have the postfix 'Impl', e.g. TermAttribute and
>   TermAttributeImpl. AttributeSource now has a factory for creating
>   the Attribute instances; the default implementation looks for
>   implementing classes with the postfix 'Impl'. Token now implements
>   all 6 TokenAttribute interfaces.
> - new method added to AttributeSource:
>   addAttributeImpl(AttributeImpl). Using reflection it walks up in the
>   class hierarchy of the passed in object and finds all interfaces
>   that the class or superclasses implement and that extend the
>   Attribute interface. It then adds the interface->instance mappings
>   to the attribute map for each of the found interfaces.
> - removes the set/getUseNewAPI() methods (including the standard
>   ones). Instead it is now enough to only implement the new API,
>   if one old TokenStream implements still the old API (next()/next(Token)),
>   it is wrapped automatically. The delegation path is determined via
>   reflection (the patch determines, which of the three methods was
>   overridden).
> - Token is no longer deprecated, instead it implements all 6 standard
>   token interfaces (see above). The wrapper for next() and next(Token)
>   uses this, to automatically map all attribute interfaces to one
>   TokenWrapper instance (implementing all 6 interfaces), that contains
>   a Token instance. next() and next(Token) exchange the inner Token
>   instance as needed. For the new incrementToken(), only one
>   TokenWrapper instance is visible, delegating to the currect reusable
>   Token. This API also preserves custom Token subclasses, that maybe
>   created by very special token streams (see example in Backwards-Test).
> - AttributeImpl now has a default implementation of toString that uses
>   reflection to print out the values of the attributes in a default
>   formatting. This makes it a bit easier to implement AttributeImpl,
>   because toString() was declared abstract before.
> - Cloning is now done much more efficiently in
>   captureState. The method figures out which unique AttributeImpl
>   instances are contained as values in the attributes map, because
>   those are the ones that need to be cloned. It creates a single
>   linked list that supports deep cloning (in the inner class
>   AttributeSource.State). AttributeSource keeps track of when this
>   state changes, i.e. whenever new attributes are added to the
>   AttributeSource. Only in that case will captureState recompute the
>   state, otherwise it will simply clone the precomputed state and
>   return the clone. restoreState(AttributeSource.State) walks the
>   linked list and uses the copyTo() method of AttributeImpl to copy
>   all values over into the attribute that the source stream
>   (e.g. SinkTokenizer) uses.
> - Tee- and SinkTokenizer were deprecated, because they use
> Token instances for caching. This is not compatible to the new API
> using AttributeSource.State objects. You can still use the old
> deprecated ones, but new features provided by new Attribute types
> may get lost in the chain. A replacement is a new TeeSinkTokenFilter,
> which has a factory to create new Sink instances, that have compatible
> attributes. Sink instances created by one Tee can also be added to
> another Tee, as long as the attribute implementations are compatible
> (it is not possible to add a sink from a tee using one Token instance
> to a tee using the six separate attribute impls). In this case UOE is thrown.
> The cloning performance can be greatly improved if not multiple
> AttributeImpl instances are used in one TokenStream. A user can
> e.g. simply add a Token instance to the stream instead of the individual
> attributes. Or the user could implement a subclass of AttributeImpl that
> implements exactly the Attribute interfaces needed. I think this
> should be considered an expert API (addAttributeImpl), as this manual
> optimization is only needed if cloning performance is crucial. I ran
> some quick performance tests using Tee/Sink tokenizers (which do
> cloning) and the performance was roughly 20% faster with the new
> API. I'll run some more performance tests and post more numbers then.
> Note also that when we add serialization to the Attributes, e.g. for
> supporting storing serialized TokenStreams in the index, then the
> serialization should benefit even significantly more from the new API
> than cloning.
> This issue contains one backwards-compatibility break:
> TokenStreams/Filters/Tokenizers should normally be final
> (see LUCENE-1753 for the explaination). Some of these core classes are
> not final and so one could override the next() or next(Token) methods.
> In this case, the backwards-wrapper would automatically use
> incrementToken(), because it is implemented, so the overridden
> method is never called. To prevent users from errors not visible
> during compilation or testing (the streams just behave wrong),
> this patch makes all implementation methods final
> (next(), next(Token), incrementToken()), whenever the class
> itsself is not final. This is a BW break, but users will clearly see,
> that they have done something unsupoorted and should better
> create a custom TokenFilter with their additional implementation
> (instead of extending a core implementation).
> For further changing contrib token streams the following procedere should be used:
>     *  rewrite and replace next(Token)/next() implementations by new API
>     * if the class is final, no next(Token)/next() methods needed (must be removed!!!)
>     * if the class is non-final add the following methods to the class:
> {code:java}
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next(final Token reusableToken) throws java.io.IOException {
>         return super.next(reusableToken);
>       }
>       /** @deprecated Will be removed in Lucene 3.0. This method is final, as it should
>        * not be overridden. Delegates to the backwards compatibility layer. */
>       public final Token next() throws java.io.IOException {
>         return super.next();
>       }
> {code}
> Also the incrementToken() method must be final in this case
> (and the new method end() of LUCENE-1448)

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: java-dev-unsubscribe@...
For additional commands, e-mail: java-dev-help@...