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

[Erik Joelsson]

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