RFR: 8067951: System.loadLibrary cannot find library when path contains quoted entry

[Roger]

Hi Ivan,

Looks pretty good.

Just a thought on ClassLoader:1754;  If you avoid the local for 
ClassLoaderHelper.allowsQuotedPathElements
then the compiler can optimize the code and may do dead code elimination 
since it is a final static boolean.
On Mac and Unix it reduces to just the original code.

(ClassLoaderHelper seems like a weak case for a separate class, IMHO)

$.02, Roger


On 1/5/2015 12:49 PM, Ivan Gerasimov wrote:
> Hi Roger!
>
> I've updated the webrev similarly to what you've suggested:
> 1) Presence of quotes is checked when counting the separators.
> 2) If quotes weren't found, we'll execute the same optimized loop as 
> for Unix.
>
> http://cr.openjdk.java.net/~igerasim/8067951/4/webrev/
>
> Sincerely yours,
> Ivan
>
>
> On 05.01.2015 18:40, Roger wrote:
>> Hi Ivan,
>>
>> For this small difference in the implementations, I'd recommend 
>> against having
>> two different source files.  The path initialization function is a 
>> one time function and
>> the performance improvement is not significant.
>>
>>  I'd suggest a few comments on your 2nd version[1].
>>
>> - The windows check should check the system property or other 
>> definitive os check
>>   and could be better expressed (as Alan suggested) as quotesAllowed.
>> - in the loop testing for the quote (") can come before the 
>> quotesAllowed check to
>>   speed things up (no need to check if they are allowed if they do 
>> not occur).
>>   This code is unlikely to be executed enough times to optimized.
>>
>> Roger
>>
>> [1] http://cr.openjdk.java.net/~igerasim/8067951/2/webrev/
>>
>> On 1/4/2015 3:23 PM, Ivan Gerasimov wrote:
>>>
>>> On 04.01.2015 22:50, Alan Bateman wrote:
>>>> On 03/01/2015 17:39, Ivan Gerasimov wrote:
>>>>>
>>>>> Currently, there are tree variants of ClassLoaderHelper: for 
>>>>> Windows, for Unix and for MacOS.
>>>>> We have to either duplicate code in Unix and MacOS realizations, 
>>>>> or introduce another Helper class for initializing paths only, 
>>>>> which would have only two realizations: for Windows and all Unixes.
>>>> When I made the comment then I was thinking of a method such as 
>>>> allowsQuotedPathElements (or a better name) that returns a boolean 
>>>> to indicate if quoting of path elements is allowed or not. That 
>>>> would abstract the capability a bit without needing to do isWindows 
>>>> checks.
>>>>
>>> Ah, I see.
>>> Though, not needing to check for quotes allows a bit more efficient 
>>> implementation, so splitting the code for different platforms may 
>>> also make sense.
>>>
>>> I did it with another helper class in this webrev:
>>> http://cr.openjdk.java.net/~igerasim/8067951/3/webrev/
>>>
>>> Sincerely yours,
>>> Ivan
>>>
>>>
>>>> -Alan.
>>>>
>>>>
>>>
>>
>>
>>
>