Remove the assert in Integer.valueOf()

Rémi Forax forax at univ-mlv.fr
Tue Apr 24 13:56:24 UTC 2012


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 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.

Rémi

>
>
> 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.
>> [...]
>> 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 escape analysis will not remove the allocation,
>> I think it's a good idea to comment this assert.
>>
>> cheers,
>> Rémi




More information about the core-libs-dev mailing list