Lucene - Core
  1. Lucene - Core
  2. LUCENE-6242

SparseFixedBitDocIdSet.ramBytesUsed() reports wrong size if alignment of JVM is not 8

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.1
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      There seems to be a bug in SparseFixedBitDocIdSet's ramBytesUsed. To me this is a bit crazy implemented, so I have not yet found the issue. To me it looks like some of the summing up breaks if the alignment of the JVM is not 8:

         [junit4] Suite: org.apache.lucene.util.TestSparseFixedBitDocIdSet
         [junit4]   2> NOTE: reproduce with: ant test  -Dtestcase=TestSparseFixedBitDocIdSet -Dtests.method=testRamBytesUsed -Dtests.seed=
      C1F3B881CB1C5E8A -Dtests.locale=es_BO -Dtests.timezone=America/Detroit -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1
         [junit4] FAILURE 0.06s J1 | TestSparseFixedBitDocIdSet.testRamBytesUsed <<<
         [junit4]    > Throwable #1: java.lang.AssertionError: expected:<264> but was:<256>
         [junit4]    >        at __randomizedtesting.SeedInfo.seed([C1F3B881CB1C5E8A:3350AAC1016341DC]:0)
         [junit4]    >        at org.apache.lucene.util.BaseDocIdSetTestCase.testRamBytesUsed(BaseDocIdSetTestCase.java:104)
         [junit4]    >        at java.lang.Thread.run(Thread.java:745)
      

      To reproduce this failure, run inside core (with 64 bits JVM):

      ant test -Dtestcase=TestSparseFixedBitDocIdSet -Dtests.method=testRamBytesUsed -Dargs="-XX:ObjectAlignmentInBytes=16" -Dtests.seed=C1F3B881CB1C5E8A
      

      The default works:

      ant test -Dtestcase=TestSparseFixedBitDocIdSet -Dtests.method=testRamBytesUsed -Dargs="-XX:ObjectAlignmentInBytes=8" -Dtests.seed=C1F3B881CB1C5E8A
      

      I think we should randomly also specify the ObjectAlignmentInBytes for test runs on Policeman Jenkins. Any Power of 2 is fine.

      1. LUCENE-6242.patch
        0.7 kB
        Adrien Grand

        Activity

        Hide
        Dawid Weiss added a comment -

        Just a note – I was toying with alignments a while ago and I had random JVM crashes.

        Show
        Dawid Weiss added a comment - Just a note – I was toying with alignments a while ago and I had random JVM crashes.
        Hide
        Uwe Schindler added a comment -

        Oha I assume for large ones or also with stuff like 16?

        At least we should fix this issue here. If we enable random alignments could be decided after some local tests. In any case I would enable this only very seldom with values 8 <= x <= 128.

        Show
        Uwe Schindler added a comment - Oha I assume for large ones or also with stuff like 16? At least we should fix this issue here. If we enable random alignments could be decided after some local tests. In any case I would enable this only very seldom with values 8 <= x <= 128.
        Hide
        Dawid Weiss added a comment -

        Definitely the larger ones (> 32 from what I recall).

        Show
        Dawid Weiss added a comment - Definitely the larger ones (> 32 from what I recall).
        Hide
        Adrien Grand added a comment -

        Thanks Uwe, I will look into this issue.

        Show
        Adrien Grand added a comment - Thanks Uwe, I will look into this issue.
        Hide
        Adrien Grand added a comment -

        Here is a patch.

        Show
        Adrien Grand added a comment - Here is a patch.
        Hide
        Uwe Schindler added a comment -

        Hehe, I was expecting that something like this is missing, I assumed that the Length-1-Longs are the bad ones. I have no idea if this fix is correct, because I have no idea how the ramBytesUsed are calculated

        Did you check some values of the alignment and ran it with "test beast" on several values of alignment?

        Show
        Uwe Schindler added a comment - Hehe, I was expecting that something like this is missing, I assumed that the Length-1-Longs are the bad ones. I have no idea if this fix is correct, because I have no idea how the ramBytesUsed are calculated Did you check some values of the alignment and ran it with "test beast" on several values of alignment?
        Hide
        Adrien Grand added a comment -

        I just beasted (50 iters) with alignements of 8, 16, 32, 64 and 128 and tests passed.

        Show
        Adrien Grand added a comment - I just beasted (50 iters) with alignements of 8, 16, 32, 64 and 128 and tests passed.
        Hide
        Uwe Schindler added a comment -

        Yeah, I think I understand the logic now. Looks good to me!
        If you tested already I am more than fine with this patch!

        Show
        Uwe Schindler added a comment - Yeah, I think I understand the logic now. Looks good to me! If you tested already I am more than fine with this patch!
        Hide
        Robert Muir added a comment -

        At least we should fix this issue here. If we enable random alignments could be decided after some local tests. In any case I would enable this only very seldom with values 8 <= x <= 128.

        You can do what you like with your jenkins, but if the failures aren't using the default alignment, I won't even look at them.

        I don't think we should test this.

        Show
        Robert Muir added a comment - At least we should fix this issue here. If we enable random alignments could be decided after some local tests. In any case I would enable this only very seldom with values 8 <= x <= 128. You can do what you like with your jenkins, but if the failures aren't using the default alignment, I won't even look at them. I don't think we should test this.
        Hide
        Uwe Schindler added a comment -

        I would only look at them if they are related to ramBytesUsed calculations This was my intention in running those tests.

        I agree, we should not really randomize those. I am already annoyed by G1GC tests!

        Show
        Uwe Schindler added a comment - I would only look at them if they are related to ramBytesUsed calculations This was my intention in running those tests. I agree, we should not really randomize those. I am already annoyed by G1GC tests!
        Hide
        ASF subversion and git services added a comment -

        Commit 1659273 from Adrien Grand in branch 'dev/trunk'
        [ https://svn.apache.org/r1659273 ]

        LUCENE-6242: Fix SparseFixedBitSet ram usage estimation when object alignment is different from 8.

        Show
        ASF subversion and git services added a comment - Commit 1659273 from Adrien Grand in branch 'dev/trunk' [ https://svn.apache.org/r1659273 ] LUCENE-6242 : Fix SparseFixedBitSet ram usage estimation when object alignment is different from 8.
        Hide
        Robert Muir added a comment -

        I am already annoyed by G1GC tests!

        Yes, thats exactly my problem. When looking at any test failure that has G1GC, i know there is a 50% chance (at least) that I am completely wasting my time. Many days, i see G1GC was in use, and just move on with my day, because I simply dont have the time to waste.

        Show
        Robert Muir added a comment - I am already annoyed by G1GC tests! Yes, thats exactly my problem. When looking at any test failure that has G1GC, i know there is a 50% chance (at least) that I am completely wasting my time. Many days, i see G1GC was in use, and just move on with my day, because I simply dont have the time to waste.
        Hide
        Uwe Schindler added a comment -

        I think we should still run the G1GC tests to be able to show to people, that it is currently a bad idea to run with G1GC.

        Show
        Uwe Schindler added a comment - I think we should still run the G1GC tests to be able to show to people, that it is currently a bad idea to run with G1GC.
        Hide
        ASF subversion and git services added a comment -

        Commit 1659278 from Adrien Grand in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1659278 ]

        LUCENE-6242: Fix SparseFixedBitSet ram usage estimation when object alignment is different from 8.

        Show
        ASF subversion and git services added a comment - Commit 1659278 from Adrien Grand in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1659278 ] LUCENE-6242 : Fix SparseFixedBitSet ram usage estimation when object alignment is different from 8.
        Hide
        Timothy Potter added a comment -

        Bulk close after 5.1 release

        Show
        Timothy Potter added a comment - Bulk close after 5.1 release

          People

          • Assignee:
            Unassigned
            Reporter:
            Uwe Schindler
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development