On Thu, 17 Oct 2024 08:40:05 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> I guess performance leans on there being only one or a small number of >> versions per entry. Which I think is a fair assumption. Still would be good >> to have a stress test with many entries having many versions and make sure >> we don't regress such cases too much, no? > >> I guess performance leans on there being only one or a small number of >> versions per entry. > > I checked Spring Petclinic: > > * 109 JAR dependencies > * 15 are multi-release enabled. > * The 15 MR-JARs have 9347 entries, of which 21 are versioned > * 9 of 15 MR JARs use the feature exclusively to hide `module-info.class` > from Java 8. > * 3 of the 6 remaining JAR files version a single class file > * The remaining JARs mostly version on the 9 release. > * The maximum number of versioned entries in a file is 5. > * Zero entries have more than a single version. > >> Which I think is a fair assumption. Still would be good to have a stress >> test with many entries having many versions and make sure we don't regress >> such cases too much, no? > > Such a test would be somewhat speculative. If I add 5 versions of each entry > in `ZipFileOpen`, then yes, this PR sees a regression over mainline: > > Baseline: > > Benchmark (size) Mode Cnt Score Error Units > ZipFileOpen.openCloseZipFile 1024 avgt 15 718234.380 ? 29454.645 ns/op > > > PR: > > Benchmark (size) Mode Cnt Score Error > Units > ZipFileOpen.openCloseZipFile 1024 avgt 15 1284944.601 ? 74919.522 > ns/op > > > While `JarFileGetEntry` improves over baseline: > > Baseline: > > Benchmark (mr) (size) Mode Cnt Score > Error Units > JarFileGetEntry.getEntryHit false 1024 avgt 15 68.984 ? > 2.791 ns/op > JarFileGetEntry.getEntryHit true 1024 avgt 15 324.763 ? > 9.690 ns/op > JarFileGetEntry.getEntryHitUncached false 1024 avgt 15 95.884 ? > 1.404 ns/op > JarFileGetEntry.getEntryHitUncached true 1024 avgt 15 333.491 ? > 3.576 ns/op > JarFileGetEntry.getEntryMiss false 1024 avgt 15 37.358 ? > 2.215 ns/op > JarFileGetEntry.getEntryMiss true 1024 avgt 15 650.467 ? > 5.116 ns/op > JarFileGetEntry.getEntryMissUncached false 1024 avgt 15 61.059 ? > 0.382 ns/op > JarFileGetEntry.getEntryMissUncached true 1024 avgt 15 678.457 ? > 4.390 ns/op > > > PR: > > > Benchmark (mr) (size) Mode Cnt Score > Error Units > JarFileGetEntry.getEntryHit false 1024 avgt 15 61.333 ? > 3.665 ns/op > JarFileGetEntry.getEntryHit true 1024 avgt 15 153.987 ? > 8.889 ns/op > JarFileGetEntry.getEntryHitUncached false 1024 avgt 15 90.008 ? > 4.158 ns/op > JarFileGetEntry.getEntryHitUncached true 1024 avgt 15 168.831 ? > 1.676 ns/o... I poked around with an alternative approach that uses `java.util.BitSet`. This also allows us to use the map we build up as-is, avoids sorting et.c. Performance on `JarFileGetEntry` (#21489 version) is the same in my testing, but performance on `ZipFileOpen` with your 5 versions of each entry stress test should be much improved (might beat the baseline). Can you share your local changes to that micro in a gist or e-mail so I can verify? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21489#discussion_r1805524895
