Remove the assert in Integer.valueOf() (original) (raw)

Joseph Darcy joe.darcy at oracle.com
Fri Apr 27 23:32:43 UTC 2012

Hi Remi,

On 4/27/2012 3:38 PM, Rémi Forax wrote:

On 04/27/2012 05:29 AM, Joe Darcy wrote:

On 4/24/2012 8:01 AM, Alan Bateman wrote:

On 24/04/2012 14:56, Rémi Forax wrote:

Here, I don't really ask for tweaking something but more to remove an assert which do something which is unrelated to the current algorithm. In my opinion, it's a debug assert used during the development that slip into the production code. The fact that the range [-128, 127] should be covered by the cache is mandated by the JLS, but if you really want an assert you should move it when high and low are initialized (they are final) and not where they are used. I also agree that the inlining heuristic should be changed and I'm sure that everybody that have looked to the inlining heuristic code of Hotspot will agree with that but this is somehow unrelated to the problem. This is how I've found this assert, it doesn't change the fact that this assert should not be there. I added that assert as part of the work of 6807702. You can't use asserts in code that is executed before system initialization is complete and I think this is why we put the assert is valueOf rather than in the initializer. I don't have any objection to removing it, assuming Joe is okay with it too. -Alan [1] http://hg.openjdk.java.net/jdk8/tl/jdk/rev/4c3f752993a5 Hi Alan, I don't object to the assert being removed if asserts or assert-equivalent checks are added in the IntegerCache class. I have moved the assert into the static block of IntegerCache. For Alan, because IntegerCache is loaded when Integer.valueOf() is called the first time the assert code is checked around the same time so after the system init but only once. webrev is here: http://cr.openjdk.java.net/~forax/integervalueof/ and I need a bug for it :)

Here is your bug:

 7165102 Only run assertion on Integer autoboxing cache size once

However, I'd prefer the assert appear right after the

  777             high = h;

assignment.

Cheers,

-Joe

More information about the core-libs-dev mailing list