RFR: 8067951: System.loadLibrary cannot find library when path contains quoted entry
Roger.Riggs at Oracle.com
Mon Jan 5 18:10:46 UTC 2015
Looks pretty good.
Just a thought on ClassLoader:1754; If you avoid the local for
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)
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.
> Sincerely yours,
> 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.
>> - 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.
>>  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
>>> 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:
>>> Sincerely yours,
More information about the core-libs-dev