RFR: 8014513: Sjavac doesn't detect 32-bit jvm properly

[Erik Joelsson]

Trying again. I would like to get a formal review on this if possible. 
Jon, you gave ok to the first version, but as I made some changes based 
on your comments, I assumed another round was needed.

/Erik

On 2013-06-13 09:42, Erik Joelsson wrote:
> Would be great if I could get a final ok on this, thanks.
>
> /Erik
>
> On 2013-06-07 13:08, Erik Joelsson wrote:
>>
>>
>> On 2013-06-05 20:40, Jonathan Gibbons wrote:
>>> On 06/05/2013 11:38 AM, Jonathan Gibbons wrote:
>>>> On 06/05/2013 07:24 AM, Erik Joelsson wrote:
>>>>> Hello,
>>>>>
>>>>> Here is a small patch fixing a problem with sjavac. Sjavac tries 
>>>>> to be smart about sizing itself to no run out of memory. The logic 
>>>>> for figuring out if it's running on a 32 or 64bit jvm is rather 
>>>>> crude and misses on several platforms. This patch makes the code 
>>>>> more robust.
>>>>>
>>>>> Fixing this issue is a requirement to get sjavac to work well on 
>>>>> all platforms in jprt.
>>>>>
>>>>> Bug: http://bugs.sun.com/view_bug.do?bug_id=8014513
>>>>>
>>>>> Webrev: 
>>>>> http://cr.openjdk.java.net/~erikj/8014513/webrev.langtools.01/
>>>>>
>>>>> /Erik
>>>>
>>>> Erik,
>>>>
>>>> It's OK, but I think it would be better if there was a back door to 
>>>> override the default value.  That would also partially address the 
>>>> comment in the bug about sjavac looking at the wrong JVM -- it's 
>>>> using the client process, not the server process.
>>>>
>>>> If you push it as is, I suggest you file a followup bug for further 
>>>> improvements.
>>>>
>>>> -- Jon
>>>
>>> Erik,
>>>
>>> The code might also look better if you used a Set<String> for 32 bit 
>>> values, and tested for set.contains(dataModel), or if not that, the 
>>> use a strings-in-switch statement.
>>>
>> Perhaps I'm missing something or I'm not familiar enough with newer 
>> features in the language, but I can't get initialization of a set of 
>> 3 Strings to feel more readable and less clumsy than the list of 
>> logical ORs. The problem with going switch is that there are two 
>> different Strings being tested.
>>
>> Thinking on this some more, I think I was mistaken. We don't need to 
>> test for both arch and data model. I was just assuming that 
>> "sun.arch.data.model" might not be as reliable as "os.arch". On the 
>> other hand, the heuristics here are rather specific to using hotspot 
>> and open/oraclejdk, so for all practical cases, using 
>> sun.arch.data.model should be safe. New webrev using just data.model:
>>
>> http://cr.openjdk.java.net/~erikj/8014513/webrev.langtools.02/
>>
>> It would be good to provide hooks to control this behavior of sjavac. 
>> Will file a followup bug for that.
>>
>> /Erik
>>> -- Jon