[ https://issues.apache.org/jira/browse/LUCENE-5372?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13857098#comment-13857098 ]
Dawid Weiss commented on LUCENE-5372: ------------------------------------- I wanted to apply these patches but then looked deeper and it seems we can't just apply them without some consideration. For example, while reviewing, I noticed things such as this one: {code} - * All use of StringBuffers has been refactored to StringBuilder for speed. + * All use of StringBuilders has been refactored to StringBuilder for speed. {code} which seem to be an auto-replacement artifact. While this is a no-problem, this may be: {code} +++ b/lucene/core/src/java/org/apache/lucene/analysis/tokenattributes/CharTermAttributeImpl.java @@ -144,8 +144,8 @@ public class CharTermAttributeImpl extends AttributeImpl implements CharTermAttr } else if (csq instanceof CharBuffer && ((CharBuffer) csq).hasArray()) { final CharBuffer cb = (CharBuffer) csq; System.arraycopy(cb.array(), cb.arrayOffset() + cb.position() + start, termBuffer, termLength, len); - } else if (csq instanceof StringBuffer) { - ((StringBuffer) csq).getChars(start, end, termBuffer, termLength); + } else if (csq instanceof StringBuilder) { + ((StringBuilder) csq).getChars(start, end, termBuffer, termLength); {code} but CharTermAttributeImpl already has an if clause for StringBuilder, the full code is: {code} if (csq instanceof String) { ((String) csq).getChars(start, end, termBuffer, termLength); } else if (csq instanceof StringBuilder) { ((StringBuilder) csq).getChars(start, end, termBuffer, termLength); } else if (csq instanceof CharTermAttribute) { System.arraycopy(((CharTermAttribute) csq).buffer(), start, termBuffer, termLength, len); } else if (csq instanceof CharBuffer && ((CharBuffer) csq).hasArray()) { final CharBuffer cb = (CharBuffer) csq; System.arraycopy(cb.array(), cb.arrayOffset() + cb.position() + start, termBuffer, termLength, len); } else if (csq instanceof StringBuffer) { ((StringBuffer) csq).getChars(start, end, termBuffer, termLength); } else { while (start < end) termBuffer[termLength++] = csq.charAt(start++); // no fall-through here, as termLength is updated! return this; } {code} I would actually leave it for Uwe to modify the api checker rules and then hand-pick offenders. Your contribution won't be lost, Joshua, it'll just go in via another route (not a direct patch, rather a good suggestion :). > IntArray toString has O(n^2) performance > ---------------------------------------- > > Key: LUCENE-5372 > URL: https://issues.apache.org/jira/browse/LUCENE-5372 > Project: Lucene - Core > Issue Type: Bug > Components: core/index > Reporter: Joshua Hartman > Assignee: Dawid Weiss > Priority: Minor > Fix For: 5.0, 4.7 > > Attachments: 5372-lucene5339.patch, 5372-v2.patch, 5372.patch, > LUCENE-5372-forbidden.patch > > > This is pretty minor, but I found a few issues with the toString > implementations while looking through the facet data structures. > The most egregious is the use of string concatenation in the IntArray class. > I have fixed that using StringBuilders. I also noticed that other classes > were using StringBuffer instead of StringBuilder. According to the javadoc, > "This class is designed for use as a drop-in replacement for StringBuffer in > places where the string buffer was being used by a single thread (as is > generally the case). Where possible, it is recommended that this class be > used in preference to StringBuffer as it will be faster under most > implementations." -- This message was sent by Atlassian JIRA (v6.1.5#6160) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org