Details

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

      Description

      This is unnecessary risk. We should remove this stuff, it is not needed here. We are a search engine, not a ram calculator.

      1. LUCENE-6239.patch
        20 kB
        Uwe Schindler
      2. LUCENE-6239.patch
        17 kB
        Uwe Schindler
      3. LUCENE-6239.patch
        16 kB
        Uwe Schindler

        Issue Links

          Activity

          Hide
          Uwe Schindler added a comment -

          Java 9 seems to make the constants public (at least there are some issues around making "Unsafe" public), so we can leave it for Java 7 and Java 8 as it is, so -1 to remove it here.

          In any case, Java 9 will not have sun.misc.Unsafe anymore - it will get removed soon (as far as I understood). I was talking with the OpenJDK guys 2 weeks ago; this also caused heavy rumors in the auditorium at FOSDEM

          In any case, I disagree on having just static constants that assume something in the JVM. Something like object headers have a large impact on heap usage of some of our Accountables.

          Show
          Uwe Schindler added a comment - Java 9 seems to make the constants public (at least there are some issues around making "Unsafe" public), so we can leave it for Java 7 and Java 8 as it is, so -1 to remove it here. In any case, Java 9 will not have sun.misc.Unsafe anymore - it will get removed soon (as far as I understood). I was talking with the OpenJDK guys 2 weeks ago; this also caused heavy rumors in the auditorium at FOSDEM In any case, I disagree on having just static constants that assume something in the JVM. Something like object headers have a large impact on heap usage of some of our Accountables.
          Hide
          Robert Muir added a comment -

          This code provides no value. We don't make small objects Accountable in general. I'll call a VOTE on the dev list, i think its important to avoid these unnecessary unsafe calls.

          Show
          Robert Muir added a comment - This code provides no value. We don't make small objects Accountable in general. I'll call a VOTE on the dev list, i think its important to avoid these unnecessary unsafe calls.
          Hide
          Robert Muir added a comment -

          My biggest problem: this stuff is called statically. If there is a bug, say compatibility bug, then lucene just flat out will not work. If there is a bug, say that crashes the JVM, then lucene will cause the jvm to crash.

          The risks are too much. Small objects dont really need to be Accountable, its more used for huge in-ram things.

          Show
          Robert Muir added a comment - My biggest problem: this stuff is called statically. If there is a bug, say compatibility bug, then lucene just flat out will not work. If there is a bug, say that crashes the JVM, then lucene will cause the jvm to crash. The risks are too much. Small objects dont really need to be Accountable, its more used for huge in-ram things.
          Hide
          Uwe Schindler added a comment - - edited

          There is one issue: The reference size. If you have an Object[] array, the compressed/uncompressed OPs difference is huge. I just checked the code with eclipse we still have that around.

          In any case, Unsafe is not used here to modify anything so there is nothing that can crush, we dont write bytes to non-aligned positions like Netty does and fails on Sparc. We just call some methods to retrieve some values from the JVM internal - there is no risk (it just may fail to get the value). I discussed that already with the JVM guys 2 weeks ago: We are perfectly safe here, so I don't understand why you suddenly want to remove that code which has no risk to actually break the JVM? As said, those per-JVM "constants" are hard to get otherwise.

          Show
          Uwe Schindler added a comment - - edited There is one issue: The reference size. If you have an Object[] array, the compressed/uncompressed OPs difference is huge. I just checked the code with eclipse we still have that around. In any case, Unsafe is not used here to modify anything so there is nothing that can crush, we dont write bytes to non-aligned positions like Netty does and fails on Sparc. We just call some methods to retrieve some values from the JVM internal - there is no risk (it just may fail to get the value). I discussed that already with the JVM guys 2 weeks ago: We are perfectly safe here, so I don't understand why you suddenly want to remove that code which has no risk to actually break the JVM? As said, those per-JVM "constants" are hard to get otherwise.
          Hide
          Robert Muir added a comment -

          In any case, Unsafe is not used here to modify anything so there is nothing that can crush, we dont write bytes to non-aligned positions like Netty does and fails on Sparc. We just call some methods to retrieve some values from the JVM internal - there is no risk (it just may fail to get the value).

          We don't even need to debate this. This is absurdly risky. Just look at the conversation you are having. We don't need this in a search engine library.

          Show
          Robert Muir added a comment - In any case, Unsafe is not used here to modify anything so there is nothing that can crush, we dont write bytes to non-aligned positions like Netty does and fails on Sparc. We just call some methods to retrieve some values from the JVM internal - there is no risk (it just may fail to get the value). We don't even need to debate this. This is absurdly risky. Just look at the conversation you are having. We don't need this in a search engine library.
          Hide
          Robert Muir added a comment -

          In any case, Unsafe is not used here to modify anything so there is nothing that can crush, we dont write bytes to non-aligned positions like Netty does and fails on Sparc.

          You only need to read to crash, if the architecture does not support unaligned access. The fact you don't know this, scares me, that you are also trying to write and defend sun.misc.unsafe code.

          Show
          Robert Muir added a comment - In any case, Unsafe is not used here to modify anything so there is nothing that can crush, we dont write bytes to non-aligned positions like Netty does and fails on Sparc. You only need to read to crash, if the architecture does not support unaligned access. The fact you don't know this, scares me, that you are also trying to write and defend sun.misc.unsafe code.
          Hide
          Uwe Schindler added a comment - - edited

          We don't even need to debate this. This is absurdly risky. Just look at the conversation you are having. We don't need this in a search engine library.

          Which conversation? There is a difference in calling stuff like Unsafe.writeFooBar or retruveing some values. We actually only read some offsets thats all. No risk. Where is your problem now. Who reported a crush? What line of code caused issues?

          I agree that code using unsafe to allocate off-heap memoy and directly modify values in memory using pointes is risky => see the funny test that forces to crush the JVM. Please see the difference here, its just some call to a read-only method that might disappear -> and we use reflection here to be prepared for that. But there is nothing that can crash.

          Actually the JVM people now have issues because the name "Unsafe" is just wrong - and thats what you are about (its called "unsafe" so it could break). Most of the stuff isn't actually unsafe, so they want to make it public in Java 9: Stuff like those constants or atomic compareAndGet - of course stuff writing and reading from actuall off-heap memory will be removed in Java 9 or made "safe".

          I just repeat: We dont read or write stuff! We just call "information" methods. Please read the code!

          Show
          Uwe Schindler added a comment - - edited We don't even need to debate this. This is absurdly risky. Just look at the conversation you are having. We don't need this in a search engine library. Which conversation? There is a difference in calling stuff like Unsafe.writeFooBar or retruveing some values. We actually only read some offsets thats all. No risk. Where is your problem now. Who reported a crush? What line of code caused issues? I agree that code using unsafe to allocate off-heap memoy and directly modify values in memory using pointes is risky => see the funny test that forces to crush the JVM. Please see the difference here, its just some call to a read-only method that might disappear -> and we use reflection here to be prepared for that. But there is nothing that can crash. Actually the JVM people now have issues because the name "Unsafe" is just wrong - and thats what you are about (its called "unsafe" so it could break). Most of the stuff isn't actually unsafe, so they want to make it public in Java 9: Stuff like those constants or atomic compareAndGet - of course stuff writing and reading from actuall off-heap memory will be removed in Java 9 or made "safe". I just repeat: We dont read or write stuff! We just call "information" methods. Please read the code!
          Hide
          Dawid Weiss added a comment -

          A similar argument could be made about the absolute need to compile and restrict the core to compact1 profile... why would you need that in a search library if it adds like a few milliseconds time at startup (once)?

          The code isn't useless, but it may be irrelevant. If there are any places in the code that measure reference arrays then I am for keeping the estimation heuristics in, they provide value. If there are no places doing that (other than logging) then I'd remove it.

          Show
          Dawid Weiss added a comment - A similar argument could be made about the absolute need to compile and restrict the core to compact1 profile... why would you need that in a search library if it adds like a few milliseconds time at startup (once)? The code isn't useless, but it may be irrelevant. If there are any places in the code that measure reference arrays then I am for keeping the estimation heuristics in, they provide value. If there are no places doing that (other than logging) then I'd remove it.
          Hide
          Uwe Schindler added a comment -

          I agree with Dawid! If Robert would not be so pessimistic and actually make us be "the bad" guys, I would be happy to talk with him.

          I am already validating most stuff and how we use the constants. We are indeed moving away from places where the actual values are relevant. The reference size is one of those that is actually relevant. And this one is just one Unsafe call to an informational method. The other ones just compare offset values - in any case there is nowhere any read/write to memory done. It is just calling some methods in a safe way. So I just have a problem with Robert's way of communicating this. We all know Robert, but I just prefer to not be called a guy who does not have an idea about memory models.

          Show
          Uwe Schindler added a comment - I agree with Dawid! If Robert would not be so pessimistic and actually make us be "the bad" guys, I would be happy to talk with him. I am already validating most stuff and how we use the constants. We are indeed moving away from places where the actual values are relevant. The reference size is one of those that is actually relevant. And this one is just one Unsafe call to an informational method. The other ones just compare offset values - in any case there is nowhere any read/write to memory done. It is just calling some methods in a safe way. So I just have a problem with Robert's way of communicating this. We all know Robert, but I just prefer to not be called a guy who does not have an idea about memory models.
          Hide
          Uwe Schindler added a comment -

          About the detection of compressed OOPs:
          We can do this with the HotspotBean (same way like Object Alignment). The code is already dynamic (see LUCENE-6069). We just can look for the VM option "CompressedOops", it it is true, we can assume 32 bits for reference size. This would not work on J9, but J9 is dead (at least in Trunk, because there is no Java 8 IBM J9),

          The other constants are not important.

          Robert: Would you be happy to assign sane defaults for all the constants and just do the HotspotMXBean trick to find out the Reference size? This one is uesed quite often in Lucene, because it has large impact for arrays of Object References. All other Unsafe code could be removed.

          Show
          Uwe Schindler added a comment - About the detection of compressed OOPs: We can do this with the HotspotBean (same way like Object Alignment). The code is already dynamic (see LUCENE-6069 ). We just can look for the VM option "CompressedOops", it it is true, we can assume 32 bits for reference size. This would not work on J9, but J9 is dead (at least in Trunk, because there is no Java 8 IBM J9), The other constants are not important. Robert: Would you be happy to assign sane defaults for all the constants and just do the HotspotMXBean trick to find out the Reference size? This one is uesed quite often in Lucene, because it has large impact for arrays of Object References. All other Unsafe code could be removed.
          Hide
          Robert Muir added a comment -

          I would love it if we avoided unsafe usage and replaced it with something safer like that.

          Show
          Robert Muir added a comment - I would love it if we avoided unsafe usage and replaced it with something safer like that.
          Hide
          Robert Muir added a comment -

          A similar argument could be made about the absolute need to compile and restrict the core to compact1 profile... why would you need that in a search library if it adds like a few milliseconds time at startup (once)?

          The main issue there is, people have complained in the past, that they cant use lucene on e.g. some mobile platform because it used XYZ api. My problem with supporting that in the past was, there was no way to "test" that used some restricted subset of the JDK apis.

          But now java 8 has this feature, which allows you to specify the subset, they provide this information in the javadocs, and the compiler will fail and all the infrastructure is in place, so I think we should only use what we need?

          I think claiming that this only saves a few milliseconds is incorrect, perhaps you should read the article on the motivation for these profiles:
          http://www.oracle.com/technetwork/articles/java/architect-profiles-2227131.html

          But these profiles are unrelated to this issue. In this issue i just want to remove unnecessary Unsafe calls. its far more critical because Unsafe is well, Unsafe.

          Show
          Robert Muir added a comment - A similar argument could be made about the absolute need to compile and restrict the core to compact1 profile... why would you need that in a search library if it adds like a few milliseconds time at startup (once)? The main issue there is, people have complained in the past, that they cant use lucene on e.g. some mobile platform because it used XYZ api. My problem with supporting that in the past was, there was no way to "test" that used some restricted subset of the JDK apis. But now java 8 has this feature, which allows you to specify the subset, they provide this information in the javadocs, and the compiler will fail and all the infrastructure is in place, so I think we should only use what we need? I think claiming that this only saves a few milliseconds is incorrect, perhaps you should read the article on the motivation for these profiles: http://www.oracle.com/technetwork/articles/java/architect-profiles-2227131.html But these profiles are unrelated to this issue. In this issue i just want to remove unnecessary Unsafe calls. its far more critical because Unsafe is well, Unsafe.
          Hide
          Dawid Weiss added a comment -

          Nah, sorry but the work-on-mobile argument is not really convincing. I've done a lot of work on constrained platforms and I really don't think anybody who embeds Lucene (for indexing or search) on such a platform is doing the right thing, unsafe has nothing to do with it – they'll have more problems than that to deal with...

          As for VM profiles... I know what they are. Which motivating element you think is specifically important other than the one resulting from potentially smaller memory load/ load time? Because I fail to see anything worthy mentioning other than that. With jigsaw already shipping you get most of the benefits of profiles anyway (these stem from the fact that there's no monolithic rt.jar to parse/ map into memory).

          Show
          Dawid Weiss added a comment - Nah, sorry but the work-on-mobile argument is not really convincing. I've done a lot of work on constrained platforms and I really don't think anybody who embeds Lucene (for indexing or search) on such a platform is doing the right thing, unsafe has nothing to do with it – they'll have more problems than that to deal with... As for VM profiles... I know what they are. Which motivating element you think is specifically important other than the one resulting from potentially smaller memory load/ load time? Because I fail to see anything worthy mentioning other than that. With jigsaw already shipping you get most of the benefits of profiles anyway (these stem from the fact that there's no monolithic rt.jar to parse/ map into memory).
          Hide
          Michael McCandless added a comment -

          +1 to stop using Unsafe and switch to dynamic hotspot bean check instead, or even a simpler/naive heuristic.

          If we have places in Lucene where estimating pointer size as 8 bytes when it was really 4 bytes in fact makes a practical difference (do we have big Object[] anywhere) then that's bad: most of Lucene's RAM heavy structures are also "compact" (not using so many pointers)?

          Show
          Michael McCandless added a comment - +1 to stop using Unsafe and switch to dynamic hotspot bean check instead, or even a simpler/naive heuristic. If we have places in Lucene where estimating pointer size as 8 bytes when it was really 4 bytes in fact makes a practical difference (do we have big Object[] anywhere) then that's bad: most of Lucene's RAM heavy structures are also "compact" (not using so many pointers)?
          Hide
          Robert Muir added a comment -

          Which motivating element you think is specifically important other than the one resulting from potentially smaller memory load/ load time? Because I fail to see anything worthy mentioning other than that. With jigsaw already shipping you get most of the benefits of profiles anyway (these stem from the fact that there's no monolithic rt.jar to parse/ map into memory).

          I think mike's response on that issue already explains my perspective on it. and to boot, by doing this i found a test bug as well (use of javax.management.Query when it should have been org.apache.lucene.search.Query). To me its just proper java 8 adoption, to define what portions of the JDK we are using. If there is really some feature we want in lucene-core that requires compact3 or whatever, i mean this is like a warning sign, why do we need such an advanced API to implement search?

          But please, this should really be discussed on LUCENE-6069.

          And I think your reasoning is slightly biased, we all know the only thing standing in the way of this stuff is RamUsageEstimator.

          Show
          Robert Muir added a comment - Which motivating element you think is specifically important other than the one resulting from potentially smaller memory load/ load time? Because I fail to see anything worthy mentioning other than that. With jigsaw already shipping you get most of the benefits of profiles anyway (these stem from the fact that there's no monolithic rt.jar to parse/ map into memory). I think mike's response on that issue already explains my perspective on it. and to boot, by doing this i found a test bug as well (use of javax.management.Query when it should have been org.apache.lucene.search.Query). To me its just proper java 8 adoption, to define what portions of the JDK we are using. If there is really some feature we want in lucene-core that requires compact3 or whatever, i mean this is like a warning sign, why do we need such an advanced API to implement search? But please, this should really be discussed on LUCENE-6069 . And I think your reasoning is slightly biased, we all know the only thing standing in the way of this stuff is RamUsageEstimator.
          Hide
          Uwe Schindler added a comment -

          I would love it if we avoided unsafe usage and replaced it with something safer like that.

          I can take care of that. Do you want me to make a patch removing Unsafe from trunk?

          Show
          Uwe Schindler added a comment - I would love it if we avoided unsafe usage and replaced it with something safer like that. I can take care of that. Do you want me to make a patch removing Unsafe from trunk?
          Hide
          Robert Muir added a comment -

          Yes, as said before I am +1 to your proposal. Its cleaner.

          Show
          Robert Muir added a comment - Yes, as said before I am +1 to your proposal. Its cleaner.
          Hide
          Uwe Schindler added a comment - - edited

          It is cleaner with Java 8 and Java 9. The reference size in Java 7 was just horrible to detect, because IBM J9 did not have hotspot bean. So the Unsafe approach was cleaner at that time. The remaining constants would be just simple static values, only dependend on bitness.

          Show
          Uwe Schindler added a comment - - edited It is cleaner with Java 8 and Java 9. The reference size in Java 7 was just horrible to detect, because IBM J9 did not have hotspot bean. So the Unsafe approach was cleaner at that time. The remaining constants would be just simple static values, only dependend on bitness.
          Hide
          Uwe Schindler added a comment -

          Path removing Unsafe.

          I also found out that Constants.Java also used Unsfae for the bitness. Now it uses solely sun.misc.data.model sysprop. I will investigate if we can get the information otherwise.

          Dawid Weiss: Can you look at the array header value? The one prevously looked strange to me, now the constant does the same as the comment says. I am not sure where the comment is documented, I assume, you wrote that.

          Show
          Uwe Schindler added a comment - Path removing Unsafe. I also found out that Constants.Java also used Unsfae for the bitness. Now it uses solely sun.misc.data.model sysprop. I will investigate if we can get the information otherwise. Dawid Weiss : Can you look at the array header value? The one prevously looked strange to me, now the constant does the same as the comment says. I am not sure where the comment is documented, I assume, you wrote that.
          Hide
          Robert Muir added a comment -

          +1

          Show
          Robert Muir added a comment - +1
          Hide
          Uwe Schindler added a comment - - edited

          Hi,
          I just found out: With Java 1.7+, all the Unsafe constants are exposed as public static final variables. So we dont need to directly access the unsafe instance object.

          By that it would be possible to get the REFERENCE_SIZE without hotspot bean just by getting a static final int constant... The same applies fo the JVM bitness.

          Would this be a valid use? In fact there can break nothing, it could just be that our code cannot see those constants, but thats not different from the HotspotBean. We are just reading a public static constant from Unsafe (via reflection).

          We just did not use that in RAMUsageEstimator before, because in Java 6, those constants were not there! On the other hand, in Java 9, Unsafe is likely to disappear, so I think we should really work without Unsafe.

          Show
          Uwe Schindler added a comment - - edited Hi, I just found out: With Java 1.7+, all the Unsafe constants are exposed as public static final variables. So we dont need to directly access the unsafe instance object. By that it would be possible to get the REFERENCE_SIZE without hotspot bean just by getting a static final int constant... The same applies fo the JVM bitness. Would this be a valid use? In fact there can break nothing, it could just be that our code cannot see those constants, but thats not different from the HotspotBean. We are just reading a public static constant from Unsafe (via reflection). We just did not use that in RAMUsageEstimator before, because in Java 6, those constants were not there! On the other hand, in Java 9, Unsafe is likely to disappear, so I think we should really work without Unsafe.
          Hide
          Uwe Schindler added a comment -

          New patch. I did additional comparisons with the Unsafe detected constants. I tested various JVMs, all is consistent now.

          I changed the code a little bit so 32 bit and 64 bits JVMs are handled separately. For 32 bit JVMs it does not even try to get the alignment size or compressed oops value. Also I fixed the array haeder, on 32 bit it is not aligned.

          I think it's ready, maybe Dawid Weiss can have a look, too.

          About backporting: We can do this, but reference size detection would not work correctly with IBM J9, so it would not detect compressed references there and always assume 64 bits. But J9 does not enable compressed refs by default...

          Show
          Uwe Schindler added a comment - New patch. I did additional comparisons with the Unsafe detected constants. I tested various JVMs, all is consistent now. I changed the code a little bit so 32 bit and 64 bits JVMs are handled separately. For 32 bit JVMs it does not even try to get the alignment size or compressed oops value. Also I fixed the array haeder, on 32 bit it is not aligned. I think it's ready, maybe Dawid Weiss can have a look, too. About backporting: We can do this, but reference size detection would not work correctly with IBM J9, so it would not detect compressed references there and always assume 64 bits. But J9 does not enable compressed refs by default...
          Hide
          Robert Muir added a comment -

          +1 to backport as well. If the reference size is wrong on IBM J9 it wont have a huge impact on the ramBytesUsed of lucene's data structures, as we have all mentioned on this issue.

          Furthermore, I don't know of a configuration of J9 that actually works right now, you will get false NPE's in normswriter when indexing, etc etc.

          Show
          Robert Muir added a comment - +1 to backport as well. If the reference size is wrong on IBM J9 it wont have a huge impact on the ramBytesUsed of lucene's data structures, as we have all mentioned on this issue. Furthermore, I don't know of a configuration of J9 that actually works right now, you will get false NPE's in normswriter when indexing, etc etc.
          Hide
          Dawid Weiss added a comment -

          The patch looks good.

          I still disagree with what's been said about the insecurity of our previous usage of Unsafe – we made safe measurements and fell back to sane defaults. No failure of this code was ever reported (and wouldn't be). Now we don't make measurements, but rely on (still sane!) assumptions. But these may or may not hold true; -XX:ObjectAlignmentInBytes=16...

          But ok, I shut up now.

          Show
          Dawid Weiss added a comment - The patch looks good. I still disagree with what's been said about the insecurity of our previous usage of Unsafe – we made safe measurements and fell back to sane defaults. No failure of this code was ever reported (and wouldn't be). Now we don't make measurements, but rely on (still sane!) assumptions. But these may or may not hold true; -XX:ObjectAlignmentInBytes=16 ... But ok, I shut up now.
          Hide
          Uwe Schindler added a comment -

          Hi Dawid,
          this was my intention behind the comment about "use constants": Since Java 7, Unsafe.java exposes most of the "static" stuff as public final static constants (like ADDRESS_SIZE,...). So theoretically, we could read those constants without risk. My problem is now more the fact that Unsafe is likely to disappear in Java 9 (they announced that at FOSDEM several times): "Make your code free of Unsafe, its will disappear - for sure!".
          Of course with the constants alone we cannot measure, as we did before. So I think we should not bother using Unsafe. I just hope that Java 9 brings the improvements we hope for, so we can then use "official" Java 9 APIs. For Java 7 and Java 8, I think the above assumptions still hold and are perfectly fine.
          The above assumptions may no longer hold when Java 10 (maybe Java 9?) brings the new Garbage Collector "Shenandoah", because this will add an additional indirection into the Object header (an additional pointer to the "live" version of an Object, so they can copy the object and update it concurrently).

          Show
          Uwe Schindler added a comment - Hi Dawid, this was my intention behind the comment about "use constants": Since Java 7, Unsafe.java exposes most of the "static" stuff as public final static constants (like ADDRESS_SIZE,...). So theoretically, we could read those constants without risk. My problem is now more the fact that Unsafe is likely to disappear in Java 9 (they announced that at FOSDEM several times): "Make your code free of Unsafe, its will disappear - for sure!". Of course with the constants alone we cannot measure, as we did before. So I think we should not bother using Unsafe. I just hope that Java 9 brings the improvements we hope for, so we can then use "official" Java 9 APIs. For Java 7 and Java 8, I think the above assumptions still hold and are perfectly fine. The above assumptions may no longer hold when Java 10 (maybe Java 9?) brings the new Garbage Collector "Shenandoah", because this will add an additional indirection into the Object header (an additional pointer to the "live" version of an Object, so they can copy the object and update it concurrently).
          Hide
          Uwe Schindler added a comment -

          But these may or may not hold true; -XX:ObjectAlignmentInBytes=16

          That works with current patch, I tested all combinations!

          Show
          Uwe Schindler added a comment - But these may or may not hold true; -XX:ObjectAlignmentInBytes=16 That works with current patch, I tested all combinations!
          Hide
          Dawid Weiss added a comment -

          Ah, sorry – I read the patch but missed this part:

          +          try {
          +            final Object vmOption = getVMOptionMethod.invoke(hotSpotBean, "ObjectAlignmentInBytes");
          +            objectAlignment = Integer.parseInt(
          +                vmOption.getClass().getMethod("getValue").invoke(vmOption).toString()
          +            );
          +          } catch (ReflectiveOperationException | IllegalArgumentException | SecurityException e) {
          +            // ignore
          +          }
          

          I'm fine with these changes, really. Thanks for exploring the alternatives, Uwe.

          Show
          Dawid Weiss added a comment - Ah, sorry – I read the patch but missed this part: + try { + final Object vmOption = getVMOptionMethod.invoke(hotSpotBean, "ObjectAlignmentInBytes" ); + objectAlignment = Integer .parseInt( + vmOption.getClass().getMethod( "getValue" ).invoke(vmOption).toString() + ); + } catch (ReflectiveOperationException | IllegalArgumentException | SecurityException e) { + // ignore + } I'm fine with these changes, really. Thanks for exploring the alternatives, Uwe.
          Hide
          Dawid Weiss added a comment -

          On a brighter note it just occurred to me that Unsafe can be described accurately as...
          http://en.wikipedia.org/wiki/Mostly_Harmless

          Show
          Dawid Weiss added a comment - On a brighter note it just occurred to me that Unsafe can be described accurately as... http://en.wikipedia.org/wiki/Mostly_Harmless
          Hide
          Uwe Schindler added a comment -

          New patch with some documentation improvements and a testcase that checks if Hotspot VM is used and validates that everything is sane. This test would fail if you corrupt the stringified method names.

          Show
          Uwe Schindler added a comment - New patch with some documentation improvements and a testcase that checks if Hotspot VM is used and validates that everything is sane. This test would fail if you corrupt the stringified method names.
          Hide
          ASF subversion and git services added a comment -

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

          LUCENE-6239: Removed RAMUsageEstimator's sun.misc.Unsafe calls

          Show
          ASF subversion and git services added a comment - Commit 1659303 from Uwe Schindler in branch 'dev/trunk' [ https://svn.apache.org/r1659303 ] LUCENE-6239 : Removed RAMUsageEstimator's sun.misc.Unsafe calls
          Hide
          ASF subversion and git services added a comment -

          Commit 1659305 from Uwe Schindler in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1659305 ]

          Merged revision(s) 1659303 from lucene/dev/trunk:
          LUCENE-6239: Removed RAMUsageEstimator's sun.misc.Unsafe calls

          Show
          ASF subversion and git services added a comment - Commit 1659305 from Uwe Schindler in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1659305 ] Merged revision(s) 1659303 from lucene/dev/trunk: LUCENE-6239 : Removed RAMUsageEstimator's sun.misc.Unsafe calls
          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:
              Uwe Schindler
              Reporter:
              Robert Muir
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development