Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.1, 6.0
    • Component/s: core/FSTs
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Current method to make a packed FST:
      1. Create an FST Builder with willPack=true, telling it you are later going to pack() it.
      2. Create your fst with finish() as normal.
      3. Take that fst, and call pack() on it to get another FST.

      This makes no sense. if you pass willPack=true, then I think finish() should just return a packed fst.

      1. LUCENE-4617.patch
        14 kB
        Robert Muir
      2. LUCENE-4617.patch
        5 kB
        Robert Muir

        Activity

        Hide
        Michael McCandless added a comment -

        +1!

        How silly

        Show
        Michael McCandless added a comment - +1! How silly
        Hide
        Uwe Schindler added a comment -

        J9 now works without excluding this method from JIT - and you really want to remove it??? - HIHI, how funny! (I am aware that it will survive inside another method).

        Show
        Uwe Schindler added a comment - J9 now works without excluding this method from JIT - and you really want to remove it??? - HIHI, how funny! (I am aware that it will survive inside another method).
        Hide
        Robert Muir added a comment -

        even as a start it could just become package-private and maybe called from finish() transparently.

        I want to fix the API as progress for LUCENE-4593, we can think of this packed thing as like a little "codec".

        I'm not sure why today Builder needs to know willPack=true up front, so its not obvious how hard this will be to factor out,
        or if its even possible, or the right thing to do.

        But i would just like some abstractions here in FSTs so we can do interesting things with them in the future.

        Show
        Robert Muir added a comment - even as a start it could just become package-private and maybe called from finish() transparently. I want to fix the API as progress for LUCENE-4593 , we can think of this packed thing as like a little "codec". I'm not sure why today Builder needs to know willPack=true up front, so its not obvious how hard this will be to factor out, or if its even possible, or the right thing to do. But i would just like some abstractions here in FSTs so we can do interesting things with them in the future.
        Hide
        Michael McCandless added a comment -

        But i would just like some abstractions here in FSTs so we can do interesting things with them in the future.

        +1

        even as a start it could just become package-private and maybe called from finish() transparently.

        Right we should be able to do that right away.

        I'm not sure why today Builder needs to know willPack=true up front, so its not obvious how hard this will be to factor out,
        or if its even possible, or the right thing to do.

        It's actually only the "mutable FST" that needs to know willPack, because in that mode it allocates two additional packed ints arrays (GrowableWriter) and dereferences node lookups through one of those ... the Builder itself doesn't need to know.

        Show
        Michael McCandless added a comment - But i would just like some abstractions here in FSTs so we can do interesting things with them in the future. +1 even as a start it could just become package-private and maybe called from finish() transparently. Right we should be able to do that right away. I'm not sure why today Builder needs to know willPack=true up front, so its not obvious how hard this will be to factor out, or if its even possible, or the right thing to do. It's actually only the "mutable FST" that needs to know willPack, because in that mode it allocates two additional packed ints arrays (GrowableWriter) and dereferences node lookups through one of those ... the Builder itself doesn't need to know.
        Hide
        Robert Muir added a comment -

        Here's a start. I have no idea what all these packing params are: so i stole the defaults from MemoryPostings.

        Show
        Robert Muir added a comment - Here's a start. I have no idea what all these packing params are: so i stole the defaults from MemoryPostings.
        Hide
        Michael McCandless added a comment -

        +1

        Show
        Michael McCandless added a comment - +1
        Hide
        Robert Muir added a comment -

        updated patch with fixed javadocs and fixing some tests (still calling the pkg-private pack before).

        I think its ready

        Show
        Robert Muir added a comment - updated patch with fixed javadocs and fixing some tests (still calling the pkg-private pack before). I think its ready
        Hide
        Michael McCandless added a comment -

        +1

        Show
        Michael McCandless added a comment - +1
        Hide
        Commit Tag Bot added a comment -

        [trunk commit] Robert Muir
        http://svn.apache.org/viewvc?view=revision&revision=1420951

        LUCENE-4617: remove fst.pack method

        Show
        Commit Tag Bot added a comment - [trunk commit] Robert Muir http://svn.apache.org/viewvc?view=revision&revision=1420951 LUCENE-4617 : remove fst.pack method
        Hide
        Commit Tag Bot added a comment -

        [trunk commit] Robert Muir
        http://svn.apache.org/viewvc?view=revision&revision=1420955

        LUCENE-4617: willPackFST -> doPackFST

        Show
        Commit Tag Bot added a comment - [trunk commit] Robert Muir http://svn.apache.org/viewvc?view=revision&revision=1420955 LUCENE-4617 : willPackFST -> doPackFST
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Robert Muir
        http://svn.apache.org/viewvc?view=revision&revision=1420958

        LUCENE-4617: remove fst.pack method

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Robert Muir http://svn.apache.org/viewvc?view=revision&revision=1420958 LUCENE-4617 : remove fst.pack method

          People

          • Assignee:
            Unassigned
            Reporter:
            Robert Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development