Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-2169

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

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: 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
        thetaphi Uwe Schindler added a comment -

        Committed revision: 893655

        Show
        thetaphi Uwe Schindler added a comment - Committed revision: 893655
        Hide
        thetaphi Uwe Schindler added a comment -

        OK.

        I will commit this together with LUCENE-2179.

        Show
        thetaphi Uwe Schindler added a comment - OK. I will commit this together with LUCENE-2179 .
        Hide
        simonw 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
        simonw 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
        thetaphi 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
        thetaphi 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
        simonw 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
        simonw 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
        simonw 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
        simonw 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
        thetaphi Uwe Schindler added a comment - - edited

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

        Show
        thetaphi Uwe Schindler added a comment - - edited Better patch, that is more clear and structured. Also improved javadocs.
        Hide
        thetaphi 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
        thetaphi 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
        simonw Simon Willnauer added a comment -

        here is a patch

        Show
        simonw Simon Willnauer added a comment - here is a patch

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development