Re: Remove the assert in Integer.valueOf()

2012-05-01 Thread Rémi Forax
On 05/01/2012 12:09 AM, Ulf Zibis wrote: Hi Rémi, Am 28.04.2012 00:42, schrieb Rémi Forax: While you are there: IntegerCache.cache/high/low are static final, so should be named _upper case_. Not done because the system property is also spelled in lower case. Hm, but does that justify

Re: Remove the assert in Integer.valueOf()

2012-04-30 Thread Alan Bateman
On 27/04/2012 23:38, Rémi Forax wrote: : 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

Re: Remove the assert in Integer.valueOf()

2012-04-30 Thread Ulf Zibis
Hi Rémi, Am 28.04.2012 00:42, schrieb Rémi Forax: While you are there: IntegerCache.cache/high/low are static final, so should be named _upper case_. Not done because the system property is also spelled in lower case. Hm, but does that justify violating very common code conventions? IMO it's

Re: Remove the assert in Integer.valueOf()

2012-04-27 Thread Ulf Zibis
Am 23.04.2012 19:35, schrieb Rémi Forax: Hi guys, I've found a case where assert is harmful because it doesn't play well with Hotspot inlining heuristic. [...] I think it's a good idea to comment this assert. While you are there: IntegerCache.cache/high/low are static final, so should be named

Re: Remove the assert in Integer.valueOf()

2012-04-27 Thread Rémi Forax
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

Re: Remove the assert in Integer.valueOf()

2012-04-27 Thread Rémi Forax
On 04/27/2012 02:14 PM, Ulf Zibis wrote: Am 23.04.2012 19:35, schrieb Rémi Forax: Hi guys, I've found a case where assert is harmful because it doesn't play well with Hotspot inlining heuristic. [...] I think it's a good idea to comment this assert. Hi Ulf, While you are there:

Re: Remove the assert in Integer.valueOf()

2012-04-27 Thread Joseph Darcy
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

Re: Remove the assert in Integer.valueOf()

2012-04-26 Thread Joe Darcy
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

Re: Remove the assert in Integer.valueOf()

2012-04-24 Thread Rémi Forax
On 04/24/2012 02:12 AM, Ulf Zibis wrote: Hi Rémi, I think, instead tweaking the java code, Hotspot inlining heuristic should better be changed to count the bytes of the compiled code. Than any code would benefit from, not only Integer.valueOf(). -Ulf Here, I don't really ask for tweaking

Re: Remove the assert in Integer.valueOf()

2012-04-24 Thread Alan Bateman
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

Re: Remove the assert in Integer.valueOf()

2012-04-23 Thread Mario Torre
2012/4/23 Rémi Forax fo...@univ-mlv.fr: The issue is that Hotspot also count the bytecodes related to assert in its inlining heuristic. If the assert is commented, the inlining tree is good. [...] Given that Integer.valueOf() is a method used very often and that if the inlining fails, the

Re: Remove the assert in Integer.valueOf()

2012-04-23 Thread Rémi Forax
On 04/23/2012 07:43 PM, Mario Torre wrote: 2012/4/23 Rémi Foraxfo...@univ-mlv.fr: The issue is that Hotspot also count the bytecodes related to assert in its inlining heuristic. If the assert is commented, the inlining tree is good. [...] Given that Integer.valueOf() is a method used very

Re: Remove the assert in Integer.valueOf()

2012-04-23 Thread Alex Lam S.L.
Just curious - I am assuming that assertions are disabled during the test runs, so wouldn't one expect the assert statements to be ignored / removed? Obviously it didn't in this case, yet I thought we are expecting constant conditionals to be optimised, e.g. if (a == null) {...} to be removed if

Re: Remove the assert in Integer.valueOf()

2012-04-23 Thread Rémi Forax
On 04/24/2012 12:32 AM, Alex Lam S.L. wrote: Just curious - I am assuming that assertions are disabled during the test runs, so wouldn't one expect the assert statements to be ignored / removed? Obviously it didn't in this case, yet I thought we are expecting constant conditionals to be

Re: Remove the assert in Integer.valueOf()

2012-04-23 Thread Ulf Zibis
Hi Rémi, I think, instead tweaking the java code, Hotspot inlining heuristic should better be changed to count the bytes of the compiled code. Than any code would benefit from, not only Integer.valueOf(). -Ulf Am 23.04.2012 19:35, schrieb Rémi Forax: Hi guys, I've found a case where assert