Lucene - Core
  1. Lucene - Core
  2. LUCENE-5844

ArrayUtil.grow should not pretend you can actually allocate array[Integer.MAX_VALUE]

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.9.1, 4.10, 6.0
    • Component/s: core/other
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Today if the growth it wants would exceed Integer.MAX_VALUE, it returns Integer.MAX_VALUE, but you can't actually allocate arrays this large; the actual limit is JVM dependent and varies across JVMs ...

      It would be nice if we could somehow "introspect" the JVM to find out what its actual limit is and use that. http://stackoverflow.com/questions/3038392/do-java-arrays-have-a-maximum-size seems to imply that using Integer.MAX_VALUE - 8 may be "safe" (it's what ArrayList.java apparently uses).

      1. LUCENE-5844.patch
        5 kB
        Michael McCandless
      2. LUCENE-5844.patch
        3 kB
        Michael McCandless

        Activity

        Hide
        Robert Muir added a comment -

        +1, i hit this today when trying to corrupt my stored fields: I can't make a document with 2^31 - 2^14 + 1 bytes, because it tries to overallocate to a bigger array that java cant actually make. so today grow() limts you to Integer.MAX_VALUE - (Integer.MAX_VALUE/8) or something in practice.

        Show
        Robert Muir added a comment - +1, i hit this today when trying to corrupt my stored fields: I can't make a document with 2^31 - 2^14 + 1 bytes, because it tries to overallocate to a bigger array that java cant actually make. so today grow() limts you to Integer.MAX_VALUE - (Integer.MAX_VALUE/8) or something in practice.
        Hide
        Michael McCandless added a comment -

        Simple patch, I used Integer.MAX_VALUE-8, and added a couple tests.

        Show
        Michael McCandless added a comment - Simple patch, I used Integer.MAX_VALUE-8, and added a couple tests.
        Hide
        Michael McCandless added a comment -

        New patch, just simplifying / fixing confusing comment in PriorityQueue.java...

        Show
        Michael McCandless added a comment - New patch, just simplifying / fixing confusing comment in PriorityQueue.java...
        Hide
        Robert Muir added a comment -

        +1

        Show
        Robert Muir added a comment - +1
        Hide
        ASF subversion and git services added a comment -

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

        LUCENE-5844: ArrayUtil.grow/oversize now returns at most Integer.MAX_VALUE - 8

        Show
        ASF subversion and git services added a comment - Commit 1612935 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1612935 ] LUCENE-5844 : ArrayUtil.grow/oversize now returns at most Integer.MAX_VALUE - 8
        Hide
        ASF subversion and git services added a comment -

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

        LUCENE-5844: ArrayUtil.grow/oversize now returns at most Integer.MAX_VALUE - 8

        Show
        ASF subversion and git services added a comment - Commit 1612936 from Michael McCandless in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1612936 ] LUCENE-5844 : ArrayUtil.grow/oversize now returns at most Integer.MAX_VALUE - 8
        Hide
        Dawid Weiss added a comment -

        Why a constant 8 in:

         Integer.MAX_VALUE - 8
        

        Couldn't this be somehow linked up to array header overhead in RUE? I didn't check the impl. in openjdk, but I think it probably is related to that (so that max. offset from object start is at most 2^32-1)?

        Show
        Dawid Weiss added a comment - Why a constant 8 in: Integer .MAX_VALUE - 8 Couldn't this be somehow linked up to array header overhead in RUE? I didn't check the impl. in openjdk, but I think it probably is related to that (so that max. offset from object start is at most 2^32-1)?
        Hide
        Michael McCandless added a comment -

        Couldn't this be somehow linked up to array header overhead in RUE?

        +1, the comment in ArrayList.java (OpenJDK 7) is:

        190 /**
        191 * The maximum size of array to allocate.
        192 * Some VMs reserve some header words in an array.
        193 * Attempts to allocate larger arrays may result in
        194 * OutOfMemoryError: Requested array size exceeds VM limit
        195 */
        196 private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8;

        So we could just change the 8 to RUE.NUM_BYTES_ARRAY_HEADER? Or maybe we can sneak in and try to access this private MAX_ARRAY_SIZE...?

        Show
        Michael McCandless added a comment - Couldn't this be somehow linked up to array header overhead in RUE? +1, the comment in ArrayList.java (OpenJDK 7) is: 190 /** 191 * The maximum size of array to allocate. 192 * Some VMs reserve some header words in an array. 193 * Attempts to allocate larger arrays may result in 194 * OutOfMemoryError: Requested array size exceeds VM limit 195 */ 196 private static final int MAX_ARRAY_SIZE = Integer.MAX_VALUE - 8; So we could just change the 8 to RUE.NUM_BYTES_ARRAY_HEADER? Or maybe we can sneak in and try to access this private MAX_ARRAY_SIZE...?
        Hide
        Dawid Weiss added a comment -

        I'd say we could just use RUE.NUM_BYTES_ARRAY_HEADER as the default, explaining the rationale and pointing at ArrayList#MAX_ARRAY_SIZE? It seems more elegant to me to actually explain how the max number is reached.

        Show
        Dawid Weiss added a comment - I'd say we could just use RUE.NUM_BYTES_ARRAY_HEADER as the default, explaining the rationale and pointing at ArrayList#MAX_ARRAY_SIZE? It seems more elegant to me to actually explain how the max number is reached.
        Hide
        Michael McCandless added a comment -

        OK will do!

        Show
        Michael McCandless added a comment - OK will do!
        Hide
        ASF subversion and git services added a comment -

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

        LUCENE-5844: use RUE.NUM_BYTES_ARRAY_HEADER instead of 8

        Show
        ASF subversion and git services added a comment - Commit 1613043 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1613043 ] LUCENE-5844 : use RUE.NUM_BYTES_ARRAY_HEADER instead of 8
        Hide
        ASF subversion and git services added a comment -

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

        LUCENE-5844: use RUE.NUM_BYTES_ARRAY_HEADER instead of 8

        Show
        ASF subversion and git services added a comment - Commit 1613044 from Michael McCandless in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1613044 ] LUCENE-5844 : use RUE.NUM_BYTES_ARRAY_HEADER instead of 8
        Hide
        Michael McCandless added a comment -

        Reopen for backport to 4.9.1...

        Show
        Michael McCandless added a comment - Reopen for backport to 4.9.1...
        Hide
        ASF subversion and git services added a comment -

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

        LUCENE-5844: backport to 4.9.1

        Show
        ASF subversion and git services added a comment - Commit 1625417 from Michael McCandless in branch 'dev/branches/lucene_solr_4_9' [ https://svn.apache.org/r1625417 ] LUCENE-5844 : backport to 4.9.1
        Hide
        Michael McCandless added a comment -

        Bulk close for Lucene/Solr 4.9.1 release

        Show
        Michael McCandless added a comment - Bulk close for Lucene/Solr 4.9.1 release

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development