RFR: 8175891, 8192892: JrtPath improvements

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8175891, 8192892: JrtPath improvements

Claes Redestad
Hi,

please review this patch that resolves two small but sometimes
significant inefficiencies in jdk.internal.jrtfs.JrtPath:

Improve JrtPath::getResolved fast-path test:
https://bugs.openjdk.java.net/browse/JDK-8192892

Testing for '.' is insufficient as it's likely all paths have a dot in
them, while resolving is rare. Patch uses a test that is logically
equivalent to that in ZipPath::getResolved to better filter out paths
that don't need to take the slow path.

JrtPath::resolve off-by-one pre-sizing cause for memory pressure:
https://bugs.openjdk.java.net/browse/JDK-8175891

As path is normalized to never end with a / this code can be simplified.

Webrev: http://cr.openjdk.java.net/~redestad/8192892/open.0/

In a JMH micro[1] that naively walks all entries in the JDK lib/modules
and counts classes, these fixes work together to remove overhead both in
repeated and single-shot settings (the latter is relevant since jrtfs
usage may influence startup times in certain applications and tools):

Benchmark           Mode  Cnt  Score   Error  Units
JRTWalk.walkJImage  avgt   25  0.049 ± 0.003   s/op
JRTWalk.walkJImage  avgt   25  0.027 ± 0.001   s/op  # 1.8x

Benchmark           Mode  Cnt  Score   Error  Units
JRTWalk.walkJImage    ss   20  0.436 ± 0.018   s/op  (best: 0.389 s/op)
JRTWalk.walkJImage    ss   20  0.388 ± 0.017   s/op  (best: 0.363 s/op)

Thanks!

/Claes

[1] http://cr.openjdk.java.net/~redestad/8192892/JRTWalk.java
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8175891, 8192892: JrtPath improvements

Jim Laskey (Oracle)
+1


> On Dec 1, 2017, at 9:34 AM, Claes Redestad <[hidden email]> wrote:
>
> Hi,
>
> please review this patch that resolves two small but sometimes significant inefficiencies in jdk.internal.jrtfs.JrtPath:
>
> Improve JrtPath::getResolved fast-path test:
> https://bugs.openjdk.java.net/browse/JDK-8192892
>
> Testing for '.' is insufficient as it's likely all paths have a dot in them, while resolving is rare. Patch uses a test that is logically equivalent to that in ZipPath::getResolved to better filter out paths that don't need to take the slow path.
>
> JrtPath::resolve off-by-one pre-sizing cause for memory pressure:
> https://bugs.openjdk.java.net/browse/JDK-8175891
>
> As path is normalized to never end with a / this code can be simplified.
>
> Webrev: http://cr.openjdk.java.net/~redestad/8192892/open.0/
>
> In a JMH micro[1] that naively walks all entries in the JDK lib/modules and counts classes, these fixes work together to remove overhead both in repeated and single-shot settings (the latter is relevant since jrtfs usage may influence startup times in certain applications and tools):
>
> Benchmark           Mode  Cnt  Score   Error  Units
> JRTWalk.walkJImage  avgt   25  0.049 ± 0.003   s/op
> JRTWalk.walkJImage  avgt   25  0.027 ± 0.001   s/op  # 1.8x
>
> Benchmark           Mode  Cnt  Score   Error  Units
> JRTWalk.walkJImage    ss   20  0.436 ± 0.018   s/op  (best: 0.389 s/op)
> JRTWalk.walkJImage    ss   20  0.388 ± 0.017   s/op  (best: 0.363 s/op)
>
> Thanks!
>
> /Claes
>
> [1] http://cr.openjdk.java.net/~redestad/8192892/JRTWalk.java

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8175891, 8192892: JrtPath improvements

Sundararajan Athijegannathan
In reply to this post by Claes Redestad
+1

-Sundar

On 01/12/17, 7:04 PM, Claes Redestad wrote:

> Hi,
>
> please review this patch that resolves two small but sometimes
> significant inefficiencies in jdk.internal.jrtfs.JrtPath:
>
> Improve JrtPath::getResolved fast-path test:
> https://bugs.openjdk.java.net/browse/JDK-8192892
>
> Testing for '.' is insufficient as it's likely all paths have a dot in
> them, while resolving is rare. Patch uses a test that is logically
> equivalent to that in ZipPath::getResolved to better filter out paths
> that don't need to take the slow path.
>
> JrtPath::resolve off-by-one pre-sizing cause for memory pressure:
> https://bugs.openjdk.java.net/browse/JDK-8175891
>
> As path is normalized to never end with a / this code can be simplified.
>
> Webrev: http://cr.openjdk.java.net/~redestad/8192892/open.0/
>
> In a JMH micro[1] that naively walks all entries in the JDK
> lib/modules and counts classes, these fixes work together to remove
> overhead both in repeated and single-shot settings (the latter is
> relevant since jrtfs usage may influence startup times in certain
> applications and tools):
>
> Benchmark           Mode  Cnt  Score   Error  Units
> JRTWalk.walkJImage  avgt   25  0.049 ± 0.003   s/op
> JRTWalk.walkJImage  avgt   25  0.027 ± 0.001   s/op  # 1.8x
>
> Benchmark           Mode  Cnt  Score   Error  Units
> JRTWalk.walkJImage    ss   20  0.436 ± 0.018   s/op  (best: 0.389 s/op)
> JRTWalk.walkJImage    ss   20  0.388 ± 0.017   s/op  (best: 0.363 s/op)
>
> Thanks!
>
> /Claes
>
> [1] http://cr.openjdk.java.net/~redestad/8192892/JRTWalk.java
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8175891, 8192892: JrtPath improvements

Claes Redestad
Jim, Sundar,

thanks for the quick reviews!

/Claes


On 2017-12-01 14:53, Sundararajan Athijegannathan wrote:

> +1
>
> -Sundar
>
> On 01/12/17, 7:04 PM, Claes Redestad wrote:
>> Hi,
>>
>> please review this patch that resolves two small but sometimes
>> significant inefficiencies in jdk.internal.jrtfs.JrtPath:
>>
>> Improve JrtPath::getResolved fast-path test:
>> https://bugs.openjdk.java.net/browse/JDK-8192892
>>
>> Testing for '.' is insufficient as it's likely all paths have a dot
>> in them, while resolving is rare. Patch uses a test that is logically
>> equivalent to that in ZipPath::getResolved to better filter out paths
>> that don't need to take the slow path.
>>
>> JrtPath::resolve off-by-one pre-sizing cause for memory pressure:
>> https://bugs.openjdk.java.net/browse/JDK-8175891
>>
>> As path is normalized to never end with a / this code can be simplified.
>>
>> Webrev: http://cr.openjdk.java.net/~redestad/8192892/open.0/
>>
>> In a JMH micro[1] that naively walks all entries in the JDK
>> lib/modules and counts classes, these fixes work together to remove
>> overhead both in repeated and single-shot settings (the latter is
>> relevant since jrtfs usage may influence startup times in certain
>> applications and tools):
>>
>> Benchmark           Mode  Cnt  Score   Error  Units
>> JRTWalk.walkJImage  avgt   25  0.049 ± 0.003   s/op
>> JRTWalk.walkJImage  avgt   25  0.027 ± 0.001   s/op  # 1.8x
>>
>> Benchmark           Mode  Cnt  Score   Error  Units
>> JRTWalk.walkJImage    ss   20  0.436 ± 0.018   s/op  (best: 0.389 s/op)
>> JRTWalk.walkJImage    ss   20  0.388 ± 0.017   s/op  (best: 0.363 s/op)
>>
>> Thanks!
>>
>> /Claes
>>
>> [1] http://cr.openjdk.java.net/~redestad/8192892/JRTWalk.java