[9] RFR 8177969: Faster FilePermission::implies by avoiding the use of Path::relativize
Sean Mullan
sean.mullan at oracle.com
Mon Apr 10 14:08:09 UTC 2017
The updated webrev looks good.
Thanks,
Sean
On 4/7/17 8:39 PM, Weijun Wang wrote:
> Webrev updated at
>
> http://cr.openjdk.java.net/~weijun/8177969/webrev.02/
>
> Changes since webrev.01:
>
> 1. Comments.
>
> 2. Another enhancement when I am writing comments. Since we can be sure
> that a path has no ".." by only looking at its head, we can also be sure
> that a path is all ".." by only looking at its tail.
>
> JPRT core+jaxp runs fine. JCK tests run fine.
>
> Now benchmark result (after adjusting the granted permission to
> "../../../../-") is:
>
> Benchmark Mode Cnt Score Error Units
> checkReadNewUnfixed thrpt 15 174963.698 ± 5952.239 ops/s
> checkReadNewFixed00 thrpt 15 417692.698 ± 24994.735 ops/s
> checkReadNewFixed02 thrpt 15 447824.218 ± 20199.194 ops/s
> checkReadOld200 thrpt 15 425353.705 ± 3546.411 ops/s
> checkReadOld201 thrpt 15 2045.070 ± 57.287 ops/s
>
> This time checkReadNewFixed02 looks even better than checkReadOld200
> running on jdk8u131. I dare not run it again. :-)
>
> Thanks
> Max
>
> On 04/08/2017 04:48 AM, Sean Mullan wrote:
>> This fix looks good to me. However, I would suggest adding some
>> additional comments to the body of the containsPath method explaining
>> what it is doing so that it is easier to understand.
>>
>> --Sean
>>
>> On 4/7/17 10:50 AM, Weijun Wang wrote:
>>> Webrev updated at
>>>
>>> http://cr.openjdk.java.net/~weijun/8177969/webrev.01/
>>>
>>> Changes since webrev.00 [1]
>>>
>>> 1. Copyright years.
>>>
>>> 2. Test fix. check() should include Windows drive tests like contains()
>>> does.
>>>
>>> Thanks
>>> Max
>>>
>>> [1]
>>> http://cr.openjdk.java.net/~weijun/8177969/webrev.01/interdiff.patch.html
>>>
>>>
>>> On 04/05/2017 10:13 PM, Roger Riggs wrote:
>>>> Hi Max,
>>>>
>>>> The code looks ok.
>>>>
>>>> How much faster does it make FilePermission compares?
>>>>
>>>> I assume if it is not accepted to be fixed in JDK 9, you will push
>>>> it to
>>>> JDK 10.
>>>>
>>>> Roger
>>>>
>>>>
>>>> On 4/3/2017 11:30 AM, Weijun Wang wrote:
>>>>> http://cr.openjdk.java.net/~weijun/8177969/webrev.00/
>>>>>
>>>>> This new implementation of containsPath(Path,Path) uses the logic of
>>>>> Path::relativize without directly calling it, which is much faster
>>>>> now.
>>>>>
>>>>> Thanks
>>>>> Max
>>>>
More information about the core-libs-dev
mailing list