Lucene - Core
  1. Lucene - Core
  2. LUCENE-2169

Speedup of CharArraySet#copy if a CharArraySet instance is passed to copy.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.1
    • Fix Version/s: 4.0-ALPHA
    • Component/s: modules/analysis
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      the copy method should use the entries array itself to copy the set internally instead of iterating over all values. this would speedup copying even small set

      1. LUCENE-2169.patch
        8 kB
        Simon Willnauer
      2. LUCENE-2169.patch
        8 kB
        Uwe Schindler
      3. LUCENE-2169.patch
        8 kB
        Uwe Schindler
      4. LUCENE-2169.patch
        5 kB
        Simon Willnauer

        Activity

        Hide
        Simon Willnauer added a comment -

        here is a patch

        Show
        Simon Willnauer added a comment - here is a patch
        Hide
        Uwe Schindler added a comment -

        Patch that extends test to verify that the documented behaviour on arbitrary objects using toString() works correctly.

        This also fixes the issue of preserving matchVersion.

        Show
        Uwe Schindler added a comment - Patch that extends test to verify that the documented behaviour on arbitrary objects using toString() works correctly. This also fixes the issue of preserving matchVersion.
        Hide
        Uwe Schindler added a comment - - edited

        Better patch, that is more clear and structured. Also improved javadocs.

        Show
        Uwe Schindler added a comment - - edited Better patch, that is more clear and structured. Also improved javadocs.
        Hide
        Simon Willnauer added a comment -

        Uwe, when I first looked at your patch I thought that is a good idea but once I looked at the usecases of CharArraySet differentiating between matchVersion if the given set is an instance of CharArraySet is not idea IMO. Imagine you create an analyzer with CharArraySet the analyzer will use its own given version together with the copy method internally if the analyzer is created with a different version than the provided stopset (which is already a CharArraySet) copy could change the behavior due to the given version with is actually the matchVersion for the analyzer not for the set. I would leave the decision to the user if a copy with a different version is what the user wants. If the version should not be preserved and the set to copy is a charArraySet users should use the constructor directly. I will attach a patch shortly

        Show
        Simon Willnauer added a comment - Uwe, when I first looked at your patch I thought that is a good idea but once I looked at the usecases of CharArraySet differentiating between matchVersion if the given set is an instance of CharArraySet is not idea IMO. Imagine you create an analyzer with CharArraySet the analyzer will use its own given version together with the copy method internally if the analyzer is created with a different version than the provided stopset (which is already a CharArraySet) copy could change the behavior due to the given version with is actually the matchVersion for the analyzer not for the set. I would leave the decision to the user if a copy with a different version is what the user wants. If the version should not be preserved and the set to copy is a charArraySet users should use the constructor directly. I will attach a patch shortly
        Hide
        Simon Willnauer added a comment -

        patch that incorporates uwes additions to the testcases and enforces the Version of the source set in the copy if it is an instance of CharArraySet

        Show
        Simon Willnauer added a comment - patch that incorporates uwes additions to the testcases and enforces the Version of the source set in the copy if it is an instance of CharArraySet
        Hide
        Uwe Schindler added a comment -

        But you now changed the behaviour of copy(). Before the patch it changed the CharArraySets matchVersion... That was what my patch was doing to preserve this behaviour.

        Show
        Uwe Schindler added a comment - But you now changed the behaviour of copy(). Before the patch it changed the CharArraySets matchVersion... That was what my patch was doing to preserve this behaviour.
        Hide
        Simon Willnauer added a comment -

        But you now changed the behaviour of copy(). Before the patch it changed the CharArraySets matchVersion... That was what my patch was doing to preserve this behaviour.

        the CharArraySet did not have a Version before 3.1 so this code has never been released. Changing this behavior is fine and will not break anything though.

        simon

        Show
        Simon Willnauer added a comment - But you now changed the behaviour of copy(). Before the patch it changed the CharArraySets matchVersion... That was what my patch was doing to preserve this behaviour. the CharArraySet did not have a Version before 3.1 so this code has never been released. Changing this behavior is fine and will not break anything though. simon
        Hide
        Uwe Schindler added a comment -

        OK.

        I will commit this together with LUCENE-2179.

        Show
        Uwe Schindler added a comment - OK. I will commit this together with LUCENE-2179 .
        Hide
        Uwe Schindler added a comment -

        Committed revision: 893655

        Show
        Uwe Schindler added a comment - Committed revision: 893655

          People

          • Assignee:
            Uwe Schindler
            Reporter:
            Simon Willnauer
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development