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

[Erik Joelsson]

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