[jira] Created: (LUCENE-1693) AttributeSource/TokenStream API improvements
|
View:
New views
20 Messages
—
Rating Filter:
Alert me
|
| < Prev | 1 - 2 - 3 - 4 - 5 - 6 - 7 - 8 - 9 - 10 | Next > |
 |
[jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12723473#action_12723473 ] Uwe Schindler commented on LUCENE-1693: --------------------------------------- {quote} bq. The big difference is, that you do not need to implement both APIs in your TokenStreams (the current patch removes the old API completely from core tokenizers). Thats a big back compat violation though right ? :) {quote} No it isn't. It is not completely removed, the abstact TokenStream emulates the old next() methods, so even if all core tokenizers do not implement next(), they still export the next(Token) and next() methods from TokenStream. > 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, 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@... |
|
|
[jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12723536#action_12723536 ] Mark Miller commented on LUCENE-1693: ------------------------------------- Ah, okay. Sounds like some magic you have worked here Uwe - great stuff. Very nice to not have to impl twice everywhere. > 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, 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@... |
|
|
[jira] Updated: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ 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 Hi Michael, this was a small bug in the translation next(Token) -> incrementToken(). If the call to incrementToken itsself calls other filters, that itsself delegate back to next() the wrapper.delegate can change during the incrementToken call. I fixed this, now it works: {code} [junit] ------------- Standard Output --------------- [junit] [junit] Test old API... [junit] new [junit] tokenstream --> proper noun [junit] api [junit] new [junit] tokenstream --> proper noun [junit] api [junit] new [junit] tokenstream [junit] api [junit] [junit] Test new API... [junit] new [junit] tokenstream --> proper noun [junit] api [junit] new [junit] tokenstream --> proper noun [junit] api [junit] new [junit] tokenstream [junit] api {code} Are you happy now? I think Mark is also very interesting in this and fast progress. In my opinion, only the new Tee/Sink/Cache impls are needed and the fix for the duplicate attributes in toString(). > 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, 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@... |
|
|
[jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12723549#action_12723549 ] Uwe Schindler commented on LUCENE-1693: --------------------------------------- bq. But I'll definitely buy Uwe a beer if he comes up with solution that is more elegant and doesn't have the mentioned disadvantages! By the way: I want to have my beer!!! > 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, 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@... |
|
|
[jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12723720#action_12723720 ] Michael Busch commented on LUCENE-1693: --------------------------------------- Sorry, I don't want to be the bad guy here, just trying to mention things that come to my mind. Maybe this one is negligible? If the user had extensions of streams/filters that in 2.4 didn't declare next() or next(Token) as final and calls super.next() or super.next(Token), then I think you'll get different behavior or exceptions. E.g.: {code:java} private static class ExtendedSinkTokenizer extends SinkTokenizer { public Token next() throws IOException { Token t = super.next(); // do something with t return t; } } {code} > 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, 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@... |
|
|
[jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12723727#action_12723727 ] Uwe Schindler commented on LUCENE-1693: --------------------------------------- In your example, you mean SinkTokenizer is from the core and only implements incrementToken(), the others are handles by the default in TokenStream (using this reflection-based redirect?) I tested this with overriding LowerCaseFilter. In my latest patch LowerCaseFilter only implements incrementToken(), next() and next(Token) are handled by the forwarder in TokenStream. If you override next(Token) and consume this stream using incrementToken(), then your overriden next(Token) is never called. This exact same backwards-compatibility problem was also there when changed from next() to next(Token). If some core filter only implemented next(Token) and a subclass overrides only next() (because older code-base) and calls super() there, the same happened: the tokenizer code called next(Token), but next() was never called. You have the same problem with our recent deprecations in DocIdSetIterator. I would no see this as a problem, this problem can be found everywhere in Lucene, where we deprecated methods and replaced by newer ones calling the old ones as default. > 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, 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@... |
|
|
[jira] Issue Comment Edited: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12723727#action_12723727 ] Uwe Schindler edited comment on LUCENE-1693 at 6/24/09 1:46 PM: ---------------------------------------------------------------- In your example, you mean SinkTokenizer is from the core and only implements incrementToken(), the others are handled by the default in TokenStream (using this reflection-based redirect?) I tested this with overriding LowerCaseFilter. In my latest patch LowerCaseFilter only implements incrementToken(), next() and next(Token) are handled by the forwarder in TokenStream. If you override next(Token) and consume this stream using incrementToken(), then your overriden next(Token) is never called. This exact same backwards-compatibility problem was also there when changed from next() to next(Token). If some core filter only implemented next(Token) and a subclass overrides only next() (because older code-base) and calls super() there, the same happened: the tokenizer code called next(Token), but next() was never called. You have the same problem with our recent deprecations in DocIdSetIterator. I would no see this as a problem, this problem can be found everywhere in Lucene, where we deprecated (originally abstract) methods and replaced by newer ones calling the old ones as default. was (Author: thetaphi): In your example, you mean SinkTokenizer is from the core and only implements incrementToken(), the others are handles by the default in TokenStream (using this reflection-based redirect?) I tested this with overriding LowerCaseFilter. In my latest patch LowerCaseFilter only implements incrementToken(), next() and next(Token) are handled by the forwarder in TokenStream. If you override next(Token) and consume this stream using incrementToken(), then your overriden next(Token) is never called. This exact same backwards-compatibility problem was also there when changed from next() to next(Token). If some core filter only implemented next(Token) and a subclass overrides only next() (because older code-base) and calls super() there, the same happened: the tokenizer code called next(Token), but next() was never called. You have the same problem with our recent deprecations in DocIdSetIterator. I would no see this as a problem, this problem can be found everywhere in Lucene, where we deprecated methods and replaced by newer ones calling the old ones as default. > 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, 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@... |
|
|
[jira] Updated: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Michael Busch updated LUCENE-1693: ---------------------------------- Attachment: TestCompatibility.java Not sure if we're talking about the same thing here. The problem is not that next() in the extended class doesn't get called. It gets called, but the super.next() call throws either a ClassCastException, when consuming using the old API, or produces a different result than before uwhen consuming the new API. This is the output on your patch: {noformat} Testing old API... new tokenstream --> proper noun api new tokenstream --> proper noun api new tokenstream api new tokenstream --> proper noun api (new,12,15) new (tokenstream,16,27) tokenstream (api,28,31) api null java.lang.ClassCastException: org.apache.lucene.util.AttributeSource$State at org.apache.lucene.analysis.SinkTokenizer.next(SinkTokenizer.java:97) at org.apache.lucene.analysis.TestCompatibility.consumeStreamOldAPI(TestCompatibility.java:194) at org.apache.lucene.analysis.TestCompatibility.test2(TestCompatibility.java:142) at org.apache.lucene.analysis.TestCompatibility.main(TestCompatibility.java:87) Testing new API... new tokenstream --> proper noun api new tokenstream --> proper noun api new tokenstream api new tokenstream --> proper noun api (new,12,15) new (tokenstream,16,27) tokenstream (api,28,31) api null new tokenstream api {noformat} On my patch: {noformat} Testing old API... new tokenstream --> proper noun api new tokenstream --> proper noun api new tokenstream api new tokenstream --> proper noun api (new,12,15) new (tokenstream,16,27) tokenstream --> proper noun (api,28,31) api null new tokenstream api Testing new API... org.apache.lucene.analysis.TokenStream$UnsupportedTokenStreamAPIException: This stream or filter does not support the new Lucene 2.9 TokenStream API yet. at org.apache.lucene.analysis.TokenStream.incrementToken(TokenStream.java:114) at org.apache.lucene.analysis.TestAPICompatibility.consumeStreamNewAPI(TestAPICompatibility.java:210) at org.apache.lucene.analysis.TestAPICompatibility.test3(TestAPICompatibility.java:162) at org.apache.lucene.analysis.TestAPICompatibility.main(TestAPICompatibility.java:93) org.apache.lucene.analysis.TokenStream$UnsupportedTokenStreamAPIException: This stream or filter does not support the new Lucene 2.9 TokenStream API yet. at org.apache.lucene.analysis.TokenStream.incrementToken(TokenStream.java:114) at org.apache.lucene.analysis.TestAPICompatibility.consumeStreamNewAPI(TestAPICompatibility.java:210) at org.apache.lucene.analysis.TestAPICompatibility.test4(TestAPICompatibility.java:183) at org.apache.lucene.analysis.TestAPICompatibility.main(TestAPICompatibility.java:98) {noformat} > 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, 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@... |
|
|
[jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12723749#action_12723749 ] Uwe Schindler commented on LUCENE-1693: --------------------------------------- bq. Not sure if we're talking about the same thing here. The problem is not that next() in the extended class doesn't get called. I think we are talking about the same. We have exact the same problem, if you override OpenBitSetIterator, but only implement next() instead of nextDoc(). next() is never called, because the new API (nextDoc()) is handling everything. It's exactly the same. You have this always, if you deprecate former abstract methods and make the new one call the old one. I a subclass overrides the new API, the old API is dead for this class. A further (3rd party) subclass then overrides the deprecated method, it is never called -> bäng. Other example: Filter. We have this everywhere (DocIdSetIterator next() vs nextDoc(), Filter getBits() vs. getDocIdSet(), TokenStream very old API vs. old API). bq. It gets called, but the super.next() call throws either a ClassCastException, when consuming using the old API, or produces a different result than before uwhen consuming the new API. A ClassCastException? Why that? super.next() is always available (at least TokenStream on top of the hierarchy defines 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, 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@... |
|
|
[jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12723751#action_12723751 ] Uwe Schindler commented on LUCENE-1693: --------------------------------------- One solution around this poblem would be to always implement both incrementToken() and next(Token) in non-final classes that tend to be overridden (CharTokenizer). In this case you are right. So I should revert the removal of next(Token). But very old code only overriding next() still fails. But final things like StandardFilter do not need to have both implementations. > 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, 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@... |
|
|
[jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12723759#action_12723759 ] Michael Busch commented on LUCENE-1693: --------------------------------------- {quote} I decided a while ago that you'll definitely get it, because of your admirable tenaciousness. :) {quote} > 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, 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@... |
|
|
[jira] Issue Comment Edited: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12723759#action_12723759 ] Michael Busch edited comment on LUCENE-1693 at 6/24/09 2:50 PM: ---------------------------------------------------------------- {quote} By the way: I want to have my beer!!! {quote} I decided a while ago that you'll definitely get it, because of your admirable tenaciousness. :) was (Author: michaelbusch): {quote} I decided a while ago that you'll definitely get it, because of your admirable tenaciousness. :) {quote} > 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, 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@... |
|
|
Re: [jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by markrmiller
::
Rate this Message:
Hows the progress here guys? I have 2 or 3 issues that relate to this, and I really don't want to commit/finish them until this is done ...
|
|
|
RE: [jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by Uwe Schindler
::
Rate this Message:
Some parts of this message have been removed.
Learn more about Nabble's security policy.
I do not work on this at
the moment. The current status is: - All final token stream/filter
classes only need to implement incrementToken() as they have no backwards
compatibility problem. - If it is possible for
somebody (or better likely the case) to override one of both methods, where
only one of them is implemented in lucene, there may be the problem that overridden
code is never called (because incrementToken is always preferred). So: For most analyzers from
Lucene core this is no problem, as they are final. Only classes like
CharTokenizer, that are written explicitely for subclassing must implement both.
I will revert my changes (remove all next(Token) from all streams) soon (post a
new patch). But this problem is the same we had before: If one overrides only
next() in one of the non-final streams, this method is also never called. So
this was a break in backwards compatibility in the past, too. My recommendation: Even
if classes are not final, we could convert next(Token) to incrementToken() and
remove the old ones, if it is not likely the case, that somebody have tried to
customize one of the deprecated methods. And a clear not in the Changes.txt. In my opinion, this small
backwards break is better than creating a switch like userNewAPI, that is never
set to 1, so the code is never used in Lucene 2.9 and everybody breaks when 3.0
comes out… In my opinion, this patch
can be extensive tested, the questions about backwards compatibility are
solved. What is still broken is Sink and TeeTokenizer, the old classes must be deprecated,
because it is different if you store attribute states in the map or Tokens and
is not implementable in one class (it is already broken in trunk, as the javadocs
always talk about Token instances in the Set, but when useneAPI is on they are
suddenly Attribute states). I hope Michael will have time to review this, too. It
seems that we are the only ones who can follow the thread J and all these “hey here is a BW break…”
hick-hacks. Uwe ----- From: Mark Miller
[mailto:markrmiller@...] Hows the progress here guys? I have 2 or 3 issues that relate to this,
and I really don't want to commit/finish them until this is done ... |
|
|
[jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12726066#action_12726066 ] Uwe Schindler commented on LUCENE-1693: --------------------------------------- After thinking one round about it again. Your original patch with the UnsupportedOperationException have the same problem. If one overrides next(Token) in one non-final TokenStream/Filter, the overriden method is never called. And it is also not enough to always override both methods in subclass-able streams/filters. With both batches we break BW compatibility for users that do not override TokenStream or TokenFilter directly. Even my last comment on java-dev is not correct. Somebody who overrides a Filter (not direct subclass of TokenStream/Filter/Tokenizer, or the in-between classes are also of this user and only override deprecated APIs) and only override next(Token) (e.g. subclass of CharTokenizer), the overriden method will never be called, if the Filter/Stream is top-level in the filter chain :( I think, we must have somewhere a BW break and clearly document it in the following way in CHANGES.txt: - You can mix old and new filters/streams without problems. - Such mixed chains can be consumed with both new/old API - To have this working, always override the abstract TokenStream/TokenFilter/Tokenizer directly and see all other core classes as "final". In all other cases your filters/token streams will have unpredicatable behaviour. Uwe > 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, 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@... |
|
|
RE: [jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by Uwe Schindler
::
Rate this Message:
Some parts of this message have been removed.
Learn more about Nabble's security policy.
After thinking one more
time about it, there are still possibilities to break BW in some corner cases
with non-final classes. See my last comment in the issue! This new API is hard to
integrate in a BW compatible way. But in all cases, this case and the new
TokenStream API is a show stopper for 2.9. Uwe ----- From: Uwe Schindler
[mailto:uwe@...] I do not work on this at the
moment. The current status is: - All final token
stream/filter classes only need to implement incrementToken() as they have no
backwards compatibility problem. - If it is possible for somebody
(or better likely the case) to override one of both methods, where only one of
them is implemented in lucene, there may be the problem that overridden code is
never called (because incrementToken is always preferred). So: For most analyzers
from Lucene core this is no problem, as they are final. Only classes like
CharTokenizer, that are written explicitely for subclassing must implement
both. I will revert my changes (remove all next(Token) from all streams) soon
(post a new patch). But this problem is the same we had before: If one
overrides only next() in one of the non-final streams, this method is also
never called. So this was a break in backwards compatibility in the past, too. My recommendation: Even
if classes are not final, we could convert next(Token) to incrementToken() and
remove the old ones, if it is not likely the case, that somebody have tried to
customize one of the deprecated methods. And a clear not in the Changes.txt. In my opinion, this small
backwards break is better than creating a switch like userNewAPI, that is never
set to 1, so the code is never used in Lucene 2.9 and everybody breaks when 3.0
comes out… In my opinion, this patch
can be extensive tested, the questions about backwards compatibility are
solved. What is still broken is Sink and TeeTokenizer, the old classes must be
deprecated, because it is different if you store attribute states in the map or
Tokens and is not implementable in one class (it is already broken in trunk, as
the javadocs always talk about Token instances in the Set, but when useneAPI is
on they are suddenly Attribute states). I hope Michael will have time to review
this, too. It seems that we are the only ones who can follow the thread J and all these “hey here is a BW break…”
hick-hacks. Uwe ----- From: Mark Miller
[mailto:markrmiller@...] Hows the progress here guys? I have 2 or 3 issues that relate to this,
and I really don't want to commit/finish them until this is done ... |
|
|
Re: [jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by markrmiller
::
Rate this Message:
>> I hope Michael will have time to review this, too. It
seems that we are the only ones who can follow the thread J and all these “hey here is a BW break…”
hick-hacks. Personally, I'm not a big fan of the new API. I think its going to be *much* harder for new users to grok/manipulate. These back compat breaks are only going to make it that much more difficult. I think that it could be a fine trade off if Flexible Indexing ends up being able to move mountains for me, but as I think that's going to end up being niche coolness, I'm just not 100% keen on the new API. So basically, I don't have enough interest to catch up with these patches and read through this whole thread. I'm also concerned with a point someone previously brought up - we are deprecating the old API, telling users to move, but we are labeling the new API as experimental and subject to change. Jump user, jump. -- -- - Mark http://www.lucidimagination.com |
|
|
[jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12728823#action_12728823 ] Mark Miller commented on LUCENE-1693: ------------------------------------- Mr. Busch my friend, I'll buy both you and Uwe *many* beers if you resolve this issue soon! > 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, 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@... |
|
|
[jira] Commented: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ https://issues.apache.org/jira/browse/LUCENE-1693?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12728879#action_12728879 ] Michael Busch commented on LUCENE-1693: --------------------------------------- Alright, I hope you are coming to Oakland in November! I had a few (literally) sleepless nights last week to meet some internal deadlines; but it looks like I'll now have time to work on Lucene, so I'll continue on this issue tonight! > 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, 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@... |
|
|
[jira] Updated: (LUCENE-1693) AttributeSource/TokenStream API improvements
by JIRA jira@apache.org
::
Rate this Message:
[ 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 Attached is a new patch to current trunk. I did a hard work to make it compatible to also test-tag (see the java-dev discussion last weekend: test-tag was not really testing against 2.4, the bw-branch was created in November from trunk, when the new TokenStream API was already committed): - All tests core, contrib and tag pass correctly (and no changes to bw-branch needed!) - Michaels latest TestCompatibility also passes - All non-final and abstract Tokenizers implement both next(Token) and incrementToken() for maximum compatibility (but there are still problems mentioned above). This problems are always there and cannot be worked around, even with Michael's initial patch and at other places in Lucene, too (Filters, HitCollector). - CachingTokenFilter, TeeTokenizer and SinkTokenizer and the corresponding tests are reverted to 2.4 API and deprecated. There is not yet an implementation in new classes using solely the new API available. This is the only thing on the TODO-list for this case. I suggest the following name: CachingAttributesFilter; but for TeeTokenizer and SinkTokenizer I have no idea :( In my opinion, this is ready to commit and the non-deprecated Caching/Sink/Tee things could be a separate issue. We also need to write some conclusion or howto for CHANGES.txt, mentioning the problems with the switch to the new 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, 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@... |
| < Prev | 1 - 2 - 3 - 4 - 5 - 6 - 7 - 8 - 9 - 10 | Next > |
| Free embeddable forum powered by Nabble | Forum Help |
