On Wed, 30 Jun 2021 11:47:42 GMT, Сергей Цыпанов
<github.com+10835776+stsypa...@openjdk.org> wrote:
>> There is an optimization opportunity for the widespread use-case when a
>> resource is read from classpath using
>> `getClass().getClassLoader().getResource()` or
>> `getClass().getClassLoader().getResourceAsStream()`.
>>
>> Pay attention to lines starting from 261. In case I run something like
>>
>> var props =
>> getClass().getClassLoader().getResource("/application.properties");
>>
>> I get into the if-else block starting from 251 and here 'separator' variable
>> is an empty String. In this case we can skip 'separator' from concatenation
>> chain and use `String.concat()` as there are only two items concatenated.
>>
>> In the opposite case `separator` variable is `"/"` and at the same time
>> `ind` variable is `-1`. This means that expression `path.substring(0, ind +
>> 1)` always returns an empty String and again can be excluded from
>> concatenation chain allowing usage of `String.concat()` which allows to
>> dodge utilization of `StringBuilder` (here `StringConcatFactory` is not
>> available, see https://github.com/openjdk/jdk/pull/3627)
>>
>> In the next else-block, starting from 274, again, `String.concat()` is
>> applicable.
>>
>> In another if-else block, starting from 277, when id is 0 again
>> path.substring(0, ind) returns empty String making concatenation pointless
>> and avoidable.
>>
>> There are also some other minor clean-ups possible regarding constant
>> conditions (lines 252 and 161).
>>
>> The change allows to reduce significantly resource look-up costs for a very
>> wide-spread case:
>>
>> @State(Scope.Benchmark)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> @Fork(jvmArgsAppend = {"-Xms2g", "-Xmx2g"})
>> public class ClassGetResourceBenchmark {
>> private final Class<?> clazz = getClass();
>>
>> @Benchmark
>> public URL getResource() {
>> return clazz.getResource("/application.properties");
>> }
>> }
>>
>> The change allows to reduce memory consumption significantly:
>>
>> before
>>
>> Benchmark Mode
>> Cnt Score Error Units
>> ClassGetResourceBenchmark.getResource avgt
>> 100 1649,367 ± 5,904 ns/op
>> ClassGetResourceBenchmark.getResource:·gc.alloc.rate avgt
>> 100 619,204 ± 2,413 MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm avgt
>> 100 1339,232 ± 4,909 B/op
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space avgt
>> 100 627,192 ± 74,972 MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm avgt
>> 100 1356,681 ± 162,354 B/op
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space avgt
>> 100 0,119 ± 0,100 MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm avgt
>> 100 0,257 ± 0,217 B/op
>> ClassGetResourceBenchmark.getResource:·gc.count avgt
>> 100 128,000 counts
>> ClassGetResourceBenchmark.getResource:·gc.time avgt
>> 100 227,000 ms
>>
>> after
>>
>> Benchmark Mode
>> Cnt Score Error Units
>> ClassGetResourceBenchmark.getResource avgt
>> 100 1599,948 ± 4,115 ns/op
>> ClassGetResourceBenchmark.getResource:·gc.alloc.rate avgt
>> 100 358,434 ± 0,922 MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.alloc.rate.norm avgt
>> 100 752,016 ± 0,004 B/op
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space avgt
>> 100 342,778 ± 76,490 MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Eden_Space.norm avgt
>> 100 719,264 ± 160,513 B/op
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space avgt
>> 100 0,008 ± 0,005 MB/sec
>> ClassGetResourceBenchmark.getResource:·gc.churn.G1_Survivor_Space.norm avgt
>> 100 0,017 ± 0,010 B/op
>> ClassGetResourceBenchmark.getResource:·gc.count avgt
>> 100 70,000 counts
>> ClassGetResourceBenchmark.getResource:·gc.time avgt
>> 100 151,000 ms
>
> Сергей Цыпанов has updated the pull request with a new target base due to a
> merge or a rebase. The incremental webrev excludes the unrelated changes
> brought in by the merge/rebase. The pull request contains three additional
> commits since the last revision:
>
> - Merge branch 'master' into 8267840
> - Merge branch 'master' into 8267840
> - 8267840: Improve URLStreamHandler.parseURL()
src/java.base/share/classes/java/net/URLStreamHandler.java line 275:
> 273: path = "/";
> 274: } else {
> 275: path = path.substring(0, ind).concat("/");
would that be equivalent to
path = path.substring(0, ind + 1);
given that ind = path.lastIndexOf('/') ?
-------------
PR: https://git.openjdk.java.net/jdk/pull/4526
