Lucene - Core
  1. Lucene - Core
  2. LUCENE-5775

JaspellTernarySearchTrie.ramBytesUsed hits StackOverflowError

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: 4.9, Trunk
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      I hit this when trying to run LookupBenchmarkTest for LUCENE-5752:

         [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=LookupBenchmarkTest -Dtests.method=testStorageNeeds -Dtests.seed=EA0FADB2EE37D385 -Dtests.locale=es_ES -Dtests.timezone=Etc/Greenwich -Dtests.file.encoding=UTF-8
         [junit4] ERROR   1.89s | LookupBenchmarkTest.testStorageNeeds <<<
         [junit4]    > Throwable #1: java.lang.StackOverflowError
         [junit4]    > 	at __randomizedtesting.SeedInfo.seed([EA0FADB2EE37D385:DF8106BCB29C472F]:0)
         [junit4]    > 	at java.lang.Class.getMethod0(Class.java:2774)
         [junit4]    > 	at java.lang.Class.isCheckMemberAccessOverridden(Class.java:2214)
         [junit4]    > 	at java.lang.Class.checkMemberAccess(Class.java:2233)
         [junit4]    > 	at java.lang.Class.getDeclaredFields(Class.java:1805)
         [junit4]    > 	at org.apache.lucene.util.RamUsageEstimator.shallowSizeOfInstance(RamUsageEstimator.java:351)
         [junit4]    > 	at org.apache.lucene.util.RamUsageEstimator.shallowSizeOf(RamUsageEstimator.java:329)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:100)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
         [junit4]    > 	at org.apache.lucene.search.suggest.jaspell.JaspellTernarySearchTrie$TSTNode.ramBytesUsed(JaspellTernarySearchTrie.java:103)
      

      I think we should just remove/deprecate this suggester? The FST based suggesters are far more RAM efficient...

        Issue Links

          Activity

          Hide
          ASF subversion and git services added a comment -

          Commit 1604711 from Uwe Schindler in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1604711 ]

          Merged revision(s) 1604710 from lucene/dev/trunk:
          Move changes of:
          SOLR-6178, LUCENE-5775: Deprecate JaspellLookupFactory

          Show
          ASF subversion and git services added a comment - Commit 1604711 from Uwe Schindler in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1604711 ] Merged revision(s) 1604710 from lucene/dev/trunk: Move changes of: SOLR-6178 , LUCENE-5775 : Deprecate JaspellLookupFactory
          Hide
          ASF subversion and git services added a comment -

          Commit 1604710 from Uwe Schindler in branch 'dev/trunk'
          [ https://svn.apache.org/r1604710 ]

          Move changes of:
          SOLR-6178, LUCENE-5775: Deprecate JaspellLookupFactory

          Show
          ASF subversion and git services added a comment - Commit 1604710 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1604710 ] Move changes of: SOLR-6178 , LUCENE-5775 : Deprecate JaspellLookupFactory
          Hide
          ASF subversion and git services added a comment -

          Commit 1604707 from Uwe Schindler in branch 'dev/branches/lucene_solr_4_9'
          [ https://svn.apache.org/r1604707 ]

          Revert:
          Merged revision(s) 1604122 from lucene/dev/trunk:
          SOLR-6178, LUCENE-5775: Deprecate JaspellLookupFactory

          Show
          ASF subversion and git services added a comment - Commit 1604707 from Uwe Schindler in branch 'dev/branches/lucene_solr_4_9' [ https://svn.apache.org/r1604707 ] Revert: Merged revision(s) 1604122 from lucene/dev/trunk: SOLR-6178 , LUCENE-5775 : Deprecate JaspellLookupFactory
          Hide
          ASF subversion and git services added a comment -

          Commit 1604125 from Uwe Schindler in branch 'dev/branches/lucene_solr_4_9'
          [ https://svn.apache.org/r1604125 ]

          Merged revision(s) 1604122 from lucene/dev/trunk:
          SOLR-6178, LUCENE-5775: Deprecate JaspellLookupFactory

          Show
          ASF subversion and git services added a comment - Commit 1604125 from Uwe Schindler in branch 'dev/branches/lucene_solr_4_9' [ https://svn.apache.org/r1604125 ] Merged revision(s) 1604122 from lucene/dev/trunk: SOLR-6178 , LUCENE-5775 : Deprecate JaspellLookupFactory
          Hide
          ASF subversion and git services added a comment -

          Commit 1604124 from Uwe Schindler in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1604124 ]

          Merged revision(s) 1604122 from lucene/dev/trunk:
          SOLR-6178, LUCENE-5775: Deprecate JaspellLookupFactory

          Show
          ASF subversion and git services added a comment - Commit 1604124 from Uwe Schindler in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1604124 ] Merged revision(s) 1604122 from lucene/dev/trunk: SOLR-6178 , LUCENE-5775 : Deprecate JaspellLookupFactory
          Hide
          ASF subversion and git services added a comment -

          Commit 1604122 from Uwe Schindler in branch 'dev/trunk'
          [ https://svn.apache.org/r1604122 ]

          SOLR-6178, LUCENE-5775: Deprecate JaspellLookupFactory

          Show
          ASF subversion and git services added a comment - Commit 1604122 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1604122 ] SOLR-6178 , LUCENE-5775 : Deprecate JaspellLookupFactory
          Hide
          Michael McCandless added a comment -

          OK I committed the deprecation to trunk; I'd really like to just remove it, but we can't do that until we address SOLR-6178 ... so I'll leave this issue open to remove Jaspell in trunk.

          Show
          Michael McCandless added a comment - OK I committed the deprecation to trunk; I'd really like to just remove it, but we can't do that until we address SOLR-6178 ... so I'll leave this issue open to remove Jaspell in trunk.
          Hide
          ASF subversion and git services added a comment -

          Commit 1603542 from Michael McCandless in branch 'dev/trunk'
          [ https://svn.apache.org/r1603542 ]

          LUCENE-5775: deprecate JaspellLookup

          Show
          ASF subversion and git services added a comment - Commit 1603542 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1603542 ] LUCENE-5775 : deprecate JaspellLookup
          Hide
          Michael McCandless added a comment -

          I opened SOLR-6178

          Show
          Michael McCandless added a comment - I opened SOLR-6178
          Hide
          Michael McCandless added a comment -

          So maybe we should change defaults. Should we open SOLR issue?

          OK I'll open a Solr issue.

          Show
          Michael McCandless added a comment - So maybe we should change defaults. Should we open SOLR issue? OK I'll open a Solr issue.
          Hide
          Uwe Schindler added a comment -

          With Eclipse it looks like this should be enough to deprecate the factory you mentioned. But the default lookup for file-based stuff in Solr is still Jaspell:

          ./core/src/java/org/apache/solr/spelling/suggest/jaspell/JaspellLookupFactory.java:public class JaspellLookupFactory extends LookupFactory {
          ./core/src/java/org/apache/solr/spelling/suggest/LookupFactory.java:import org.apache.solr.spelling.suggest.jaspell.JaspellLookupFactory;
          ./core/src/java/org/apache/solr/spelling/suggest/LookupFactory.java:  public static String DEFAULT_FILE_BASED_DICT = JaspellLookupFactory.class.getName();
          ./core/src/java/org/apache/solr/spelling/suggest/Suggester.java:import org.apache.solr.spelling.suggest.jaspell.JaspellLookupFactory;
          ./core/src/java/org/apache/solr/spelling/suggest/Suggester.java:      lookupImpl = JaspellLookupFactory.class.getName();
          

          So maybe we should change defaults. Should we open SOLR issue?

          Show
          Uwe Schindler added a comment - With Eclipse it looks like this should be enough to deprecate the factory you mentioned. But the default lookup for file-based stuff in Solr is still Jaspell: ./core/src/java/org/apache/solr/spelling/suggest/jaspell/JaspellLookupFactory.java:public class JaspellLookupFactory extends LookupFactory { ./core/src/java/org/apache/solr/spelling/suggest/LookupFactory.java:import org.apache.solr.spelling.suggest.jaspell.JaspellLookupFactory; ./core/src/java/org/apache/solr/spelling/suggest/LookupFactory.java: public static String DEFAULT_FILE_BASED_DICT = JaspellLookupFactory.class.getName(); ./core/src/java/org/apache/solr/spelling/suggest/Suggester.java:import org.apache.solr.spelling.suggest.jaspell.JaspellLookupFactory; ./core/src/java/org/apache/solr/spelling/suggest/Suggester.java: lookupImpl = JaspellLookupFactory.class.getName(); So maybe we should change defaults. Should we open SOLR issue?
          Hide
          Uwe Schindler added a comment -

          I am trying to check that out, too

          Show
          Uwe Schindler added a comment - I am trying to check that out, too
          Hide
          Michael McCandless added a comment -

          OK will do; just add @deprecated / @Deprecated to JaspellLookupFactory right?

          Show
          Michael McCandless added a comment - OK will do; just add @deprecated / @Deprecated to JaspellLookupFactory right?
          Hide
          Uwe Schindler added a comment -

          Hi,
          should we maybe also deprecate the Solr factories? Because then Solr prints a warning on startup if they are used.

          Show
          Uwe Schindler added a comment - Hi, should we maybe also deprecate the Solr factories? Because then Solr prints a warning on startup if they are used.
          Hide
          ASF subversion and git services added a comment -

          Commit 1603523 from Michael McCandless in branch 'dev/branches/branch_4x'
          [ https://svn.apache.org/r1603523 ]

          LUCENE-5775: Deprecate JaspellLookup; fix its ramBytesUsed to not StackOverflow

          Show
          ASF subversion and git services added a comment - Commit 1603523 from Michael McCandless in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1603523 ] LUCENE-5775 : Deprecate JaspellLookup; fix its ramBytesUsed to not StackOverflow
          Hide
          ASF subversion and git services added a comment -

          Commit 1603521 from Michael McCandless in branch 'dev/branches/lucene_solr_4_9'
          [ https://svn.apache.org/r1603521 ]

          LUCENE-5775: Deprecate JaspellLookup; fix its ramBytesUsed to not StackOverflow

          Show
          ASF subversion and git services added a comment - Commit 1603521 from Michael McCandless in branch 'dev/branches/lucene_solr_4_9' [ https://svn.apache.org/r1603521 ] LUCENE-5775 : Deprecate JaspellLookup; fix its ramBytesUsed to not StackOverflow
          Hide
          Michael McCandless added a comment -

          Simple patch fixes it:

          Index: lucene/suggest/src/java/org/apache/lucene/search/suggest/jaspell/JaspellTernarySearchTrie.java
          ===================================================================
          --- lucene/suggest/src/java/org/apache/lucene/search/suggest/jaspell/JaspellTernarySearchTrie.java	(revision 1603490)
          +++ lucene/suggest/src/java/org/apache/lucene/search/suggest/jaspell/JaspellTernarySearchTrie.java	(working copy)
          @@ -98,7 +98,9 @@
               @Override
               public long ramBytesUsed() {
                 long mem = RamUsageEstimator.shallowSizeOf(this) + RamUsageEstimator.shallowSizeOf(relatives);
          -      for (TSTNode node : relatives) {
          +      // We don't need to add parent since our parent added itself:
          +      for (int i=1;i<4;i++) {
          +        TSTNode node = relatives[i];
                   if (node != null) {
                     mem += node.ramBytesUsed();
                   }
          

          Basically just sum up "downwards" in the tree ... I'll commit this & deprecate in 4.x, and remove in trunk.

          Show
          Michael McCandless added a comment - Simple patch fixes it: Index: lucene/suggest/src/java/org/apache/lucene/search/suggest/jaspell/JaspellTernarySearchTrie.java =================================================================== --- lucene/suggest/src/java/org/apache/lucene/search/suggest/jaspell/JaspellTernarySearchTrie.java (revision 1603490) +++ lucene/suggest/src/java/org/apache/lucene/search/suggest/jaspell/JaspellTernarySearchTrie.java (working copy) @@ -98,7 +98,9 @@ @Override public long ramBytesUsed() { long mem = RamUsageEstimator.shallowSizeOf(this) + RamUsageEstimator.shallowSizeOf(relatives); - for (TSTNode node : relatives) { + // We don't need to add parent since our parent added itself: + for (int i=1;i<4;i++) { + TSTNode node = relatives[i]; if (node != null) { mem += node.ramBytesUsed(); } Basically just sum up "downwards" in the tree ... I'll commit this & deprecate in 4.x, and remove in trunk.

            People

            • Assignee:
              Michael McCandless
              Reporter:
              Michael McCandless
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:

                Development