Lucene - Core
  1. Lucene - Core
  2. LUCENE-6885

StandardDirectoryReader (initialCapacity) tweaks

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.4, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      proposed patch against trunk to follow

      1. LUCENE-6885.patch
        1 kB
        Christine Poerschke
      2. LUCENE-6885.patch
        2 kB
        Christine Poerschke

        Activity

        Hide
        Adrien Grand added a comment -

        I'm concerned the lazy initialization of segmentReaders makes the code more complex but does not really buy us anything?

        Show
        Adrien Grand added a comment - I'm concerned the lazy initialization of segmentReaders makes the code more complex but does not really buy us anything?
        Hide
        Christine Poerschke added a comment -

        Fair point re: code complexity. The intention was to avoid allocation of the segmentReaders when they will remain empty because oldReaders is null and to allocate as many elements as will be needed (usually that would be more than the default 10 initial elements).

        Would it be clearer to do

        final Map<String,Integer> segmentReaders = (oldReaders == null ? null : new HashMap<>(oldReaders.size()));
        

        i.e. no lazy initialisation as such but the segmentReaders == null check later on would remain?

        Show
        Christine Poerschke added a comment - Fair point re: code complexity. The intention was to avoid allocation of the segmentReaders when they will remain empty because oldReaders is null and to allocate as many elements as will be needed (usually that would be more than the default 10 initial elements). Would it be clearer to do final Map< String , Integer > segmentReaders = (oldReaders == null ? null : new HashMap<>(oldReaders.size())); i.e. no lazy initialisation as such but the segmentReaders == null check later on would remain?
        Hide
        Christine Poerschke added a comment -

        If the segmentReaders == null check is a complexity concern then perhaps

        final Map<String,Integer> segmentReaders = (oldReaders == null ? new HashMap<>(1) : new HashMap<>(oldReaders.size()));
        

        instead?

        Show
        Christine Poerschke added a comment - If the segmentReaders == null check is a complexity concern then perhaps final Map< String , Integer > segmentReaders = (oldReaders == null ? new HashMap<>(1) : new HashMap<>(oldReaders.size())); instead?
        Hide
        Adrien Grand added a comment -

        This looks better to me indeed (or even better Collections.emptyMap() in the null case).

        Show
        Adrien Grand added a comment - This looks better to me indeed (or even better Collections.emptyMap() in the null case).
        Hide
        Christine Poerschke added a comment -

        Hadn't considered Collections.emptyMap() - good idea, thanks!

        Show
        Christine Poerschke added a comment - Hadn't considered Collections.emptyMap() - good idea, thanks!
        Hide
        Christine Poerschke added a comment -

        Attaching revised/simplified patch against trunk.

        Show
        Christine Poerschke added a comment - Attaching revised/simplified patch against trunk.
        Hide
        ASF subversion and git services added a comment -

        Commit 1712939 from Christine Poerschke in branch 'dev/trunk'
        [ https://svn.apache.org/r1712939 ]

        LUCENE-6885: StandardDirectoryReader (initialCapacity) tweaks

        Show
        ASF subversion and git services added a comment - Commit 1712939 from Christine Poerschke in branch 'dev/trunk' [ https://svn.apache.org/r1712939 ] LUCENE-6885 : StandardDirectoryReader (initialCapacity) tweaks
        Hide
        Christine Poerschke added a comment -

        Interesting/Unexpected branch_5x precommit error, looking into.

        compile-core:
            [mkdir] Created dir: ...branches\branch_5x\lucene\build\core\classes\java
            [javac] Compiling 731 source files to ...branches\branch_5x\lucene\build\core\classes\java
            [javac] ...branches\branch_5x\lucene\core\src\java\org\apache\lucene\index\StandardDirectoryReader.java:139: error: incompatible types: Map<Object,Object> cannot be converted to Map<String,Integer>
            [javac]     final Map<String,Integer> segmentReaders = (oldReaders == null ? Collections.emptyMap() : new HashMap<>(oldReaders.size()));
            [javac]                                                                    ^
            [javac] Note: Some input files use or override a deprecated API.
            [javac] Note: Recompile with -Xlint:deprecation for details.
            [javac] 1 error
        
        Show
        Christine Poerschke added a comment - Interesting/Unexpected branch_5x precommit error, looking into. compile-core: [mkdir] Created dir: ...branches\branch_5x\lucene\build\core\classes\java [javac] Compiling 731 source files to ...branches\branch_5x\lucene\build\core\classes\java [javac] ...branches\branch_5x\lucene\core\src\java\org\apache\lucene\index\StandardDirectoryReader.java:139: error: incompatible types: Map< Object , Object > cannot be converted to Map< String , Integer > [javac] final Map< String , Integer > segmentReaders = (oldReaders == null ? Collections.emptyMap() : new HashMap<>(oldReaders.size())); [javac] ^ [javac] Note: Some input files use or override a deprecated API. [javac] Note: Recompile with -Xlint:deprecation for details. [javac] 1 error
        Hide
        Adrien Grand added a comment -

        FYI I got this error a few times, and this is due to the fact that the Java 8 compiler is a bit smarter at guessing generic types. I think you can fix it by making the generic type explicit in the Collections.emptyMap() call: Collections.<String,Integer>emptyMap()

        Show
        Adrien Grand added a comment - FYI I got this error a few times, and this is due to the fact that the Java 8 compiler is a bit smarter at guessing generic types. I think you can fix it by making the generic type explicit in the Collections.emptyMap() call: Collections.<String,Integer>emptyMap()
        Hide
        Christine Poerschke added a comment -

        Thanks Adrien. Just been trying with the <String,Integer> also and I think it's something to do with the ? and : because on its own final Map<String,Integer> segmentReaders0 = Collections.<String,Integer>emptyMap(); is fine but in the real use it gives a different error

            [javac] ...branches\branch_5x\lucene\core\src\java\org\apache\lucene\index\StandardDirectoryReader.java:140: error: incompatible types: Map<? extends Object,? extends Object> cannot be converted to Map<String,Integer>
            [javac]     final Map<String,Integer> segmentReaders = (oldReaders == null ? Collections.<String,Integer>emptyMap() : new HashMap<>(oldReaders.size()));
            [javac]                                                                    ^
        

        and actually copying and pasting this, the ^ both in this and the previous error pointed in that direction also. Some parentheses should fix it then.

        Show
        Christine Poerschke added a comment - Thanks Adrien. Just been trying with the <String,Integer> also and I think it's something to do with the ? and : because on its own final Map<String,Integer> segmentReaders0 = Collections.<String,Integer>emptyMap(); is fine but in the real use it gives a different error [javac] ...branches\branch_5x\lucene\core\src\java\org\apache\lucene\index\StandardDirectoryReader.java:140: error: incompatible types: Map<? extends Object ,? extends Object > cannot be converted to Map< String , Integer > [javac] final Map< String , Integer > segmentReaders = (oldReaders == null ? Collections.< String , Integer >emptyMap() : new HashMap<>(oldReaders.size())); [javac] ^ and actually copying and pasting this, the ^ both in this and the previous error pointed in that direction also. Some parentheses should fix it then.
        Hide
        Christine Poerschke added a comment -

        Never mind parentheses, String,Integer needed for new HashMap<> also now.

        -    final Map<String,Integer> segmentReaders = new HashMap<>();
        +    final Map<String,Integer> segmentReaders = (oldReaders == null ? Collections.<String,Integer>emptyMap() : new HashMap<String,Integer>(oldReaders.size()));
        
        Show
        Christine Poerschke added a comment - Never mind parentheses, String,Integer needed for new HashMap<> also now. - final Map< String , Integer > segmentReaders = new HashMap<>(); + final Map< String , Integer > segmentReaders = (oldReaders == null ? Collections.< String , Integer >emptyMap() : new HashMap< String , Integer >(oldReaders.size()));
        Hide
        ASF subversion and git services added a comment -

        Commit 1712961 from Christine Poerschke in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1712961 ]

        LUCENE-6885: StandardDirectoryReader (initialCapacity) tweaks (merge in revision 1712939 from trunk)

        Show
        ASF subversion and git services added a comment - Commit 1712961 from Christine Poerschke in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1712961 ] LUCENE-6885 : StandardDirectoryReader (initialCapacity) tweaks (merge in revision 1712939 from trunk)

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development