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

remove repeated nl.getLength() calls in (Boolean|DisjunctionMax|FuzzyLikeThis)QueryBuilder

    Details

    • Type: Task
    • Status: Resolved
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.x, 6.1
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      proposed trivial patch against master to follow

      1. LUCENE-7205.patch
        3 kB
        Christine Poerschke

        Activity

        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit f2f484680f155e89139e5651955ff057660bd2aa in lucene-solr's branch refs/heads/master from Christine Poerschke
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f2f4846 ]

        LUCENE-7205: Remove repeated nl.getLength() calls in (Boolean|DisjunctionMax|FuzzyLikeThis)QueryBuilder.

        Show
        jira-bot ASF subversion and git services added a comment - Commit f2f484680f155e89139e5651955ff057660bd2aa in lucene-solr's branch refs/heads/master from Christine Poerschke [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f2f4846 ] LUCENE-7205 : Remove repeated nl.getLength() calls in (Boolean|DisjunctionMax|FuzzyLikeThis)QueryBuilder.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 87d7de0d9892eab8462f08b876af3481f5b0ccc3 in lucene-solr's branch refs/heads/branch_6x from Christine Poerschke
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=87d7de0 ]

        LUCENE-7205: Remove repeated nl.getLength() calls in (Boolean|DisjunctionMax|FuzzyLikeThis)QueryBuilder.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 87d7de0d9892eab8462f08b876af3481f5b0ccc3 in lucene-solr's branch refs/heads/branch_6x from Christine Poerschke [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=87d7de0 ] LUCENE-7205 : Remove repeated nl.getLength() calls in (Boolean|DisjunctionMax|FuzzyLikeThis)QueryBuilder.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 0cdb3cafb2f1bcb975be85cbc63ab124ddba31a8 in lucene-solr's branch refs/heads/branch_5x from Christine Poerschke
        [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=0cdb3ca ]

        LUCENE-7205: Remove repeated nl.getLength() calls in (DisjunctionMax|FuzzyLikeThis)QueryBuilder and Boolean(Query|Filter)Builder.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 0cdb3cafb2f1bcb975be85cbc63ab124ddba31a8 in lucene-solr's branch refs/heads/branch_5x from Christine Poerschke [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=0cdb3ca ] LUCENE-7205 : Remove repeated nl.getLength() calls in (DisjunctionMax|FuzzyLikeThis)QueryBuilder and Boolean(Query|Filter)Builder.
        Hide
        dsmiley David Smiley added a comment -

        What prompted this? I would expect NodeList.getLength() implementations to be trivial getters; no?

        Show
        dsmiley David Smiley added a comment - What prompted this? I would expect NodeList.getLength() implementations to be trivial getters; no?
        Hide
        cpoerschke Christine Poerschke added a comment -

        I agree, the NodeList.getLength() implementations should be a trivial getter. With multiple/many child nodes in the node list that would be repeated methods calls nonetheless, all returning the same result since the list doesn't change, so this is meant to be very minor optimisation (the extra `int nlLen` presumably doesn't really cost since even when called within the for-loop the getLength() result needs to be stored?).

        Show
        cpoerschke Christine Poerschke added a comment - I agree, the NodeList.getLength() implementations should be a trivial getter. With multiple/many child nodes in the node list that would be repeated methods calls nonetheless, all returning the same result since the list doesn't change, so this is meant to be very minor optimisation (the extra `int nlLen` presumably doesn't really cost since even when called within the for-loop the getLength() result needs to be stored?).
        Hide
        dsmiley David Smiley added a comment -

        Personally I don't think we should add extra lines of code (a form of cost in readability) where there is negligible benefit to optimize. But I'm not standing in your way.

        Show
        dsmiley David Smiley added a comment - Personally I don't think we should add extra lines of code (a form of cost in readability) where there is negligible benefit to optimize. But I'm not standing in your way.
        Hide
        cpoerschke Christine Poerschke added a comment -

        Extra lines of code as a form of cost in readability is a fair point. Will keep that in mind going forward (and have no further optimisations like this planned at the moment). Thanks for your input!

        Show
        cpoerschke Christine Poerschke added a comment - Extra lines of code as a form of cost in readability is a fair point. Will keep that in mind going forward (and have no further optimisations like this planned at the moment). Thanks for your input!

          People

          • Assignee:
            cpoerschke Christine Poerschke
            Reporter:
            cpoerschke Christine Poerschke
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development